Repository navigation
CI quarantine list: a test dev's latest push failed is skipped on PRs for 72 hours, never a test the PR touches (#5942) - #5964
Merged
Conversation
… for 72 hours, never a test the PR touches (#5942)
erikdarlingdata
marked this pull request as ready for review
October 16, 2026 19:02
This was referenced Oct 16, 2026
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.
Fixes #5942.
Claude posting for Erik Darling.
The smallest useful version from the adversarial review: a quarantine file that sets aside a test dev's own latest push broke, only on a pull request that does not reach that test, for at most 72 hours. The file ships EMPTY, so merging this changes no CI result.
The file:
.github/ci-quarantine.txtOne test per line, comments start with
#:Darling.Tests.X.Y [FAIL]). A theory case's arguments are left off and the entry leaves out every case of the method. It must start withDarling.Tests.orLite.Tests.. An entry is one method; a whole class is one line per failing method.When an entry is honored (
ci-select.py quarantine)All of these must hold, or the test runs (every doubt fails open):
build.yml(a cancelled or skipped run is passed over) failed this exact test. The check isGET /repos/{repo}/actions/workflows/build.yml/runs?branch=dev&event=push&status=completed, then that run's failed jobs, then each failed job's log, taking the<time>Z Name [FAIL]lines. If that run succeeded, nothing is valid. If any API call fails, the run list or job list is unreadable, or more than 30 jobs failed, the answer is "unknown" and NO entry is honored (fail open: the test runs). The entry's own run id is for humans; it is not checked.map-select'swhy) is usable (not FULL, not missing) and gives the test's class none of these reasons:own file changed,covers a changed file,text pattern,names a changed symbol,shares a collection with a changed class. A FULL or missing selection honors nothing.Decision for the coordinator to check:
reads the tree,guard stage,new classandgated on an environmentare NOT counted as reach. A census test reads the whole tree, so counting it would make an entry useless for exactly the break (#5845's census) that cost the most. The cost: a PR that itself adds a violation a quarantined census would catch is not stopped by that census until the entry goes; the dev push after it still runs the test.The decision prints
::notice title=CI quarantine honored/ignored::annotations, one per entry with the reason, and a line in the job summary.Where it runs (scope: the Guard job and the two shard runners)
test-map-select(pull requests, already has the checkout, the map selection andactions: read) runsci-select.py quarantineonce and uploadsquarantine.jsoninside the existingtest-map-selectionartifact. The step iscontinue-on-error, bounded to 150 seconds, and calls the API only when the file has an entry (it ships empty, so there is no API call and no wait today).run-darling-pg-shard.ps1andrun-lite-shard.ps1readquarantine.jsonbeside their-SelectionFile(no new parameter, no YAML change in the shard jobs) and add-method- <test>to every chunk's runner command. I confirmed on a builtLite.Tests.exethat-class X -method- X.Mruns the class minus that method (23 tests became 22). Note: thebuildjob's no-store Darling pass calls the Darling runner script with its selection file too, so it honors the decision as well.guard-tests) getsactions: readand one new step that fetchestest-map-selectiononly when the quarantine file has an entry, retrying up to 90 seconds (it runs in parallel with the Guard build, and a missing decision just means nothing is left out). The test run adds@quarantineArgsafter-failSkips. I did NOT addtest-map-selectto the job'sneeds: that would change itsif(a skipped need skips the job on a push) and the workflow pins aroundneeds: [gate, guard-build]..github/REVIEW-TRAPS.mdAdded under "Workflows and CI": "Our wrong test is a bug we introduced, never a flake: a test that dev's own push broke ... failed ten unrelated PRs in a week (#5942). It gets an issue and a fix, and a line in
.github/ci-quarantine.txtonly while that fix lands:<Namespace.Class.Method> | <dev push run id> | <UTC time> | #<issue>, honored for 72 hours, only while the latest completed dev push run failed that exact test, and never on a PR that touches the test's file or one the test map says reaches it." The format is in that line and in the file header.Tests
New
Lite.Tests/CiSelectQuarantineGuardTests.cs(14 cases, behaviour only, no source-text reading), drivingci-select.pyon fixture files and a fixed clock:-method- <test>(and only for its own suite);reads the treeandguard stagestay quarantined;map-selectchained intoquarantine: Lite/B.cs (unrelated) honors the entry, Lite/A.cs (covered) and the test's own file run it, so a rename of a selection reason cannot turn the reach check off unseen;[FAIL]format, checked also by hand against the real log of dev run 38903116863 (job log API).Ran (Debug build, 0 warnings):
Lite.Testsclasses*CiSelect* *Census* *Ratchet* *TestMap* *GuardStage* *Workflow*: Total 130, Failed 0;CiSelectQuarantineGuardTestsalone: Total 14, Failed 0.Darling.Testsclasses*GuardStage* *Census* *Ratchet* *Workflow*: Total 526, Failed 0, Skipped 1. Workflow YAML parses (yaml.safe_load); both runner scripts parse with the PowerShell parser. A live call ofdev_failures()against the repo returned the latest completed dev push run (success, no failures).Not run: the workflow itself in Actions (the file is empty, so the first real use is the first entry), the full suite (CI decides), a nested-class test name through
-method-(an entry copies the log's spelling; I did not verify the runner acceptsOuter+Innerthere).Rebase
Dev had not moved when I last fetched; the siblings' edits (#5944, #5955/#5946, #5947) are not in. My edits are a new function block and two subcommands in
ci-select.py, one new step plus one artifact path intest-map-select, one new step and one run-line edit inguard-tests, and one block plus@quarantineArgsin each runner script.CHANGELOG entry
SECTION: Added
ENTRY:
.github/ci-quarantine.txtlists such a test with its dev run, time and issue; the Guard job and the shard runners leave it out only while the latest dev push run failed it and the pull request does not touch it. The file ships empty.REF:
[CI: a quarantine list, so a test that breaks dev stops failing unrelated PRs while its fix is made #5942]: CI: a quarantine list, so a test that breaks dev stops failing unrelated PRs while its fix is made #5942