Skip to content

[ready to merge] docs(flaky-tests): document Never Quarantine context menu in tests table - #597

Merged
trunk-staging-io[bot] merged 5 commits into
mainfrom
sam-gutentag/never-quarantine-context-menu
May 7, 2026
Merged

[ready to merge] docs(flaky-tests): document Never Quarantine context menu in tests table#597
trunk-staging-io[bot] merged 5 commits into
mainfrom
sam-gutentag/never-quarantine-context-menu

Conversation

@samgutentag

Copy link
Copy Markdown
Contributor

Summary

Documents the new context menu actions in the Flaky Tests table that allow admins to set Quarantine, Unquarantine, Never Quarantine, and Remove Never Quarantine overrides directly from the table without opening the test details page. Also clarifies admin-only permission gating.

Source

Test plan

  • Preview in GitBook

Generated by Claude Code

…es in table

Adds docs for the new context menu in the Flaky Tests table that lets
admins quarantine, unquarantine, set Never Quarantine, or remove Never
Quarantine directly from a test row without navigating to the detail page.

Source: trunk-io/trunk2#3902

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@trunk-staging-io

trunk-staging-io Bot commented May 6, 2026

Copy link
Copy Markdown

😎 Merged successfully - details.

@claude

claude Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @samgutentag's task in 1m 45s —— View job


Claude Code Review

  • Read the changed file (flaky-tests/quarantining.md)
  • Compare against origin/main
  • Check related context (CLAUDE.md, related docs)
  • Provide review feedback

Summary

The PR is clear, well-structured, and uses the same bold-paragraph sub-section pattern already established in this file (e.g., **Using Flaky Tests as a single step**). No typos or grammar issues. The new content correctly positions the context menu as an alternative entry point and preserves the existing "When a manual override is active, a banner shows who set it and when." flow.

Inline comments left (all minor, non-blocking):

  1. Line 204 — terminology consistency: "suppressed" isn't used elsewhere in this file, and "auto-quarantine is on" mismatches the file's existing "auto-quarantining is enabled" phrasing (line 215).
  2. Line 206 — "Both actions" is slightly ambiguous when the bullets above list four menu labels (two toggle pairs). Suggested clearer phrasing.
  3. Line 195 — optional: the existing figure only shows the test details page. A screenshot of the right-click menu would help readers discover the new flow visually. Not a blocker.

No blocking issues. Suggestions are stylistic.
· Branch

Comment thread flaky-tests/quarantining.md Outdated
Comment thread flaky-tests/quarantining.md Outdated
@@ -194,7 +194,18 @@ If you have tests that should never be quarantined or should always be quarantin

<figure><img src="../.gitbook/assets/qurantine-individual-tests.png" alt=""><figcaption><p>overriding</p></figcaption></figure>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: the figure here predates the new context-menu entry point and shows the test details page only. If you have a screenshot of the new right-click menu, adding it under the From the Flaky Tests table section would make the new flow visually discoverable for readers who skim. Not a blocker.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: Claude code review passed.

@claude

claude Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @samgutentag's task in 1m 4s —— View job


Claude Code Review

  • Read changed file (flaky-tests/quarantining.md)
  • Compare against origin/main
  • Verify prior review feedback addressed
  • Post review

Summary

This synchronize was triggered by a main → branch merge (commit a8d3185); the PR's content commit (41a3143) is unchanged since the prior review. The doc changes themselves still look good — clear structure, consistent with the file's existing bold-paragraph sub-section pattern, no typos or grammar issues.

Status of prior review comments

The three suggestions from the previous review are non-blocking but still open on this revision:

  1. flaky-tests/quarantining.md:220 — "failures are never suppressed" / "auto-quarantine is on" doesn't match the file's existing terminology ("quarantined", "auto-quarantining is enabled for the repo" — see line 231). Fix this →

  2. flaky-tests/quarantining.md:222 — "Both actions" is ambiguous since the bullets above list four menu labels (two toggle pairs). Consider "All four options" or similar. Fix this →

  3. flaky-tests/quarantining.md:211 — Optional: a screenshot of the right-click menu under From the Flaky Tests table would help readers discover the new flow visually. The existing figure only shows the test details page. Not a blocker.

No blocking issues. ✅
· Branch

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: Claude code review passed.

