From e781ea1ec9b7c886ccb4486f93c865d32ce698f7 Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Wed, 5 Aug 2026 07:01:48 +0200 Subject: [PATCH 1/2] fix(test): the fixture timeout has to outlast the property it wraps #374 raised the property floor to 300s and, separately, wrapped each fixture in {timeout, 120, {setup, ...}} to cover setup/0 and cleanup/1. The outer timeout covers the whole fixture, tests included, so 120 pre-empted the 300 it was meant to complement and cancelled the group anyway - with zero failures and nothing named, the same symptom. The failure moved rather than went away: prop_reconnect_lifecycle before #374, prop_input_never_dropped after it, both around the 6.5-minute mark. The outer one is a hang detector for setup and cleanup, not a budget for the tests inside. 900s, and the rule is written down where the next person will hit it. --- test/prop_chat_zone_routing.erl | 15 +++++++++------ test/prop_input_never_dropped.erl | 15 +++++++++------ test/prop_reconnect_lifecycle.erl | 15 +++++++++------ test/prop_zone_invariants.erl | 15 +++++++++------ 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/test/prop_chat_zone_routing.erl b/test/prop_chat_zone_routing.erl index 5fdaf44c..bfc3b6fe 100644 --- a/test/prop_chat_zone_routing.erl +++ b/test/prop_chat_zone_routing.erl @@ -20,12 +20,15 @@ chat_zone_routing_test_() -> %% Two timeouts, because there are two ways this cancels a group with %% zero failures and nothing named. The outer one covers setup/0 and - %% cleanup/1, which otherwise run under eunit's 5s default; the inner - %% one is the property's own, and its floor is 300s rather than 60s - %% because 60 is a performance budget, not a hang detector - a CI runner - %% managed 21 of 25 iterations of the reconnect property inside it - %% (asobi#376). - {timeout, 120, + %% cleanup/1, which otherwise run under eunit's 5s default; the inner one + %% is the property's own, and its floor is 300s rather than 60s because 60 + %% is a performance budget, not a hang detector (asobi#376). + %% + %% The outer MUST outlast the inner. It covers the whole fixture, tests + %% included, so a shorter outer silently pre-empts the inner and cancels + %% the group - which is exactly what #374 did, moving the failure from one + %% property module to the next. + {timeout, 900, {setup, fun setup/0, fun cleanup/1, [ {timeout, max(300, ?NUMTESTS div 2), ?_assert( diff --git a/test/prop_input_never_dropped.erl b/test/prop_input_never_dropped.erl index e91d18a4..d98f9e8e 100644 --- a/test/prop_input_never_dropped.erl +++ b/test/prop_input_never_dropped.erl @@ -32,12 +32,15 @@ input_never_dropped_test_() -> %% Two timeouts, because there are two ways this cancels a group with %% zero failures and nothing named. The outer one covers setup/0 and - %% cleanup/1, which otherwise run under eunit's 5s default; the inner - %% one is the property's own, and its floor is 300s rather than 60s - %% because 60 is a performance budget, not a hang detector - a CI runner - %% managed 21 of 25 iterations of the reconnect property inside it - %% (asobi#376). - {timeout, 120, + %% cleanup/1, which otherwise run under eunit's 5s default; the inner one + %% is the property's own, and its floor is 300s rather than 60s because 60 + %% is a performance budget, not a hang detector (asobi#376). + %% + %% The outer MUST outlast the inner. It covers the whole fixture, tests + %% included, so a shorter outer silently pre-empts the inner and cancels + %% the group - which is exactly what #374 did, moving the failure from one + %% property module to the next. + {timeout, 900, {setup, fun setup/0, fun cleanup/1, fun(Ctx) -> [ {timeout, max(300, ?NUMTESTS div 2), diff --git a/test/prop_reconnect_lifecycle.erl b/test/prop_reconnect_lifecycle.erl index 17b7676e..9b066266 100644 --- a/test/prop_reconnect_lifecycle.erl +++ b/test/prop_reconnect_lifecycle.erl @@ -34,12 +34,15 @@ reconnect_lifecycle_test_() -> %% Two timeouts, because there are two ways this cancels a group with %% zero failures and nothing named. The outer one covers setup/0 and - %% cleanup/1, which otherwise run under eunit's 5s default; the inner - %% one is the property's own, and its floor is 300s rather than 60s - %% because 60 is a performance budget, not a hang detector - a CI runner - %% managed 21 of 25 iterations of the reconnect property inside it - %% (asobi#376). - {timeout, 120, + %% cleanup/1, which otherwise run under eunit's 5s default; the inner one + %% is the property's own, and its floor is 300s rather than 60s because 60 + %% is a performance budget, not a hang detector (asobi#376). + %% + %% The outer MUST outlast the inner. It covers the whole fixture, tests + %% included, so a shorter outer silently pre-empts the inner and cancels + %% the group - which is exactly what #374 did, moving the failure from one + %% property module to the next. + {timeout, 900, {setup, fun setup/0, fun cleanup/1, fun(Ctx) -> [ {timeout, max(300, ?NUMTESTS div 2), diff --git a/test/prop_zone_invariants.erl b/test/prop_zone_invariants.erl index 96914c05..450256d6 100644 --- a/test/prop_zone_invariants.erl +++ b/test/prop_zone_invariants.erl @@ -34,12 +34,15 @@ zone_invariants_test_() -> %% Two timeouts, because there are two ways this cancels a group with %% zero failures and nothing named. The outer one covers setup/0 and - %% cleanup/1, which otherwise run under eunit's 5s default; the inner - %% one is the property's own, and its floor is 300s rather than 60s - %% because 60 is a performance budget, not a hang detector - a CI runner - %% managed 21 of 25 iterations of the reconnect property inside it - %% (asobi#376). - {timeout, 120, + %% cleanup/1, which otherwise run under eunit's 5s default; the inner one + %% is the property's own, and its floor is 300s rather than 60s because 60 + %% is a performance budget, not a hang detector (asobi#376). + %% + %% The outer MUST outlast the inner. It covers the whole fixture, tests + %% included, so a shorter outer silently pre-empts the inner and cancels + %% the group - which is exactly what #374 did, moving the failure from one + %% property module to the next. + {timeout, 900, {setup, fun setup/0, fun cleanup/1, fun(Ctx) -> [ {timeout, max(300, ?NUMTESTS div 2), From 169acaef3a07d1434cb6b6a4719cd70f64cc14e1 Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Wed, 5 Aug 2026 07:17:54 +0200 Subject: [PATCH 2/2] test: clear a leaked mock before creating one Every property fixture creates its asobi_repo and asobi_presence mocks with `no_link`, so they outlive the process that created them. One left behind by an earlier module makes the next module's meck:new/2 raise `already_started` - a fixture that fails in milliseconds, which eunit reports as a cancelled group with `undefined` for a name and {shutdown, timeout} for a reason. That is the shape of what is left of #376 after the timeout fixes: on the last CI run prop_reconnect_lifecycle passed in 1.23s where it had never passed, and the cancellation moved to prop_zone_invariants - 99ms after its module header, which no timeout in the tree is short enough to explain. Unloading first is right whether or not it is the cause: a fixture that cannot run twice in one VM is a fixture that depends on every other module's cleanup having succeeded. --- test/prop_chat_zone_routing.erl | 16 ++++++++++++++++ test/prop_input_never_dropped.erl | 16 ++++++++++++++++ test/prop_reconnect_lifecycle.erl | 16 ++++++++++++++++ test/prop_zone_invariants.erl | 16 ++++++++++++++++ 4 files changed, 64 insertions(+) diff --git a/test/prop_chat_zone_routing.erl b/test/prop_chat_zone_routing.erl index bfc3b6fe..7ad836c8 100644 --- a/test/prop_chat_zone_routing.erl +++ b/test/prop_chat_zone_routing.erl @@ -51,6 +51,11 @@ setup() -> _ -> ok end, + %% `no_link` mocks outlive the process that created them, so one left + %% behind by an earlier module makes this meck:new/2 raise + %% `already_started` - a fixture that fails in milliseconds, which eunit + %% reports as a cancelled group with nothing named. Clear first. + unload_stale([asobi_repo, asobi_presence]), meck:new(asobi_repo, [no_link]), meck:expect(asobi_repo, insert, fun(_CS) -> {ok, #{}} end), ok. @@ -250,3 +255,14 @@ cleanup_listeners(Listeners) -> -spec narrow_list(term()) -> [term()]. narrow_list(L) when is_list(L) -> L. + +unload_stale(Modules) -> + [ + try + meck:unload(M) + catch + _:_ -> ok + end + || M <- Modules + ], + ok. diff --git a/test/prop_input_never_dropped.erl b/test/prop_input_never_dropped.erl index d98f9e8e..883022df 100644 --- a/test/prop_input_never_dropped.erl +++ b/test/prop_input_never_dropped.erl @@ -64,6 +64,11 @@ setup() -> _ -> ets:delete_all_objects(asobi_player_worlds) end, + %% `no_link` mocks outlive the process that created them, so one left + %% behind by an earlier module makes this meck:new/2 raise + %% `already_started` - a fixture that fails in milliseconds, which eunit + %% reports as a cancelled group with nothing named. Clear first. + unload_stale([asobi_repo, asobi_presence]), meck:new(asobi_repo, [no_link]), meck:expect(asobi_repo, insert, fun(_CS) -> {ok, #{}} end), meck:expect(asobi_repo, insert, fun(_CS, _Opts) -> {ok, #{}} end), @@ -210,3 +215,14 @@ start_world() -> timer:sleep(40), ServerPid = asobi_world_instance:get_child(InstancePid, asobi_world_server), #{instance_pid => InstancePid, world_pid => ServerPid}. + +unload_stale(Modules) -> + [ + try + meck:unload(M) + catch + _:_ -> ok + end + || M <- Modules + ], + ok. diff --git a/test/prop_reconnect_lifecycle.erl b/test/prop_reconnect_lifecycle.erl index 9b066266..a6d9f883 100644 --- a/test/prop_reconnect_lifecycle.erl +++ b/test/prop_reconnect_lifecycle.erl @@ -66,6 +66,11 @@ setup() -> _ -> ets:delete_all_objects(asobi_player_worlds) end, + %% `no_link` mocks outlive the process that created them, so one left + %% behind by an earlier module makes this meck:new/2 raise + %% `already_started` - a fixture that fails in milliseconds, which eunit + %% reports as a cancelled group with nothing named. Clear first. + unload_stale([asobi_repo, asobi_presence]), meck:new(asobi_repo, [no_link]), meck:expect(asobi_repo, insert, fun(_CS) -> {ok, #{}} end), meck:expect(asobi_repo, insert, fun(_CS, _Opts) -> {ok, #{}} end), @@ -212,3 +217,14 @@ fake_session(PlayerId) -> -spec narrow_list(term()) -> [term()]. narrow_list(L) when is_list(L) -> L. + +unload_stale(Modules) -> + [ + try + meck:unload(M) + catch + _:_ -> ok + end + || M <- Modules + ], + ok. diff --git a/test/prop_zone_invariants.erl b/test/prop_zone_invariants.erl index 450256d6..361a7786 100644 --- a/test/prop_zone_invariants.erl +++ b/test/prop_zone_invariants.erl @@ -69,6 +69,11 @@ setup() -> _ -> ets:delete_all_objects(asobi_player_worlds) end, + %% `no_link` mocks outlive the process that created them, so one left + %% behind by an earlier module makes this meck:new/2 raise + %% `already_started` - a fixture that fails in milliseconds, which eunit + %% reports as a cancelled group with nothing named. Clear first. + unload_stale([asobi_repo, asobi_presence]), meck:new(asobi_repo, [no_link]), meck:expect(asobi_repo, insert, fun(_CS) -> {ok, #{}} end), meck:expect(asobi_repo, insert, fun(_CS, _Opts) -> {ok, #{}} end), @@ -203,3 +208,14 @@ start_world() -> timer:sleep(40), ServerPid = asobi_world_instance:get_child(InstancePid, asobi_world_server), #{instance_pid => InstancePid, world_pid => ServerPid}. + +unload_stale(Modules) -> + [ + try + meck:unload(M) + catch + _:_ -> ok + end + || M <- Modules + ], + ok.