-
Notifications
You must be signed in to change notification settings - Fork 77
Description
I think this is related to #170
After upgrading from Mox 1.1.0 to 1.2.0, database records leak between async: false tests when verify_on_exit!/1 is used alongside set_mox_from_context (or set_mox_global) and Ecto's SQL Sandbox in shared mode.
The root I think is that verify_on_exit!/1 calls NimbleOwnership.set_owner_to_manual_cleanup/2, which prevents NimbleOwnership from cleaning up when the test process dies. Instead, cleanup is deferred to an on_exit callback. This interacts badly with Ecto Sandbox's stop_owner (which also runs in on_exit), because the ordering of on_exit callbacks is not guaranteed, and the delayed NimbleOwnership cleanup appears to interfere with Ecto Sandbox's ability to properly rollback the shared database transaction.
The setup stripped down would be this:
# test/support/repo.ex
defmodule MyApp.Repo do
use Ecto.Repo, otp_app: :my_app, adapter: Ecto.Adapters.Postgres
end
# test/support/data_case.ex
# Follows phoenix pattern for generating DataCase
defmodule MyApp.DataCase do
use ExUnit.CaseTemplate
alias Ecto.Adapters.SQL.Sandbox
setup tags do
pid = Sandbox.start_owner!(MyApp.Repo, shared: not tags[:async])
on_exit(fn -> Sandbox.stop_owner(pid) end)
:ok
end
end
# lib/my_app/weather.ex
defmodule MyApp.WeatherBehaviour do
@callback get_temp(String.t()) :: {:ok, integer()}
end
# test/support/mocks.ex (in test_helper.exs or similar)
Mox.defmock(MyApp.MockWeather, for: MyApp.WeatherBehaviour)
# lib/my_app/thing.ex
defmodule MyApp.Thing do
use Ecto.Schema
schema "things" do
field :name, :string
timestamps()
end
end
# test/my_app/leak_test.exs
defmodule MyApp.LeakTest do
# Let's assume the code being tested spawns processes with DB access and we need async false.
# For brevity setting this up has is omitted from this example
use MyApp.DataCase, async: false
import Mox
setup :set_mox_from_context
setup :verify_on_exit! # ← THIS CAUSES THE ISSUE. Worked fine with up to Mox 1.1.0
test "test A - inserts a record" do
stub(MyApp.MockWeather, :get_temp, fn _ -> {:ok, 25} end)
MyApp.Repo.insert!(%MyApp.Thing{name: "from_test_A"})
assert MyApp.Repo.aggregate(MyApp.Thing, :count) == 1
end
test "test B - expects empty table but finds leaked record" do
stub(MyApp.MockWeather, :get_temp, fn _ -> {:ok, 30} end)
# THIS FAILS: expects 0 but gets 1 because test A's record leaked
assert MyApp.Repo.aggregate(MyApp.Thing, :count) == 0
end
endExpected behaviour: Test B sees 0 records because test A's transaction should have been rolled back.
Actual behaviour: Test B sees 1 record because the transaction rollback from test A did not work properly. This is intermittent and depends on test ordering (run with --seed 0 or similar to reproduce consistently).
Note: Removing setup :verify_on_exit! fixes the issue. The problem only occurs in async: false tests (shared Sandbox mode). async: true tests are unaffected because they use private Sandbox mode.
A few possible approaches:
- Document the limitation — Add a warning to
verify_on_exit!/1docs that it's incompatible with Ecto Sandbox in shared mode (async: false) and recommend usingverify!explicitly after each test - Detect shared mode and warn — When verify_on_exit! is called and the NimbleOwnership server is in
{:shared, _}mode, emit a warning or raise. Would alert others of issues in their setup - Use a different verification strategy — Instead of set_owner_to_manual_cleanup, use a separate mechanism to track whether verification is needed restoring the 1.1.0 behaviour.
Let me know if the issue makes sense, been trying to wrap my head around why tests have been flaking for the past few days 😅