@samgutentag

samgutentag commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Verification status (2026-05-07, re-verified): live

Verified: customers can use this. Ready to publish.

  • Eng PR: trunk-io/trunk2#3902 (merged 2026-05-05)
  • Flag: none (no LaunchDarkly references in PR diff)
  • Signals:
    • Merge commit on main (status: behind, merge intact)
    • No feature flag patterns in the PR diff
    • UI labels confirmed verbatim: "Quarantine test", "Unquarantine test", "Never Quarantine test", "Remove Never Quarantine"
    • Admin gate tooltip confirmed verbatim: "Only admins can set manual quarantine"

Branch is up to date with main. Ready to publish.

samgutentag and others added 2 commits May 7, 2026 10:30
Per review feedback, two consistency nits:

- Replaced "failures are never suppressed" with "test is never
  quarantined" to match the surrounding terminology in this file
  (line 231 already uses this phrasing).
- Replaced "auto-quarantine is on" with "auto-quarantining is enabled
  for the repo" to match line 215 / 231.
- Replaced "Both actions require admin permissions" with
  "All four options require admin permissions" since the two
  toggle bullets together describe four distinct menu items.

Also adds (?i)unquarantine to the vale terms vocabulary so
"Unquarantine test" passes Trunk Check (the dictionary already
included `unquarantined` but not the bare verb form).

Verified the doc claims against trunk2#3902:
- Menu labels: "Quarantine test", "Unquarantine test",
  "Never Quarantine test", "Remove Never Quarantine"
- Tooltip: "Only admins can set manual quarantine"
- Disabled state: gated on `isQuarantineRestricted`
All match the doc verbatim.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @samgutentag's task in 1m 31s —— View job


Claude Code Review

  • Read changed files (flaky-tests/quarantining.md, vocab accept.txt)
  • Compare against origin/main
  • Verify status of prior review feedback
  • Post review

Summary

Both blocking-adjacent suggestions from the prior review are now resolved in commit abc7c8f:

  1. flaky-tests/quarantining.md:220 ✅ — Now reads "the test is never quarantined, even if auto-quarantining is enabled for the repo." Matches the file's existing terminology (line 231 uses the same wording).
  2. flaky-tests/quarantining.md:222 ✅ — Now reads "All four options require admin permissions. Non-admin users see them as disabled..." No longer ambiguous.

The vocabulary addition ((?i)unquarantine in accept.txt:244) is appropriate — "Unquarantine test" is a verbatim UI label, and the dictionary already contained unquarantined but not the bare verb form. Placement is alphabetically correct.

Minor remaining note (non-blocking)

flaky-tests/quarantining.md:217 says "Two quarantine actions are available:" while line 222 says "All four options require admin permissions." The numeric mismatch (two toggle pairs vs. four menu labels) is logically consistent but reads as a slight tension. If you want perfect symmetry, "Two toggle pairs" or "Four menu options" on line 217 would harmonize with line 222 — but this is stylistic, not a blocker.

The optional screenshot of the right-click menu (mentioned in the prior review) is still not added; that remains optional.

No blocking issues. ✅
· Branch

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: Claude code review passed.

@claude

claude Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @samgutentag's task in 3m 10s —— View job


Claude Code Review

  • Read changed files (flaky-tests/quarantining.md, vocab accept.txt)
  • Compare against origin/main
  • Verify status of prior review feedback
  • Post review

Summary

This synchronize event fired with no new content commits since the prior review at 2026-05-07 17:40Z — branch tip is still a62d75b (the main→branch merge), with abc7c8f as the latest content commit. All prior review feedback remains resolved:

  • flaky-tests/quarantining.md:220 ✅ — "the test is never quarantined, even if auto-quarantining is enabled for the repo." matches the file's existing terminology (line 231).
  • flaky-tests/quarantining.md:222 ✅ — "All four options require admin permissions. Non-admin users see them as disabled..." is unambiguous.
  • .trunk/configs/vale/styles/config/vocabularies/terms/accept.txt:244 ✅ — (?i)unquarantine placement is alphabetically correct, and the verb form is needed since the file already had unquarantined but not the bare verb that the new "Unquarantine test" UI label requires.

Re-verification of UI claims

