Don't let integration tests race a background git repack - #5898
Merged
Conversation
Every `git commit` forks `git maintenance run --auto --quiet --detach`,
and git 2.54 changed what that runs from the `gc` task to the
"geometric" strategy. The geometric repack's auto condition passes its
threshold of 100 to too_many_loose_objects(), which estimates the loose
object count from the objects/17 fanout directory times 256, so the real
trigger is two objects in that one directory -- where the old gc task
needed 27. Fixture repos reach two easily: every CreateNCommits(n>=6)
repo already stores the blob for file06.txt there, so a single commit
object hashing into 17 (about 4% of fixtures with 10 commits, 15% with
40) tips it over, and from then on every commit in that repo forks a
detached `git repack -d`, which prunes loose objects while the next
fixture command -- or lazygit under test -- is still working in the same
repo.
That is where the CI panics during fixture setup come from:
panic: error running command: [git commit -m commit-10]
error: invalid object 100644 50d5612... for 'file09.txt'
error: Error building trees
The reported hashes are exactly the fixture blobs, so `git add` staged
them correctly; they were unlinked underneath the commit. Only the
"git latest" jobs saw this, since the pinned 2.32/2.38/2.44 jobs predate
the strategy change.
git's own test suite guards against the same thing by exporting
GIT_TEST_MAINT_AUTO_DETACH=false ("Ensure that tests cannot race with
background maintenance by default"). Turning maintenance off outright is
stronger: no test repo needs it, and it also spares us a forked git
process per commit. maintenance.auto has been honored since git 2.29, so
it covers every version in the CI matrix.
Measured on a 40-commit fixture: a background repack fired in 4 of 25
runs before, 0 of 25 after.
A failing setup step called Shell.fail, which panicked. Tests run as parallel subtests, so that panic aborted the entire test binary: one bad fixture cost us the results of all ~500 tests, and the failure was reported as a stack trace rather than against the test that caused it. Keep panicking to skip the remaining setup steps -- they would only produce follow-on failures -- but recover in createFixture and return the message as that test's error. All three clients already propagate an error from a test, so they report it the way they report any other failure.
stefanhaller
added
the
maintenance
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.
This fixes spurious test failures caused by git's auto maintenance running concurrently with a test's fixture setup; see the first commit's message for details.
In addition, we change the test harness so that such a failure in a setup method doesn't take down the whole test binary.