Repository navigation
Fix the Git service test racing every other test that registers services - #349
Merged
Merged
Conversation
…ices Registering RackPeek's services writes process-wide statics as a side effect, RpkConstants.HasGitServices among them. That is harmless in production, where a process registers once, and a race in a test run, where dozens of classes register with different configuration. Three groups were mutating it from three different xUnit collections — the Git tests, the YAML CLI host, and the API tests — and collections run in parallel with each other, so whichever ran last won. Git_Token_Set_Registers_... asserted the flag was true and read whatever an API test had just set, failing roughly one full-suite run in three while passing in isolation every time. They now share one collection with parallelisation off, named for the shared state rather than for the YAML CLI, because membership is decided by "does this touch the statics" rather than by what the test nominally exercises. Six consecutive full runs green, against a baseline that failed one run in two. Costs about three seconds on a fifteen-second suite. Co-Authored-By: Claude Opus 5 (1M context) <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.
GitConfigurationTests.Git_Token_Set_Registers_LibGit2GitRepository_And_Flips_HasGitServicesfails roughly one full-suite run in three, and passes in isolation every time.Cause
Registering RackPeek's services writes process-wide statics as a side effect —
RpkConstants.HasGitServicesamong them. Harmless in production, where a process registers once. A race in a test run, where dozens of classes register with different configuration.Three groups mutate it, from three different xUnit collections:
Tests/Git/GitConfigurationTests.cs"Git static state"Tests/EndToEnd/Infra/YamlCliTestHost.cs"Yaml CLI tests"(parallelisation already off)Tests/Api/ApiTestBase.csCollections run in parallel with each other, so whichever registered last won. The Git test asserts the flag is
trueand would read whatever an API test had set microseconds earlier.Fix
All three now share one collection with parallelisation off. It's named for the shared state rather than for the YAML CLI, because membership is decided by "does this touch the statics", not by what the test nominally exercises.
Evidence
Worth doing before a release, since the release guide's first gate is "staging CI is green" — a one-in-three flake makes that gate unreliable exactly when it matters most.
🤖 Generated with Claude Code