test: stop a background writer failing teardown's rm - #23
Merged
Merged
Conversation
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.
What this changes
tests/compose.batshas been failing on CI inteardown, not in anyassertion:
rm -rfreports "Directory not empty" when an entry appears after it hasalready walked the directory β something is still writing into the generated
project while the test that generated it is being torn down.
compose.batsgenerates a project per test and the lane runs
--jobs $(nproc), so on afour-core runner several laravel generations overlap.
It is a race, not a defect in any test: a different test loses it each run.
Across five runs of the same job it took down
{12, 19, 26}, then{19}, then{20}.It is also not new, and not this branch's. Measured by re-running the same job
on the same commits:
mainfails one run in three, and everynot okis thissame teardown.
Two changes:
git config --global gc.auto 0in the suite's own git configuration.scaffold newcommits what it creates, and a commit can hand the repositoryto a detached
git gcthat outlives the command and keeps writing into.git. That is the likeliest writer this races. It was never caught in theact, so this is the suspect removed rather than the cause proven β a
throwaway repository has nothing worth maintaining either way.
remove_workdirreplaces the barerm -rf "$WORKDIR"in all thirteenteardowns. It retries for five seconds, and if the tree is genuinely stuck it
still fails the test and prints what is left in it β so the next occurrence
arrives as a diagnosis instead of another mystery.
No test's assertions change.
How it was verified
bats tests/compose.batsβ 17/17, the suite the failure lands inmise run test-unitβ no failuresmise run test-runnerβ both lanesshellcheckontests/helpers/setup.bashβ clean for the added functionThe retry cannot be proven to fire from here: the race has never reproduced
locally, across a full
test-runner, three repeats of the individual test thatfailed, and a probe that looked for processes holding the generated tree the
instant
scaffold newreturned. That is why the failure path reports ratherthan just giving up.
Checklist
mise run lintpassesmise run test-runnerpassesunticked rather than claimed: this changes test infrastructure, and the
race it removes has never reproduced outside CI, so there is nothing to
write a failing test against