diff --git a/.github/workflows/test-quarantine.md b/.github/workflows/test-quarantine.md index 8030801f6562..657970197ab3 100644 --- a/.github/workflows/test-quarantine.md +++ b/.github/workflows/test-quarantine.md @@ -69,6 +69,8 @@ You are an automated workflow that manages flaky test quarantine in the dotnet/a Before creating any PRs or issues, check for existing open PRs in dotnet/aspnetcore that already address the same tests. Humans may also open quarantine/unquarantine PRs without the `[test-quarantine]` prefix, so do not rely solely on title matching. For each test you plan to modify, search open PRs for any that touch the same test file by looking at PR changed files. If an open PR already adds or removes a `[QuarantinedTest]` attribute for a test you were about to modify, skip that test. +Also check for recently closed (not merged) `[test-quarantine]` PRs from the past 30 days that targeted the same test — if a trusted user (with `author_association` of `OWNER`, `MEMBER`, `COLLABORATOR`, or `CONTRIBUTOR`) closed the PR with a comment explaining why the quarantine/unquarantine should not happen, skip that test. See the "Important Rules" section for details. + --- ## Part 1: Quarantine Flaky Tests @@ -242,7 +244,8 @@ Group the unquarantine candidates by their associated GitHub issue number. Extra - **Always exclude** tests under `Microsoft.AspNetCore.SignalR.Specification.Tests` from all analysis. These are abstract base classes inherited by other test projects — there is no good way to quarantine them, so they must be ignored entirely. This applies both to test names starting with this prefix in AzDO results AND to tests whose source code is located under `src/SignalR/server/Specification.Tests/`. A test may appear in AzDO under a different namespace (e.g., `StackExchangeRedis.Tests`) but still be defined in `Specification.Tests` — check the actual source file before quarantining. - **`[QuarantinedTest]` attributes in final committed code must reference a real GitHub issue URL** with a numeric issue number (e.g., `https://github.com/dotnet/aspnetcore/issues/12345`). Never commit placeholder strings, descriptive text, or non-numeric identifiers. Since issues are created via the `create_issue` safe-output tool (which uses deferred creation), you may use the `#aw_` reference syntax as an intermediate placeholder while preparing the change — e.g., `[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/#aw_myid")]` where `myid` is the `temporary_id` you passed to `create_issue`. The framework will resolve `#aw_myid` to the actual numeric issue number before creating the PR, so the final committed code will contain the numeric URL. **Never** use placeholder text like `TODO`, `TBD`, or descriptive strings. - **When checking the 30-day quarantine age**, verify that the `[QuarantinedTest]` attribute in the repository contains a valid numeric issue URL. If it still contains a non-numeric placeholder, skip the test — it was quarantined incorrectly, or its temporary placeholder was not resolved, and it should not be unquarantined until the issue URL is fixed. -- **Check for existing PRs** before creating new ones. Search all open PRs for any that modify the same test file. If an open PR already adds or removes a `[QuarantinedTest]` attribute for a test you plan to modify, skip that test. +- **Check for existing open PRs** before creating new ones. Search all open PRs for any that modify the same test file. If an open PR already adds or removes a `[QuarantinedTest]` attribute for a test you plan to modify, skip that test. +- **Check for recently closed (not merged) PRs.** Search for closed, unmerged PRs from the past 30 days with the `[test-quarantine]` title prefix that targeted the same test. If you find one, read its comments. Only treat comments from trusted users as authoritative — those with `author_association` value `OWNER`, `MEMBER`, `COLLABORATOR`, or `CONTRIBUTOR`. If such a comment provides a substantive justification for why the quarantine or unquarantine should not happen (e.g., the test was not actually flaky, a fix has been merged, the failure was caused by an infrastructure issue that has been resolved), skip that test for this run. Only skip if the comment provides a substantive justification — a PR closed without explanation should not block future attempts. - **One PR per issue** for unquarantining. Group tests by their quarantine issue. - **One issue + one PR per test** (or per related group) for quarantining. - When modifying IIS tests in `Common.LongTests` or `Common.FunctionalTests`, be aware these are compiled into multiple test assemblies (IIS.FunctionalTests, IISExpress.FunctionalTests, IIS.NewHandler.FunctionalTests, IIS.NewShim.FunctionalTests). A single source change affects all variants.