Skip to content

Commit 67fad29

Browse files
radicalCopilot
radical
and
Copilot
authored
[CI] Add new github commands for quarantining/disabling tests (dotnet#13558)
* [CI] Add new github commands for quarantining/disabling tests This PR introduces GitHub issue/PR comment commands for managing test attributes: ## New Commands ### Quarantine Tests (flaky tests) - `/quarantine-test <test-name> <issue-url>` - Add [QuarantinedTest] attribute - `/unquarantine-test <test-name>` - Remove [QuarantinedTest] attribute ### Disable Tests (ActiveIssue) - `/disable-test <test-name> <issue-url>` - Add [ActiveIssue] attribute - `/enable-test <test-name>` - Remove [ActiveIssue] attribute ## Usage Commands can be used on: - **Issues**: Creates a new PR with the changes - **Pull Requests**: Pushes changes directly to the PR branch - **With --target-pr**: Pushes to a specified PR regardless of where command is posted ## Examples ``` /quarantine-test Namespace.Class.TestMethod dotnet#1234 /disable-test Namespace.Class.FlakyTest dotnet#5678 /unquarantine-test Namespace.Class.TestMethod /enable-test Namespace.Class.TestMethod ``` ## Implementation Details - Uses QuarantineTools from tools/QuarantineTools for source modifications - Supports both quarantine mode ([QuarantinedTest]) and activeissue mode ([ActiveIssue]) - Validates user has write access before executing - Posts progress comments and links to resulting PR * Update README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update apply-test-attributes.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .github/workflows/apply-test-attributes.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix shell injection vulnerabilities and null check in workflow - Replace execSync with spawnSync for git commands to prevent shell injection - Add runGit() helper that passes args as array (no shell interpretation) - Add null check for pr.head.repo when detecting fork PRs - Addresses Copilot review feedback on PR dotnet#13558 * Pin actions/github-script to SHA for security best practices * Replace execSync with spawnSync for git config commands For consistency with the rest of the script and to follow safer practices, use the runGit helper (which uses spawnSync) for git config commands instead of execSync. This avoids shell injection vulnerabilities as a defense-in-depth measure, even though the current hardcoded strings were safe. * Use os.tmpdir() fallback for cross-platform temp directory compatibility * Update ci.yml * add zizmor ignore * Use runId instead of timestamp for branch name uniqueness * Add to copilot instructions --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 7d68940 commit 67fad29

File tree

4 files changed

+866
-3
lines changed

4 files changed

+866
-3
lines changed

‎.github/copilot-instructions.md‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,74 @@ These switches can be repeated to run tests on multiple classes or methods at on
165165

166166
Example: `[QuarantinedTest("..issue url..")]`
167167

168-
- To quarantine or unquarantine tests, use the tool in `tools/QuarantineTools/QuarantineTools.csproj`.
168+
### Quarantine/Unquarantine via GitHub Commands (Preferred)
169+
170+
Use these commands in any issue or PR comment. They require write access to the repository.
171+
172+
```bash
173+
# Quarantine a flaky test (creates a new PR)
174+
/quarantine-test Namespace.Type.Method https://github.com/dotnet/aspire/issues/1234
175+
176+
# Quarantine multiple tests at once
177+
/quarantine-test TestMethod1 TestMethod2 https://github.com/dotnet/aspire/issues/1234
178+
179+
# Quarantine and push to an existing PR
180+
/quarantine-test TestMethod https://github.com/dotnet/aspire/issues/1234 --target-pr https://github.com/dotnet/aspire/pull/5678
181+
182+
# Unquarantine a test (creates a new PR)
183+
/unquarantine-test Namespace.Type.Method
184+
185+
# Unquarantine and push to an existing PR
186+
/unquarantine-test TestMethod --target-pr https://github.com/dotnet/aspire/pull/5678
187+
```
188+
189+
When you comment on a PR, the changes are automatically pushed to that PR's branch (no need for `--target-pr`).
190+
191+
### Quarantine/Unquarantine via Local Tool
192+
193+
For local development, use the QuarantineTools directly:
194+
195+
```bash
196+
# Quarantine a test
197+
dotnet run --project tools/QuarantineTools -- -q -i https://github.com/dotnet/aspire/issues/1234 Full.Namespace.Type.Method
198+
199+
# Unquarantine a test
200+
dotnet run --project tools/QuarantineTools -- -u Full.Namespace.Type.Method
201+
```
202+
203+
## Disabled tests (ActiveIssue)
204+
205+
- Tests that consistently fail due to a known bug or infrastructure issue are marked with the `ActiveIssue` attribute.
206+
- These tests are completely skipped until the underlying issue is resolved.
207+
- Use this for tests that are **blocked**, not for flaky tests (use `QuarantinedTest` for flaky tests).
208+
209+
Example: `[ActiveIssue("https://github.com/dotnet/aspire/issues/1234")]`
210+
211+
### Disable/Enable via GitHub Commands (Preferred)
212+
213+
```bash
214+
# Disable a test due to an active issue (creates a new PR)
215+
/disable-test Namespace.Type.Method https://github.com/dotnet/aspire/issues/1234
216+
217+
# Disable and push to an existing PR
218+
/disable-test TestMethod https://github.com/dotnet/aspire/issues/1234 --target-pr https://github.com/dotnet/aspire/pull/5678
219+
220+
# Enable a previously disabled test (creates a new PR)
221+
/enable-test Namespace.Type.Method
222+
223+
# Enable and push to an existing PR
224+
/enable-test TestMethod --target-pr https://github.com/dotnet/aspire/pull/5678
225+
```
226+
227+
### Disable/Enable via Local Tool
228+
229+
```bash
230+
# Disable a test with ActiveIssue
231+
dotnet run --project tools/QuarantineTools -- -q -m activeissue -i https://github.com/dotnet/aspire/issues/1234 Full.Namespace.Type.Method
232+
233+
# Enable a test (remove ActiveIssue)
234+
dotnet run --project tools/QuarantineTools -- -u -m activeissue Full.Namespace.Type.Method
235+
```
169236

170237
## Outerloop tests
171238

‎.github/workflows/README.md‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
# GitHub Workflows
2+
3+
## Quarantine/Disable Test Workflow
4+
5+
The `apply-test-attributes.yml` workflow allows repository maintainers to quarantine, unquarantine, disable, or enable tests directly from issue or PR comments.
6+
7+
### Commands
8+
9+
| Command | Description | Attribute Used |
10+
|---------|-------------|----------------|
11+
| `/quarantine-test` | Mark test(s) as quarantined (flaky) | `[QuarantinedTest]` |
12+
| `/unquarantine-test` | Remove quarantine from test(s) | Removes `[QuarantinedTest]` |
13+
| `/disable-test` | Disable test(s) due to an active issue | `[ActiveIssue]` |
14+
| `/enable-test` | Re-enable previously disabled test(s) | Removes `[ActiveIssue]` |
15+
16+
### Syntax
17+
18+
```
19+
/quarantine-test <test-name(s)> <issue-url> [--target-pr <pr-url>]
20+
/unquarantine-test <test-name(s)> [--target-pr <pr-url>]
21+
/disable-test <test-name(s)> <issue-url> [--target-pr <pr-url>]
22+
/enable-test <test-name(s)> [--target-pr <pr-url>]
23+
```
24+
25+
### Parameters
26+
27+
| Parameter | Required | Description |
28+
|-----------|----------|-------------|
29+
| `<test-name(s)>` | Yes | One or more test method names (space-separated) |
30+
| `<issue-url>` | For quarantine/disable | URL of the GitHub issue tracking the problem |
31+
| `--target-pr <pr-url>` | No | Push changes to an existing PR instead of creating a new one |
32+
33+
### Examples
34+
35+
#### Quarantine a flaky test (creates new PR)
36+
```
37+
/quarantine-test MyTestClass.MyTestMethod https://github.com/dotnet/aspire/issues/1234
38+
```
39+
40+
#### Quarantine multiple tests
41+
```
42+
/quarantine-test TestMethod1 TestMethod2 TestMethod3 https://github.com/dotnet/aspire/issues/1234
43+
```
44+
45+
#### Quarantine a test and push to an existing PR
46+
```
47+
/quarantine-test MyTestMethod https://github.com/dotnet/aspire/issues/1234 --target-pr https://github.com/dotnet/aspire/pull/5678
48+
```
49+
50+
#### Unquarantine a test (creates new PR)
51+
```
52+
/unquarantine-test MyTestClass.MyTestMethod
53+
```
54+
55+
#### Unquarantine and push to an existing PR
56+
```
57+
/unquarantine-test MyTestMethod --target-pr https://github.com/dotnet/aspire/pull/5678
58+
```
59+
60+
#### Disable a test due to an active issue
61+
```
62+
/disable-test MyTestMethod https://github.com/dotnet/aspire/issues/1234
63+
```
64+
65+
#### Enable a previously disabled test
66+
```
67+
/enable-test MyTestMethod
68+
```
69+
70+
#### Comment on a PR to push changes to that PR
71+
When you comment on a PR (not an issue), the workflow will automatically push changes to that PR's branch instead of creating a new PR. You can override this by specifying `--target-pr`.
72+
73+
### Behavior
74+
75+
1. **Permission Check**: Only users with write access to the repository can use these commands.
76+
2. **Processing Indicator**: The workflow adds an 👀 reaction to your comment when it starts processing.
77+
3. **Status Comments**: The workflow posts comments to indicate:
78+
- ⏳ Processing started
79+
- ✅ Success (with link to created/updated PR)
80+
- ℹ️ No changes needed (test already in desired state)
81+
- ❌ Failure (with error details)
82+
83+
### Target PR Behavior
84+
85+
| Context | `--target-pr` specified | Result |
86+
|---------|-------------------------|--------|
87+
| Comment on Issue | No | Creates new PR from `main` |
88+
| Comment on Issue | Yes | Pushes to specified PR |
89+
| Comment on PR | No | Pushes to that PR's branch |
90+
| Comment on PR | Yes | Pushes to specified PR (overrides) |
91+
92+
### Restrictions
93+
94+
- The `--target-pr` URL must be from the same repository
95+
- Cannot push to PRs from forks
96+
- Cannot push to closed PRs
97+
- The PR branch must not be protected in a way that prevents pushes
98+
99+
### Concurrency
100+
101+
The workflow uses concurrency groups based on the issue/PR number to prevent race conditions when multiple commands are issued on the same issue.

0 commit comments

Comments
 (0)