Fix all flaky tests (#613)

Also removes Mox, yay!

Author: whatyouhide

config/config.exs

@@ -9,8 +9,9 @@ if config_env() == :test do
     root_source_code_paths: [File.cwd!()],
     hackney_opts: [recv_timeout: 50],
     send_result: :sync,
-    send_max_attempts: 1,
-    __sender_module__: Sentry.TransportSenderMock
+    send_max_attempts: 1
+
+  config :logger, backends: []
 end
 
 config :phoenix, :json_library, Jason

lib/sentry/client.ex

@@ -12,11 +12,6 @@ defmodule Sentry.Client do
   # Max message length per https://github.com/getsentry/sentry/blob/0fcec33ac94ad81a205f86f208072b0f57b39ff4/src/sentry/conf/server.py#L1021
   @max_message_length 8_192
 
-  # We read this at compile time and use it exclusively for tests. Any user of the Sentry
-  # application will get the real deal, but we'll be able to swap this out with a mock
-  # in tests.
-  @sender_module Application.compile_env(:sentry, :__sender_module__, Transport.Sender)
-
   # This is what executes the "Event Pipeline".
   # See: https://develop.sentry.dev/sdk/unified-api/#event-pipeline
   @spec send_event(Event.t(), keyword()) ::
@@ -95,7 +90,7 @@ defmodule Sentry.Client do
   end
 
   defp encode_and_send(%Event{} = event, _result_type = :none, _request_retries) do
-    :ok = @sender_module.send_async(event)
+    :ok = Transport.Sender.send_async(event)
     Sentry.put_last_event_id_and_source(event.event_id, event.source)
     {:ok, ""}
   end

lib/sentry/transport/sender.ex

@@ -9,14 +9,6 @@ defmodule Sentry.Transport.Sender do
 
   @registry Sentry.Transport.SenderRegistry
 
-  # This behaviour is only present for mocks in tests.
-  defmodule Behaviour do
-    @moduledoc false
-    @callback send_async(Event.t()) :: :ok
-  end
-
-  @behaviour Behaviour
-
   ## Public API
 
   @spec start_link(keyword()) :: GenServer.on_start()
@@ -25,7 +17,6 @@ defmodule Sentry.Transport.Sender do
     GenServer.start_link(__MODULE__, [], name: {:via, Registry, {@registry, index}})
   end
 
-  @impl Behaviour
   @spec send_async(Event.t()) :: :ok
   def send_async(%Event{} = event) do
     pool_size = Application.fetch_env!(:sentry, :sender_pool_size)

mix.exs

@@ -83,7 +83,6 @@ defmodule Sentry.Mixfile do
       {:dialyxir, "~> 1.0", only: [:test, :dev], runtime: false},
       {:ex_doc, "~> 0.29.0", only: :dev},
       {:excoveralls, "~> 0.17.1", only: [:test]},
-      {:mox, "~> 1.0", only: [:test]},
       {:phoenix, "~> 1.5", only: [:test]},
       {:phoenix_html, "~> 2.0", only: [:test]}
     ]

mix.lock

@@ -5,7 +5,6 @@
   "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.3.1", "ebd1a1d7aff97f27c66654e78ece187abdc646992714164380d8a041eda16754", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "3a6efd3366130eab84ca372cbd4a7d3c3a97bdfcfb4911233b035d117063f0af"},
   "cowlib": {:hex, :cowlib, "2.9.1", "61a6c7c50cf07fdd24b2f45b89500bb93b6686579b069a89f88cb211e1125c78", [:rebar3], [], "hexpm", "e4175dc240a70d996156160891e1c62238ede1729e45740bdd38064dad476170"},
   "dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"},
