fix(test): serialise env-mutating tests behind one lock - #178
Merged
Conversation
`demux::models::cache_dir_precedence` fails intermittently under
`cargo test --all-features`, asserting on a path belonging to a different test:
left: "/tmp/.tmpjgdEfz" right: "/x/cache"
Cargo runs tests in parallel threads and the environment is per-process, not
per-thread, so one test's `set_var` lands inside another's assertion. Both
`temp_env` helpers carried `// SAFETY: single-threaded test bodies`, which is
not true of a parallel runner — the comment asserted the invariant rather than
establishing it.
resquiggle_models' copy did take a mutex, but a module-local one, so it
serialised against itself and not against demux::models — which is the pair
that actually collides.
One shared `test_env::temp_env` behind a process-wide lock now, used by both.
That makes the SAFETY claim true rather than aspirational, and removes the
duplicate helper.
Verified: 10/10 consecutive `--all-features` runs clean (was reproducible
within a few), 58 tests, and the same passing serially.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Found while verifying #177 — pre-existing and unrelated to it, so separated.
demux::models::cache_dir_precedencefails intermittently undercargo test --all-features, asserting on a path that belongs to a different test:Cargo runs tests in parallel threads; the environment is per-process, not per-thread. One test's
set_varlands inside another's assertion.Both
temp_envhelpers carried// SAFETY: single-threaded test bodies— which is not true of a parallel test runner. The comment asserted the invariant instead of establishing it.resquiggle_modelsdid take a mutex, but a module-local one, so it serialised against itself and not againstdemux::models— which is the pair that actually collides.One shared
test_env::temp_envbehind a process-wide lock now, used by both. That makes the SAFETY claim true rather than aspirational, and drops the duplicated helper.Verified: 10/10 consecutive
--all-featuresruns clean (it was reproducible within a few before), 58 tests, still passing serially, fmt and clippy clean.🤖 Generated with Claude Code