Fix test-quarantine unquarantine matching and re-quarantine detection#67356
Merged
wtgodbe merged 2 commits intoJun 21, 2026
Merged
Conversation
The test-quarantine workflow (run 27903471243) produced two classes of incorrect output, confirmed against live AzDO/Helix pipeline data: 1. Bad unquarantines (PRs dotnet#67354, dotnet#67355). The agent matched tests by bare leaf method name instead of fully-qualified name, so it borrowed another test's pass statistics: - dotnet#67355 unquarantined HelloWorldTests.HelloWorld using pass-stats from the unrelated StartupTests.HelloWorld. - dotnet#67354 unquarantined the subclass override ServerExecutionTests.ServerRoutingTest.NavigationLock_... using the passing base class Tests.RoutingTest.NavigationLock_..., even though the actual quarantined subclass was failing on main. 2. Missed re-quarantine (PR dotnet#67349 / issue dotnet#67348). TestServerTests. LongPollingWorks was re-quarantined as a brand-new Case A quarantine (new issue) instead of a Case B re-quarantine reusing original issue dotnet#65702, because (a) Case B only triggered within a 14-day window and this test was unquarantined 62 days earlier, and (b) the original issue is titled/labeled "Known Build Error", which a title search for a "Quarantine ..."-prefixed issue would miss. Fixes in test-quarantine.md: - Unquarantine matching: group AzDO rows by exact full automatedTestName; compute pass rate and require real passing run evidence only from rows whose fully-qualified identity (namespace + concrete declaring class + method) matches the attribute in source, with explicit handling for subclass overrides, inherited base methods, parameterized/theory rows, and the IIS multi-assembly prefix exception. Fail closed on zero rows. - Re-quarantine (Case B): detect previously-unquarantined tests per-test (not per-file) by walking the file's QuarantinedTest history newest to oldest and stopping at the most recent commit that changes this exact test's attribute; removed the 14-day window; evaluate Case B before Case A (with an explicit "does not match Case B" gate on Case A); derive the original tracking issue from the issue URL removed by the unquarantine commit (reusable regardless of label, open or closed); fall back to Case A when no valid numeric issue URL is present. Recompiled test-quarantine.lock.yml with gh aw v0.79.8. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Daily Test Quarantine Management workflow’s source-of-truth documentation to prevent incorrect unquarantine matching (by leaf method name) and to correctly detect/handle re-quarantines (Case B) by reusing the original tracking issue regardless of how long ago the unquarantine happened.
Changes:
- Tighten unquarantine guidance to match/aggregate using fully-qualified identities (and to fail closed on “no run evidence”) instead of leaf method names.
- Rework Case B guidance to be per-test (not per-file), remove the 14-day window, and derive the original tracking issue from the unquarantine commit diff.
- Regenerate
test-quarantine.lock.ymlviagh aw compileto reflect the updated markdown body.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/test-quarantine.md |
Updates the workflow instructions for correct fully-qualified test matching and robust Case B re-quarantine detection/issue reuse. |
.github/workflows/test-quarantine.lock.yml |
Regenerated compiled workflow (metadata/body hash update) to reflect the updated .md. |
…ping, set-based run evidence Responds to the automated review on dotnet#67356: - Case B history walk and the 60-day attribute-age check now pass `--follow` so renamed/moved test files keep their full quarantine history. The walk uses `git log --follow -p` to read each commit's patch inline (with the file's historical path), instead of a separate `git show <sha> -- <current path>` that returns an empty diff for any commit predating a rename. The original-issue lookup reads the same `-p` patch. - Step 2.1 aggregation groups by the exact, full `automatedTestName` (the verbatim AzDO string, including theory/parameter arguments), deferring `{DeclaringClass}.{Method}` normalization to Step 2.3 — this removes the description inconsistency between the two steps. - The unquarantine "real run evidence" gate now evaluates the set of rows that resolve to the test's source identity (matching {DeclaringClass}.{Method}, including allowed IIS multi-assembly variants and inherited concrete-subclass rows) rather than a single exact name. A 0-pass/0-fail variant (e.g. a [ConditionalFact]-skipped IIS variant) no longer disqualifies a candidate as long as at least one row in the set passed and none failed; fail closed only when the whole resolved set is 0 pass / 0 fail. Step 2.3's "must pass" wording is softened to "zero failures" to stay consistent. Recompiled test-quarantine.lock.yml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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.
Problem
The
test-quarantineworkflow run 27903471243 produced two classes of incorrect output, each confirmed against live AzDO/Helix pipeline data.1. Bad unquarantines — matched by leaf method name
The agent matched tests by bare leaf method name instead of fully-qualified name, so it borrowed an unrelated test's pass statistics:
HelloWorldTests.HelloWorldusing pass-stats from the unrelatedStartupTests.HelloWorld(the target test had zero run evidence in the cited pipeline).ServerExecutionTests.ServerRoutingTest.NavigationLock_...using the passing base classTests.RoutingTest.NavigationLock_..., even though the actual quarantined subclass was failing on main (builds 1473858, 1473100).Both PRs were closed.
2. Missed re-quarantine —
LongPollingWorkstreated as newTestServerTests.LongPollingWorkswas re-quarantined as a brand-new Case A quarantine (new issue #67348 via #67349) instead of a Case B re-quarantine reusing the original tracking issue #65702, because:"Quarantine ..."-titled issue, but [Known Build Error] TestServerTests.LongPollingWorks - ObjectDisposedException in OnDisconnectedAsync during shutdown #65702 is titled/labeledKnown Build Error, so a title search missed it.Fixes (all in
test-quarantine.md)Unquarantine matching:
automatedTestName.[QuarantinedTest]attribute in source; fail closed on zero rows.Re-quarantine (Case B):
QuarantinedTesthistory newest→oldest and stop at the most recent commit changing this exact test's attribute (a single file holds many tests).test-failureorKnown Build Error), open or closed.The diff was reviewed by two independent models to convergence (both APPROVE).
Recompiled
test-quarantine.lock.ymlwithgh awv0.79.8.