-  "earmark": {:hex, :earmark, "1.4.5", "62ffd3bd7722fb7a7b1ecd2419ea0b458c356e7168c1f5d65caf09b4fbdd13c8", [:mix], [], "hexpm", "b7d0e6263d83dc27141a523467799a685965bf8b13b6743413f19a7079843f4f"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.32", "fa739a0ecfa34493de19426681b23f6814573faee95dfd4b4aafe15a7b5b32c6", [:mix], [], "hexpm", "b8b0dd77d60373e77a3d7e8afa598f325e49e8663a51bcc2b88ef41838cca755"},
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
   "ex_doc": {:hex, :ex_doc, "0.29.4", "6257ecbb20c7396b1fe5accd55b7b0d23f44b6aa18017b415cb4c2b91d997729", [:mix], [{:earmark_parser, "~> 1.4.31", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "2c6699a737ae46cb61e4ed012af931b57b699643b24dabe2400a8168414bc4f5"},
@@ -19,7 +18,6 @@
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
   "mime": {:hex, :mime, "1.5.0", "203ef35ef3389aae6d361918bf3f952fa17a09e8e43b5aa592b93eba05d0fb8d", [:mix], [], "hexpm", "55a94c0f552249fc1a3dd9cd2d3ab9de9d3c89b559c2bd01121f824834f24746"},
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
-  "mox": {:hex, :mox, "1.0.2", "dc2057289ac478b35760ba74165b4b3f402f68803dd5aecd3bfd19c183815d64", [:mix], [], "hexpm", "f9864921b3aaf763c8741b5b8e6f908f44566f1e427b2630e89e9a73b981fef2"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.3.1", "2c54013ecf170e249e9291ed0a62e5832f70a476c61da16f6aac6dca0189f2af", [:mix], [], "hexpm", "2682e3c0b2eb58d90c6375fc0cc30bc7be06f365bf72608804fb9cffa5e1b167"},
   "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
   "phoenix": {:hex, :phoenix, "1.5.8", "71cfa7a9bb9a37af4df98939790642f210e35f696b935ca6d9d9c55a884621a4", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_html, "~> 2.13", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 1.0 or ~> 2.2", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.1.2 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "35ded0a32f4836168c7ab6c33b88822eccd201bcd9492125a9bea4c54332d955"},
@@ -30,7 +28,6 @@
   "plug_crypto": {:hex, :plug_crypto, "1.2.2", "05654514ac717ff3a1843204b424477d9e60c143406aa94daf2274fdd280794d", [:mix], [], "hexpm", "87631c7ad914a5a445f0a3809f99b079113ae4ed4b867348dd9eec288cecb6db"},
   "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm", "451d8527787df716d99dc36162fca05934915db0b6141bbdac2ea8d3c7afc7d7"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
-  "ssl_verify_hostname": {:hex, :ssl_verify_hostname, "1.0.5", "2e73e068cd6393526f9fa6d399353d7c9477d6886ba005f323b592d389fb47be", [:make], []},
   "telemetry": {:hex, :telemetry, "0.4.2", "2808c992455e08d6177322f14d3bdb6b625fbcfd233a73505870d8738a2f4599", [:rebar3], [], "hexpm", "2d1419bd9dda6a206d7b5852179511722e2b18812310d304620c7bd92a13fcef"},
   "unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"},
 }

test/event_test.exs

@@ -388,7 +388,6 @@ defmodule Sentry.EventTest do
              :metrics,
              :mime,
              :mimerl,
-             :mox,
              :parse_trans,
              :phoenix,
              :phoenix_html,

test/logger_backend_test.exs

@@ -1,40 +1,24 @@
 defmodule Sentry.LoggerBackendTest do
-  use ExUnit.Case
-
-  import Mox
+  use ExUnit.Case, async: false
 
+  alias Sentry.TestEnvironmentHelper
   alias Sentry.TestGenServer
 
   require Logger
 
   @moduletag :capture_log
 
-  setup :set_mox_global
-  setup :verify_on_exit!
-
   setup do
-    {:ok, _} = Logger.add_backend(Sentry.LoggerBackend)
+    assert {:ok, _} = Logger.add_backend(Sentry.LoggerBackend)
 
-    ExUnit.Callbacks.on_exit(fn ->
+    on_exit(fn ->
       Logger.configure_backend(Sentry.LoggerBackend, [])
       :ok = Logger.remove_backend(Sentry.LoggerBackend)
     end)
   end
 
-  defp expect_sender_call do
-    pid = self()
-    ref = make_ref()
-
-    expect(Sentry.TransportSenderMock, :send_async, fn event ->
-      send(pid, {ref, event})
-      :ok
-    end)
-
-    ref
-  end
-
   test "a logged raised exception is reported" do
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Task.start(fn ->
       raise "Unique Error"
@@ -47,7 +31,7 @@ defmodule Sentry.LoggerBackendTest do
   end
 
   test "a GenServer throw is reported" do
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
     Sentry.TestGenServer.throw(pid)
@@ -61,7 +45,7 @@ defmodule Sentry.LoggerBackendTest do
   end
 
   test "abnormal GenServer exit is reported" do
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
     Sentry.TestGenServer.exit(pid)
@@ -75,7 +59,7 @@ defmodule Sentry.LoggerBackendTest do
   end
 
   test "bad function call causing GenServer crash is reported" do
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
 
@@ -96,7 +80,7 @@ defmodule Sentry.LoggerBackendTest do
   end
 
   test "GenServer timeout is reported" do
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
 
@@ -120,7 +104,7 @@ defmodule Sentry.LoggerBackendTest do
   end
 
   test "captures errors from spawn/0 in Plug app" do
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     :get
     |> Plug.Test.conn("/spawn_error_route")
@@ -135,7 +119,7 @@ defmodule Sentry.LoggerBackendTest do
   test "sends two errors when a Plug process crashes if cowboy domain is not excluded" do
     Logger.configure_backend(Sentry.LoggerBackend, excluded_domains: [])
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     {:ok, _plug_pid} = Plug.Cowboy.http(Sentry.ExamplePlugApplication, [], port: 8003)
 
@@ -152,7 +136,7 @@ defmodule Sentry.LoggerBackendTest do
       excluded_domains: [:test_domain]
     )
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Logger.error("no domain")
     Logger.error("test_domain", domain: [:test_domain])
@@ -164,7 +148,7 @@ defmodule Sentry.LoggerBackendTest do
   test "includes Logger metadata for keys configured to be included" do
     Logger.configure_backend(Sentry.LoggerBackend, metadata: [:string, :number, :map, :list])
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
     Sentry.TestGenServer.add_logger_metadata(pid, :string, "string")
@@ -182,7 +166,7 @@ defmodule Sentry.LoggerBackendTest do
 
   test "does not include Logger metadata when disabled" do
     Logger.configure_backend(Sentry.LoggerBackend, metadata: [])
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
     Sentry.TestGenServer.add_logger_metadata(pid, :string, "string")
@@ -197,7 +181,7 @@ defmodule Sentry.LoggerBackendTest do
 
   test "supports :all for Logger metadata" do
     Logger.configure_backend(Sentry.LoggerBackend, metadata: :all)
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     pid = start_supervised!(TestGenServer)
     Sentry.TestGenServer.add_logger_metadata(pid, :string, "string")
@@ -220,7 +204,7 @@ defmodule Sentry.LoggerBackendTest do
   test "sends all messages if :capture_log_messages is true" do
     Logger.configure_backend(Sentry.LoggerBackend, capture_log_messages: true)
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Logger.error("Testing")
 
@@ -236,7 +220,7 @@ defmodule Sentry.LoggerBackendTest do
       capture_log_messages: true
     )
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Sentry.Context.set_user_context(%{user_id: 3})
     Logger.log(:warning, "Testing")
@@ -252,7 +236,7 @@ defmodule Sentry.LoggerBackendTest do
   test "does not send debug messages when configured to :error" do
     Logger.configure_backend(Sentry.LoggerBackend, capture_log_messages: true)
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Sentry.Context.set_user_context(%{user_id: 3})
 
@@ -268,7 +252,7 @@ defmodule Sentry.LoggerBackendTest do
 
   test "Sentry metadata and extra context are retrieved from callers" do
     Logger.configure_backend(Sentry.LoggerBackend, capture_log_messages: true)
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Sentry.Context.set_extra_context(%{day_of_week: "Friday"})
     Sentry.Context.set_user_context(%{user_id: 3})
@@ -288,7 +272,7 @@ defmodule Sentry.LoggerBackendTest do
 
   test "handles malformed :callers metadata" do
     Logger.configure_backend(Sentry.LoggerBackend, capture_log_messages: true)
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     dead_pid = spawn(fn -> :ok end)
 
@@ -304,7 +288,7 @@ defmodule Sentry.LoggerBackendTest do
       capture_log_messages: true
     )
 
-    ref = expect_sender_call()
+    ref = register_before_send()
 
     Logger.log(:warning, "warn")
 
@@ -326,4 +310,18 @@ defmodule Sentry.LoggerBackendTest do
         exit(:shutdown)
     end
   end
+
+  defp register_before_send(_context \\ %{}) do
+    pid = self()
+    ref = make_ref()
+
+    TestEnvironmentHelper.modify_env(:sentry,
+      before_send_event: fn event ->
+        send(pid, {ref, event})
+        false
+      end
+    )
+
+    ref
+  end
 end

test/sentry/client_test.exs

@@ -2,19 +2,10 @@ defmodule Sentry.ClientTest do
   use ExUnit.Case
 
   import ExUnit.CaptureLog
-  import Mox
   import Sentry.TestEnvironmentHelper
 
   alias Sentry.{Client, Event}
 
-  setup :set_mox_global
-  setup :verify_on_exit!
-
-  setup do
-    Mox.stub_with(Sentry.TransportSenderMock, Sentry.Transport.Sender)
-    :ok
-  end
-
   describe "render_event/1" do
     test "transforms structs into maps" do
       event = Event.transform_exception(%RuntimeError{message: "foo"}, user: %{id: 1})