Doc claims still match the source PR (trunk-io/trunk2#3902, merged 2026-05-05) per Sam's verification:

  • Menu labels: "Quarantine test", "Unquarantine test", "Never Quarantine test", "Remove Never Quarantine"
  • Tooltip text: "Only admins can set manual quarantine"
  • Admin gating: isQuarantineRestricted

Note on the diff

git diff origin/main..HEAD for accept.txt also shows three unrelated entries ((?i)mergeabilitymergeability, misclick removed, (?i)rulesetruleset). These are NOT changes from this PR — they reflect that origin/main has since received commit 109f31a ("Restructure branch protection docs to prioritize Rulesets") which re-introduced the (?i) prefixes and misclick entry. The branch will pick those up on the next merge with main; no action needed in this PR.

Carryover non-blocking note

flaky-tests/quarantining.md:217 "Two quarantine actions are available" vs :222 "All four options" — the "two toggle pairs / four menu labels" mismatch is logically consistent but reads with a small tension. Optional polish: change line 217 to "Four context-menu options are available, grouped as two toggle pairs:" or similar. Not a blocker.

The optional screenshot of the right-click menu is still not added; that remains optional.

No blocking issues. ✅
· Branch ·

@samgutentag samgutentag changed the title docs(flaky-tests): document Never Quarantine context menu in tests table [ready to merge] docs(flaky-tests): document Never Quarantine context menu in tests table May 7, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: Claude code review passed.

@samgutentag
samgutentag marked this pull request as ready for review May 7, 2026 20:00
@claude

claude Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @samgutentag's task in 3m 7s —— View job


Claude Code Review

  • Read changed files (flaky-tests/quarantining.md, vocab accept.txt)
  • Compare against origin/main
  • Verify status of prior review feedback
  • Post review

Summary

This synchronize event fired with no new content commits since the prior review at 2026-05-07 19:48Z — branch tip is still a62d75b (the main→branch merge), with abc7c8f as the latest content commit. All prior review feedback remains resolved:

  • flaky-tests/quarantining.md:220 ✅ — "the test is never quarantined, even if auto-quarantining is enabled for the repo." matches the file's existing terminology (line 231).
  • flaky-tests/quarantining.md:222 ✅ — "All four options require admin permissions. Non-admin users see them as disabled..." is unambiguous.
  • .trunk/configs/vale/styles/config/vocabularies/terms/accept.txt:244 ✅ — (?i)unquarantine placement is alphabetically correct, and the verb form is needed since the file already had unquarantined but not the bare verb that the new "Unquarantine test" UI label requires.

Re-verification of UI claims

Doc claims still match the source PR (trunk-io/trunk2#3902, merged 2026-05-05) per Sam's verification:

  • Menu labels: "Quarantine test", "Unquarantine test", "Never Quarantine test", "Remove Never Quarantine"
  • Tooltip text: "Only admins can set manual quarantine"
  • Admin gating: isQuarantineRestricted

Note on the diff

git diff origin/main HEAD -- flaky-tests/quarantining.md shows an additional change at line 235 ("Quarantine Events filter within the Test History section" instead of "Events tab on the test details page") and unrelated changes in accept.txt ((?i)errored removed, mergeability/ruleset losing the (?i) prefix, misclick removed). These are NOT changes from this PR — they reflect that origin/main has moved forward (now at c5fe9af) since the branch's last merge with main. The PR's actual content commits (41a3143 and abc7c8f) only touch lines 213–224 of quarantining.md and add one line to accept.txt. The branch will pick up origin/main's newer changes on the next merge; no action needed in this PR.

Carryover non-blocking note

flaky-tests/quarantining.md:217 "Two quarantine actions are available" vs :222 "All four options" — the "two toggle pairs / four menu labels" mismatch is logically consistent but reads with a small tension. Optional polish: change line 217 to "Four context-menu options are available, grouped as two toggle pairs:" or similar. Not a blocker.

The optional screenshot of the right-click menu is still not added; that remains optional.

No blocking issues. ✅
· Branch

@trunk-staging-io
trunk-staging-io Bot merged commit f87843b into main May 7, 2026
8 checks passed
@trunk-staging-io
trunk-staging-io Bot deleted the sam-gutentag/never-quarantine-context-menu branch May 7, 2026 20:01

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: Claude code review passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants