Uh oh!
There was an error while loading. Please reload this page.
fix(parallel): key per-test result dirs on the path, not the basename - #961
Merged
Conversation
Under --parallel, each test file's results were bucketed into a directory named
by the file's basename:
local test_suite_base="${test_file##*/}"
local test_suite_dir="${TEMP_DIR_PARALLEL_TEST_SUITE}/${test_suite_base%.sh}"
Two files sharing a basename in different directories therefore shared one
bucket, and the per-suite ordinal is unique per suite rather than globally, so
`<tmp>/foo_test/1.result` was written twice and the first write was lost. The
run stayed green and simply reported fewer tests.
Same defect as #923, which fixed exactly this in build.sh: a basename is not an
identity when directories can repeat one.
Found while mirroring tests/unit onto the src/ module layout (#957/#960), which
produced three duplicate basenames and cost 52 tests under --parallel. That
layout is what the docs encourage, so any user project doing it is exposed.
The derivation now folds the whole path, and both sites that computed it --
runner/result.sh and the pre-create in runner/exec.sh (#813) -- call one shared
helper, so they cannot drift apart. It stays pure parameter expansion: this runs
once per test in every parallel worker and the path must remain fork-free. The
fork-budget acceptance tests pass unchanged.
Nothing parses the directory name; aggregate_parallel_results globs whatever
subdirectories exist.
Closes#959
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 freeto 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.
🤔 Background
Closes#959
Under
--parallel, each test file's results were bucketed into a directory named by the file's basename:Two files sharing a basename in different directories shared one bucket. The per-suite ordinal is unique per suite, not globally, so
<tmp>/foo_test/1.resultwas written twice and the first write was lost. The run stayed green and simply reported fewer tests.Same defect as #923, which fixed exactly this in
build.sh: a basename isn't an identity when directories can repeat one.📉 How it was found, and why it matters to users
Mirroring
tests/unit/onto thesrc/module layout (#957/#960) produced three duplicate basenames and cost 52 tests under--parallel:--parallel --simple --strictA
tests/tree mirroring asrc/tree is what the docs encourage, so any user project doing it is exposed — and a failing test can vanish while CI stays green.💡 Changes
runner/result.shand the pre-create inrunner/exec.sh(perf(runner): stop forking per test when publishing parallel results #813) — now call one shared helper, so they cannot drift apart.Nothing parses the directory name —
aggregate_parallel_resultsglobs whatever subdirectories exist — so the rename is invisible downstream.✅ Verification
TDD: the new acceptance test in
bashunit_parallel_consistency_test.shruns two same-named fixture files from different directories in both modes and compares counts. Verified red without the fix (sequential 2, parallel 1) and green with it.It asserts on extracted counts rather than the rendered summary — the parallel spinner leaves control bytes on that line — plus a guard so both sides can't collapse to empty and match vacuously.
Green: sequential (1630) ·
--parallel --simple --strict(1589) · fork budget ·make sa·make lint·bash build.sh bin -v→✅ Build verified ✅. CHANGELOG updated under Fixed.