Skip to content

Guard test timing and quarantine rules in CI - #173

Merged
LMC-Karma merged 4 commits into
mainfrom
feat/test-timing-guard
Oct 23, 2026
Merged

LMC-Karma merged 4 commits into
mainfrom
feat/test-timing-guard

Conversation

@LMC-Karma

@LMC-Karma LMC-Karma commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

Reject additional real-time calls in Swift tests and require issue-linked reasons for quarantined tests.

changes → guards (self-test + timing/quarantine check) → required test

Existing calls have per-file, per-pattern allowances. Intentional system timing needs // real-time: <reason>. Quarantines use .disabled("quarantine: #<issue> <reason>"); condition-gated tests remain allowed. The guard is a spelling check, with documented parser limits.

#172 (parallel CI) is merged. This PR adds the timing guard, its CI job, and documentation against main.

Evidence

  • Before: new real-time waits and unexplained disabled tests were not checked.
  • After: all 44 guard self-test cases pass; the current test tree passes the guard.
  • swift test: 1,349 passed; 11 optional native preview/benchmark tests skipped (Core 532, MacOS 507, App 321). Unsigned Debug Xcode build passed. Workflow YAML parsed and git diff --check passed. Guard also passed against the injectable-clock branch test tree.
  • Rebased onto merged Run independent CI checks in parallel #172, then merged current main to include Run AI reviews only when the maintainer requests them #174, Inject a clock into display refresh debouncing #175, and Make Dark the default and sync icon choices with Finder #170. Resolved the documentation overlap by retaining both the manual-clock and timing-guard guides. Removed the baseline allowance for the debouncer sleep eliminated by Inject a clock into display refresh debouncing #175. Final head: 103efe5.
  • Integrated validation on the resolved branch: swift test passed (1,364 passed, 12 optional native/preview/benchmark checks skipped); unsigned Debug Xcode build passed; all 44 guard self-tests and the current-tree guard passed; both workflow files parsed; git diff --check passed. Fresh hosted CI passed on 103efe5, including Ubuntu guards, unit tests, release package, both app builds, and the required aggregate.
  • An earlier automatic-review run on the intermediate rebased head stopped at the old workflow’s “Confirm the review ran” safeguard because the review workflow changed during rollout. It did not execute a review or report a code fault. The resolved branch inherits the merged on-request review policy. The completed review on the original guard changes found no confirmed fault; no new AI review has been requested.
  • Live desktop checks: not run; the guard reads files without running Swift or touching the desktop.
  • Hosted Ubuntu 24.04 guards job passed: all 44 self-test cases and the test-tree guard passed (job log).

Merge Danger

Door: two-way

Blast Radius: tests

The new guards job is covered by the required test aggregate. Existing waits are not migrated. No dependency, permission, or runtime behavior changes.

@claude

claude Bot commented Oct 22, 2026

Copy link
Copy Markdown

Review summary

Scope check: the diff is confined to .github/workflows/ci.yml, Tools/ci/check-test-timing.sh, Tools/ci/test-timing-baseline.txt, Tools/tests/check-test-timing-test.sh, and docs/DEVELOPMENT.md — no Sources/ files are touched. So the Core/AppKit import boundary, top-left/bottom-left coordinate conversion, main-actor WindowCoordinator routing, private-API disclosure, and Sparkle/SPUUpdaterDelegate ownership rules in AGENTS.md and SECURITY.md don't apply to this change, and the diff doesn't expand into any of them.

Goal vs. diff: I traced the guard's awk logic against every entry path and reverse transition described in the PR body and docs/DEVELOPMENT.md:

  • unannotated real-time call → failure, for every listed API spelling (including the Task<Never, Never>.sleep generic form and calls split across lines)
  • // real-time: <reason> annotation accepted only with a non-empty reason actually in a comment (not inside a string literal)
  • per-file/per-pattern baseline allowances, with increases rejected and --update-baseline/--allow-increase gating working as documented
  • the reverse transitions: a count decrease passes with a hint, and a removed call's key disappears from the baseline on --update-baseline rather than being written as zero
  • quarantine rules: .disabled("quarantine: #N reason") accepted, free-text .disabled(...) rejected unless already baselined, .disabled(if:)/.enabled(if:) exempted, and a // real-time: comment cannot be used to bypass an invalid quarantine

All of this matches the described behavior, and the self-test file's expect_pass/expect_fail calls total exactly 44, matching the PR's stated evidence. No confirmed code faults found — the change matches its stated goal and the repository rules that apply to it.

Two things worth tracking, neither a confirmed code fault:

  • The PR's own evidence section discloses that the guard was only exercised with macOS's awk; the new guards job runs on ubuntu-latest, where the default awk is typically mawk, a different implementation from macOS's. Since guards is now in test's required needs list, a behavioral difference there would block every future PR. Worth confirming the first hosted run is green before treating this as a reliable required check.
  • My review sandbox blocked executing the shell/awk scripts directly, so I verified the logic by static reading against the self-test cases rather than by running them. That's a limitation of my review environment, not a defect in the PR.

Base automatically changed from feat/parallel-ci to main October 22, 2026 23:58
LMC-Karma and others added 3 commits October 22, 2026 16:58
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@LMC-Karma
LMC-Karma force-pushed the feat/test-timing-guard branch from 03bdb68 to 9ede144 Compare October 22, 2026 23:58
@LMC-Karma
LMC-Karma merged commit 6a06d43 into main Oct 23, 2026
7 checks passed
@LMC-Karma
LMC-Karma deleted the feat/test-timing-guard branch October 23, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant