Skip to content

Add Playwright coverage-delta CI gate - #422

Merged
WilfordGrimley merged 1 commit into
masterfrom
worktree-agent-ac375cbdc53ad247c
Jul 24, 2026
Merged

Add Playwright coverage-delta CI gate#422
WilfordGrimley merged 1 commit into
masterfrom
worktree-agent-ac375cbdc53ad247c

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

Closes #415. Incident: #389 (the /editor route swap) skipped ~190 Playwright tests in one commit and CI stayed green — nothing checked test inventory across a diff, only whether the tests that still ran still passed.

.github/scripts/coverage_delta.py is a pure static-source parser (no browser, no npm ci) over frontend/tests/**/*.spec.ts (mirroring frontend/tests/perf/ being excluded, same as playwright.config.ts's own testIgnore):

  1. Inventories every test() title (full test.describe(...)-chain-qualified) and its skip state — test.skip, test.describe.skip, or a testInfo.skip(...) call reachable from a beforeEach or the test body itself (the actual pattern Swap Proposal H's unified page onto /editor, redirect /display #389 used file-wide).
  2. Parses the same way against the PR's merge-base via git show — no git checkout of the base ref, the working tree is never touched.
  3. Fails when a title present at base is absent at head, or was active at base and is skipped at head, unless .github/coverage-acks.txt carries a matching coverage-ack: <test id or glob> — <reason> line — same tether-discipline shape as docs_lint.py's in-file ALLOWLIST. New tests and un-skipping are always fine.

Wired as coverage-delta.yml, a standalone workflow (two jobs: parser/diff unit tests, then the real gate) on every pull_request — deliberately no path filter, so the trigger itself can't reintroduce the blind spot this gate exists to close. It's pure stdlib Python with no install step, so the always-run cost is low.

docs/infrastructure.md's CI/CD state list gets a new bullet for this workflow in the same change.

Verification

Parser accuracy vs. the real Playwright test suite (npx playwright test --list): 315 real (runtime-expanded) tests vs. 307 static manifest entries — the entire 8-test delta is exactly the 4 template-literal-titled loop call sites this parser deliberately collapses to one static entry each (documented as a known limitation in the script's own docstring); zero unexplained mismatches. Skip-state cross-check: the parser found exactly the 8 files / 36 tests using the real testInfo.skip(true, "...") beforeEach pattern in the repo today (CardImageStates, DeckbuilderConfirmAffordance, PDFGenerator, PagePreview, PostExportContributionPrompt, ProjectEditorMobileScroll, SelectedImagesRibbon, CommonCardback.visual) — matched by hand via grep first.

Two synthetic proof cases, run locally against this repo (temporary edits to frontend/tests/AddCardToFavorites.spec.ts, reverted before commit — not part of this diff):

Case 1 — renamed (= removed) a test title, Case 2 — added test.skip(...) to another test in the same file:

coverage-delta: base=origin/master (merge-base 03d9fd8ea060)
::error::coverage-delta [newly_skipped] frontend/tests/AddCardToFavorites.spec.ts::AddCardToFavorites tests > adding card to favorites - active at base, skipped at head (file=frontend/tests/AddCardToFavorites.spec.ts, line=53, reason=test.skip). If this is intentional, add a line to .github/coverage-acks.txt: "coverage-ack: frontend/tests/AddCardToFavorites.spec.ts::AddCardToFavorites tests > adding card to favorites — <reason>" (a glob over file/title also works).
::error::coverage-delta [removed] frontend/tests/AddCardToFavorites.spec.ts::AddCardToFavorites tests > renders Add to Favorites button when card is not a favorite - present at base (file=frontend/tests/AddCardToFavorites.spec.ts, line=28), absent at head. If this is intentional, add a line to .github/coverage-acks.txt: "coverage-ack: frontend/tests/AddCardToFavorites.spec.ts::AddCardToFavorites tests > renders Add to Favorites button when card is not a favorite — <reason>" (a glob over file/title also works).

2 unacked coverage regression(s) (0 acked). See issue #415 / incident #389.

Exit code 2. Adding a matching coverage-ack: line for that file then re-running: both violations flip to ACKED, exit code 0.

Unit tests: python3 .github/scripts/tests/test_coverage_delta.py — 25 tests (parser fixtures for nesting/describe.skip/test.skip/inline testInfo.skip/dynamic titles, ack loading + glob matching, diff-logic cases, and a real scratch-git-repo end-to-end integration suite that includes the two synthetic cases above as permanent regression coverage) — all pass.

Real repo, unchanged: gate run against origin/master with no edits — clean, exit 0. docs_lint.py --strict clean. pre-commit run on all new/changed files (ruff, isort, black, mypy, prettier, yaml-check) — all pass.

Not run: no full npx playwright test execution (not needed — this is a static source-inventory check with no dependency on actually running the suite).

Checklist

  • I have installed pre-commit and installed the hooks with pre-commit install before creating any commits.
  • I have updated any related tests for code I modified or added new tests where appropriate.
  • I have manually tested my changes as follows:
    • Ran the two synthetic proof cases locally (see Verification above) and confirmed both the failure and the ack-excuses-it path.
    • Ran the full unit test suite and the gate against the real, unmodified repo.
  • I have updated any relevant documentation or created new documentation where appropriate.

Closes #415. Incident #389 skipped ~190 Playwright tests in one commit
with green CI - nothing checked test inventory across a diff, only
whether tests that still ran still passed. coverage_delta.py statically
parses frontend/tests/**/*.spec.ts titles + skip state at head vs. the
PR's merge-base and fails on a removed or newly-skipped title unless
.github/coverage-acks.txt carries a matching ack line.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@WilfordGrimley
WilfordGrimley merged commit 16cf468 into master Jul 24, 2026
9 checks passed
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.

Editor-surface coverage-delta CI gate

1 participant