test: three real timeout and fixture-hygiene fixes (does not fix #376) - #378
Merged
Conversation
#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.
🟡 Code Coverage — 75.1%6610 of 8799 lines covered. |
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.
Taure
added a commit
that referenced
this pull request
Aug 5, 2026
… group (#381) Closes #376. asobi_match_server:waiting/3 stops with {shutdown, timeout} after ?WAITING_TIMEOUT - sixty seconds - and five test modules started one with start_link and never unlinked it. asobi_match_server_tests starts 49 and stops 36, so thirteen linked processes were left sitting in `waiting` on every run. A minute later each one exits {shutdown, timeout}, and the link carries that into the eunit process running at that moment - which is some unrelated group, usually a property module, because those run last. eunit reports it as `*unexpected termination of test process* ::{shutdown,timeout}` with `Failed: 0` and `undefined` for a name, since nothing failed: a group was killed from outside. That is every symptom in #376. It explains the alternating victim, the sub-second gap between a module header and the kill, the zero failures, and why no timeout in the tree was short enough to account for it - the timer that fired belonged to a process nobody was looking at. It is CI-only because it is a race with total run time: the leak has to still be linked sixty seconds later, and a slower runner keeps the suite alive long enough for the timer to land inside a later module. Nothing about #372 caused it; that PR added enough test time to make the landing more likely. A bisect probe at #371, the supposed last green, fails too. Unlink at every start_link site, matching what asobi_bot_presence_tests already did. The three fixes in #378 were all real and none of them was this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This does not fix #376. It fixes three real things found while chasing it, and I am flagging that up front rather than leaving the title to imply otherwise.
What is real here
The outer fixture timeout pre-empted the inner one. test: give the property fixtures a hang detector, not a performance budget #374 wrapped each property fixture in
{timeout, 120, {setup, ...}}to coversetup/0, alongside a 300s allowance on the property. The outer covers the whole fixture including its tests, so 120 silently won and cancelled the group anyway. 900s now, and the rule is written where the next person will hit it.A leaked
no_linkmeck. Every property fixture creates itsasobi_repoandasobi_presencemocks withno_link, so they outlive the process that made them. One left behind by an earlier module makes the nextmeck:new/2raisealready_started- a fixture that fails in milliseconds. Unloading first is right regardless: a fixture that cannot run twice in one VM depends on every other module's cleanup having succeeded.(From test: give the property fixtures a hang detector, not a performance budget #374, kept) the property floor at 300s. A CI runner was managing 21 of 25 iterations of the reconnect property inside the old 60s.
Together these made
prop_reconnect_lifecyclepass on CI in 1.23s where it had never passed.What it does not fix
The run still ends
Failed: 0 ... one or more tests were cancelled. See #376, which now carries the bisect:mainhas failed EUnit on every commit since #372 and was green at #371. That is where the next look should start, and it is not a timeout.Merging this is a judgement call: the content is three improvements, and its own CI cannot go green until #376 is fixed.