From 82bcaa5a54ff71419a2a592699e426d83a617616 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 4 Aug 2026 17:47:49 -0500 Subject: [PATCH] refactor: idiomatic test sandbox setup (manual mode, single reset hook) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the non-idiomatic :auto sandbox mode with :manual, eliminating the root cause of stale data across test runs. The :auto mode let any process auto-checkout and commit outside test transactions — Oban in testing:inline already disables plugins/queues, so :auto was never needed. Changes: - test_helper: Sandbox.mode(Repo, :manual); drop manual app boot (mix now starts the app idiomatically), drop --no-start alias, drop table-cleanup loop - DataCase: single reset_test_state/0 consolidates GridCache.clear, ScoreCache.clear, score-file wipe, and default HTTP stubs; drop the :auto-restore on_exit hack; setup_sandbox now just start_owner - ConnCase: delegates shared reset to DataCase, removes duplicate cache clears and stub installs - config: pool Ecto.Adapters.SQL.Sandbox (idiomatic canonical form) Note: GridCache/ScoreCache are globally-registered GenServers, so the architecturally clean per-test start_supervised! approach conflicts with async:true (parallel tests can't register the same name). The centralized clear in reset_test_state is the practical compromise — one place, documented, single call. --- config/test.exs | 2 +- mix.exs | 2 +- .../workers/backfill_enqueue_worker_test.exs | 4 +- test/support/conn_case.ex | 46 +--------- test/support/data_case.ex | 89 ++++++------------- test/test_helper.exs | 39 ++------ 6 files changed, 39 insertions(+), 143 deletions(-) diff --git a/config/test.exs b/config/test.exs index 362876b9..131deeb7 100644 --- a/config/test.exs +++ b/config/test.exs @@ -34,7 +34,7 @@ config :microwaveprop, Microwaveprop.Repo, password: "postgres", hostname: "localhost", database: "microwaveprop_test#{System.get_env("MIX_TEST_PARTITION")}", - pool: DBConnection.Ownership, + pool: Sandbox, pool_size: System.schedulers_online() * 2, queue_target: 500, queue_interval: 5000, diff --git a/mix.exs b/mix.exs index f8a338d7..99c75df5 100644 --- a/mix.exs +++ b/mix.exs @@ -167,7 +167,7 @@ defmodule Microwaveprop.MixProject do "ecto.setup": ["ecto.create", "ecto.migrate", "run priv/repo/seeds.exs"], "ecto.reset": ["ecto.drop", "ecto.setup"], "test.setup": ["ecto.create --quiet", "ecto.migrate --quiet"], - test: ["test --no-start"], + test: ["test"], "assets.setup": ["tailwind.install --if-missing", "esbuild.install --if-missing"], "assets.build": [ "compile", diff --git a/test/microwaveprop/workers/backfill_enqueue_worker_test.exs b/test/microwaveprop/workers/backfill_enqueue_worker_test.exs index f0b3a607..792bd5be 100644 --- a/test/microwaveprop/workers/backfill_enqueue_worker_test.exs +++ b/test/microwaveprop/workers/backfill_enqueue_worker_test.exs @@ -93,7 +93,7 @@ defmodule Microwaveprop.Workers.BackfillEnqueueWorkerTest do test "enqueues all pending contacts when limit is omitted" do Phoenix.PubSub.subscribe(Microwaveprop.PubSub, "backfill:enqueue_complete") - our_ids = + _our_ids = for i <- 1..7 do ts = DateTime.shift(~U[2026-01-01 00:00:00Z], hour: i) create_contact(%{qso_timestamp: ts}).id @@ -111,7 +111,7 @@ defmodule Microwaveprop.Workers.BackfillEnqueueWorkerTest do test "broadcasts enqueue_complete with count" do Phoenix.PubSub.subscribe(Microwaveprop.PubSub, "backfill:enqueue_complete") - contact = create_contact() + _contact = create_contact() assert :ok = BackfillEnqueueWorker.perform(%Oban.Job{ diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index cf1f8a53..ac3a1d2f 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -1,17 +1,6 @@ defmodule MicrowavepropWeb.ConnCase do @moduledoc """ Test case template for HTTP connection tests. - - Such tests rely on `Phoenix.ConnTest` and also - import other functionality to make it easier - to build common data structures and query the data layer. - - Finally, if the test case interacts with the database, - we enable the SQL sandbox, so changes done to the database - are reverted at the end of every test. If you are using - PostgreSQL, you can even run database tests asynchronously - by setting `use MicrowavepropWeb.ConnCase, async: true`, although - this option is not recommended for other databases. """ use ExUnit.CaseTemplate @@ -21,8 +10,6 @@ defmodule MicrowavepropWeb.ConnCase do alias Microwaveprop.Accounts.User alias Microwaveprop.AccountsFixtures alias Microwaveprop.DataCase - alias Microwaveprop.Propagation.ScoreCache - alias Microwaveprop.Weather.GridCache alias Phoenix.Ecto.SQL.Sandbox using do @@ -32,60 +19,33 @@ defmodule MicrowavepropWeb.ConnCase do import MicrowavepropWeb.ConnCase import Phoenix.ConnTest import Plug.Conn - # The default endpoint for testing - @endpoint MicrowavepropWeb.Endpoint - # Import conveniences for testing with connections + @endpoint MicrowavepropWeb.Endpoint end end setup tags do owner = DataCase.setup_sandbox(tags) - DataCase.reset_score_files() - DataCase.stub_nexrad_default() - GridCache.clear() - ScoreCache.clear() + DataCase.reset_test_state() - # Stamp the sandbox metadata onto the "user-agent" header so a - # LiveView's connected mount (a separate process from this one, per - # `MicrowavepropWeb.SandboxHook`) can see this test's transaction. metadata = Sandbox.metadata_for(Microwaveprop.Repo, owner) - conn = Plug.Conn.put_req_header(Phoenix.ConnTest.build_conn(), "user-agent", Sandbox.encode_metadata(metadata)) {:ok, conn: conn} end - @doc """ - Setup helper that registers and logs in users. - - setup :register_and_log_in_user - - It stores an updated connection and a registered user in the - test context. - """ @spec register_and_log_in_user(map()) :: %{conn: Plug.Conn.t(), user: User.t(), scope: Scope.t()} def register_and_log_in_user(%{conn: conn} = context) do user = AccountsFixtures.user_fixture() scope = Scope.for_user(user) - opts = - context - |> Map.take([:token_authenticated_at]) - |> Enum.to_list() - + opts = context |> Map.take([:token_authenticated_at]) |> Enum.to_list() %{conn: log_in_user(conn, user, opts), user: user, scope: scope} end - @doc """ - Logs the given `user` into the `conn`. - - It returns an updated `conn`. - """ @spec log_in_user(Plug.Conn.t(), User.t(), Keyword.t()) :: Plug.Conn.t() def log_in_user(conn, user, opts \\ []) do token = Accounts.generate_user_session_token(user) - maybe_set_token_authenticated_at(token, opts[:token_authenticated_at]) conn diff --git a/test/support/data_case.ex b/test/support/data_case.ex index 2ba7ad7f..9b9fd6c8 100644 --- a/test/support/data_case.ex +++ b/test/support/data_case.ex @@ -1,21 +1,13 @@ defmodule Microwaveprop.DataCase do @moduledoc """ Test case template for tests that need database access. - - You may define functions here to be used as helpers in - your tests. - - Finally, if the test case interacts with the database, - we enable the SQL sandbox, so changes done to the database - are reverted at the end of every test. If you are using - PostgreSQL, you can even run database tests asynchronously - by setting `use Microwaveprop.DataCase, async: true`, although - this option is not recommended for other databases. """ use ExUnit.CaseTemplate alias Ecto.Adapters.SQL.Sandbox + alias Microwaveprop.Propagation.ScoreCache + alias Microwaveprop.Weather.GridCache using do quote do @@ -29,79 +21,50 @@ defmodule Microwaveprop.DataCase do end setup tags do - Microwaveprop.DataCase.setup_sandbox(tags) - Microwaveprop.DataCase.reset_score_files() - Microwaveprop.DataCase.stub_nexrad_default() + pid = Microwaveprop.DataCase.setup_sandbox(tags) + Microwaveprop.DataCase.reset_test_state() - # sync tests (async: false) need shared Req.Test mode so stubs - # are visible to Tasks spawned by async_stream_nolink. Req.Test.set_req_test_from_context(tags) :ok end @doc """ - Installs a default 404 stub for the IEM NEXRAD client so tests that - trigger `CommonVolumeRadarWorker` via `enqueue_for_contact/1` don't - crash for lack of a Req.Test plug. Individual tests can override with - their own `Req.Test.stub/2`. + Reset state that persists across tests within a single ExUnit run. + + The sandbox isolates the database, but ETS-backed caches + (GridCache, ScoreCache) and on-disk score files survive across + test boundaries because the cache GenServers and filesystem live + in the app's BEAM process tree, not per-test. A single reset + here avoids per-file setup duplication. """ - @spec stub_nexrad_default :: :ok - def stub_nexrad_default do + @spec reset_test_state :: :ok + def reset_test_state do + dir = Application.get_env(:microwaveprop, :propagation_scores_dir) + if is_binary(dir), do: File.rm_rf(dir) + + GridCache.clear() + ScoreCache.clear() + Req.Test.stub(Microwaveprop.Weather.NexradClient, fn conn -> Plug.Conn.send_resp(conn, 404, "not found") end) + + Req.Test.stub(Microwaveprop.Weather.HrrrClient, fn conn -> + Plug.Conn.send_resp(conn, 404, "not found") + end) + + :ok end - @doc """ - Sets up the sandbox based on the test tags. Returns the owner pid so - callers (e.g. `MicrowavepropWeb.ConnCase`) can build Ecto Sandbox - metadata for out-of-process access (LiveView's connected mount). - """ @spec setup_sandbox(map()) :: pid() def setup_sandbox(tags) do shared? = not tags[:async] pid = Sandbox.start_owner!(Microwaveprop.Repo, shared: shared?) - - on_exit(fn -> - Sandbox.stop_owner(pid) - - # Ecto reverts the pool to :manual mode when a `{:shared, pid}` - # owner terminates (see Ecto.Adapters.SQL.Sandbox docs). Since - # test_helper.exs relies on global :auto mode so Oban's - # background plugins (Stager, Peers, Met.Reporter) can - # auto-checkout, every `async: false` test permanently flips the - # pool to :manual once it exits — starving those processes of - # connections and crash-looping Oban until it exceeds its - # restart intensity and takes the whole app (and Repo) down with - # it. Restore :auto after any shared owner exits. - if shared?, do: Sandbox.mode(Microwaveprop.Repo, :auto) - end) - + on_exit(fn -> Sandbox.stop_owner(pid) end) pid end - @doc """ - Wipe the propagation ScoresFile tree between tests so files don't - leak between cases sharing the same tmp dir. - """ - @spec reset_score_files :: :ok - def reset_score_files do - dir = Application.get_env(:microwaveprop, :propagation_scores_dir) - - if is_binary(dir) do - File.rm_rf(dir) - end - end - - @doc """ - A helper that transforms changeset errors into a map of messages. - - assert {:error, changeset} = Accounts.create_user(%{password: "short"}) - assert "password is too short" in errors_on(changeset).password - assert %{password: ["password is too short"]} = errors_on(changeset) - - """ @spec errors_on(Ecto.Changeset.t()) :: map() def errors_on(changeset) do Ecto.Changeset.traverse_errors(changeset, fn {message, opts} -> diff --git a/test/test_helper.exs b/test/test_helper.exs index 41ac09ec..54561b69 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -17,35 +17,14 @@ Code.require_file("test/support/fixtures/accounts_fixtures.ex") Code.require_file("test/support/fixtures/beacons_fixtures.ex") Code.require_file("test/support/fixtures/contacts_fixtures.ex") -# ExUnit.start/1 with capture_log: true starts :logger, which cascades -# into the full OTP application. mix.exs now runs `mix test --no-start` -# so *we* control when the app boots. Initialize ExUnit first (no logger -# cascade), force the Sandbox pool into the application environment, -# then start the application manually. ExUnit.start() -# With ExUnit initialized but no app running, the Repo supervisor hasn't -# started yet. Force the Sandbox ownership pool into the app env before -# the Repo child reads it. -for repo <- [Microwaveprop.Repo] do - existing = Application.get_env(:microwaveprop, repo, []) - - if existing[:url] || existing[:database] do - Application.put_env(:microwaveprop, repo, Keyword.put(existing, :pool, DBConnection.Ownership)) - end -end - -# Now start the full OTP app. The Repo supervisor reads pool: -# DBConnection.Ownership from the app env and initializes with it. -{:ok, _} = Application.ensure_all_started(:microwaveprop) - -# Clean up any rows left behind by a crashed or killed prior run. -# The sandbox rolls back per-test, but if the suite itself crashes -# mid-run, committed data persists and causes count-mismatch failures -# in the next run. Wipe known test-data tables before each suite. -for table <- ~w(contacts hrrr_fetch_tasks iemre_observations oban_jobs) do - Microwaveprop.Repo.delete_all(table) -end +# :manual mode: only the test process (via start_owner! / checkout) and +# processes explicitly allowed via Sandbox.allow/4 or Caller Tracking +# ($callers) can access the database. Background processes (Oban plugins, +# boot Tasks) that auto-checked out during app boot are checked in here, +# preventing any stale data from leaking across tests or across runs. +Sandbox.mode(Microwaveprop.Repo, :manual) # stream_data is a test-only dep — its ebin is on the code path from the # wildcard above, but the application isn't started. `check all` etc. @@ -53,12 +32,6 @@ end # the app to be loaded. Application.ensure_all_started(:stream_data) -# :auto mode lets every process (including background Tasks started by -# Application.init and Oban workers) auto-checkout from the sandbox. -# DataCase.setup_sandbox/1 still calls start_owner!/2 for per-test -# isolation, which is compatible with :auto mode. -Sandbox.mode(Microwaveprop.Repo, :auto) - # lazy_html provides an Enumerable protocol implementation for the # LazyHTML struct. When protocol consolidation is enabled (the default), # the consolidated Elixir.Enumerable.beam file is generated during