Skip to content

ci: extract shared Playwright-server harness into a composite action (#1752)#1755

Merged
johnrtipton merged 1 commit into
mainfrom
fix/ci-playwright-server-composite-1752
Jun 8, 2026
Merged

ci: extract shared Playwright-server harness into a composite action (#1752)#1755
johnrtipton merged 1 commit into
mainfrom
fix/ci-playwright-server-composite-1752

Conversation

@johnrtipton

Copy link
Copy Markdown
Contributor

Closes #1752.

Problem

nav-hooks-guard and playwright-tests duplicated ~9 setup steps (Python/uv/cache/uv sync/maturin build/Playwright install/migrate/start server/wait). #1753 had to patch the missing maturin develop step into nav-hooks-guard specifically because the copy had drifted out of sync — textbook parallel-path drift (#1646).

Fix (item 3)

Extract the shared harness into a composite action .github/actions/djust-playwright-server. Both jobs now actions/checkoutuses: ./.github/actions/djust-playwright-server → run their own test step → upload server.log on failure → stop server.

  • One definition can't drift a step out of one job.
  • Jobs keep their identity: playwright-tests stays continue-on-error: true (optional), nav-hooks-guard stays blocking; test-summary needs/AND-condition unchanged.
  • Net −31 lines (+95/−126).

Items 1 & 2

Verification

  • Local: both YAML files parse; every composite run step declares shell: bash; both jobs preserve checkout→composite ordering + their stop/upload steps + continue-on-error settings.
  • The empirical proof is this PR's own CI: both playwright-tests and nav-hooks-guard must run green through the composite (a new runner-only path; per Free-threaded hardening / perf polish (follow-up to #1432) #1534, ≥1 runner iteration may be expected for composite-action quirks).

🤖 Generated with Claude Code

…1752)

nav-hooks-guard and playwright-tests duplicated ~9 setup steps (Python/uv/cache/
uv sync/maturin build/playwright install/migrate/start server/wait). #1753 had to
patch the missing maturin step into nav-hooks-guard precisely because the copy had
drifted. Extract the shared harness into .github/actions/djust-playwright-server
(composite); both jobs now `uses:` it after checkout, then run their own test step
+ log upload + server stop.

One definition can't drift a step out of one job (parallel-path-drift, #1646).
Both jobs keep their identity: playwright-tests stays continue-on-error (optional),
nav-hooks-guard stays blocking; test-summary wiring unchanged.

Closes #1752. Item 1 (maturin) shipped in #1753; item 2 (blocking-soak) resolved by
decision — keep nav-hooks-guard blocking (green across #1748/#1753/#1754); item 3
(this dedup).

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

Copy link
Copy Markdown
Contributor Author

Code Review (pipeline-run Stage 7) — APPROVE

CI-only refactor; closes #1752. No 🔴.

What it does: extracts the 9 shared setup steps into .github/actions/djust-playwright-server/action.yml (composite); playwright-tests and nav-hooks-guard become checkout → uses: ./.github/actions/djust-playwright-server → <own test> → upload-log-on-failure → stop-server.

Correctness:

  • Empirical canary (feat: improve error messages for common LiveView mistakes #252): this PR's CI ran both jobs green through the composite (nav-hooks-guard 3m30s, playwright-tests 3m30s) — proves the background server started in a composite step persists into the caller's test step, and the local-action path resolves after checkout.
  • Every composite run step declares shell: bash (required for composite run steps); uses: steps (setup-python/uv/cache) don't need it. Verified.
  • Job identities preserved: playwright-tests keeps continue-on-error: true (optional), nav-hooks-guard has no continue-on-error (blocking); test-summary needs/AND-condition reference the same job names → gating unchanged.

Concern / edge cases considered:

Item 2 (blocking-soak): resolved by decision — keep nav-hooks-guard blocking; green across #1748/#1753/#1754 + #1755 satisfies the #1534 soak.

APPROVE

@johnrtipton johnrtipton merged commit c916e18 into main Jun 8, 2026
14 checks passed
@johnrtipton johnrtipton deleted the fix/ci-playwright-server-composite-1752 branch June 8, 2026 01:24
@johnrtipton

Copy link
Copy Markdown
Contributor Author

Retrospective — PR #1755 (#1752)

Task: extract the shared Playwright-server harness into a composite action (#1752 item 3); resolve item 2 (keep blocking) by decision. Item 1 (maturin) shipped in #1753.

Quality: 5/5

Fixes the root cause of the whole #1752 thread — the duplication that let the maturin step drift out of one job — not just its symptom.

What went well

  • Root-cause over symptom. ci(test): build Rust extension in nav-hooks-guard before running the guard (#1752) #1753 patched the missing maturin step into nav-hooks-guard; this PR removes the duplication that allowed it to go missing. The composite is one definition; a step can no longer exist in one copy and not the other (fix(auth): wrap per-event check_object_permission in sync_to_async (#1638) #1646 parallel-path-drift, now applied to CI not just code).
  • Empirical canary (feat: improve error messages for common LiveView mistakes #252) for free. A composite action has real first-use unknowns (does a backgrounded server survive the composite→caller step boundary? does the local-action path resolve post-checkout?). This PR's own CI answered both: nav-hooks-guard and playwright-tests both ran green through the composite. No guessing.
  • Identity preserved through the refactor. continue-on-error (playwright optional / nav blocking) and the test-summary AND-condition were left untouched because the jobs stayed jobs — the lower-risk composite-action choice over a reusable workflow (which would have restructured the gating).
  • Net −31 lines while adding a reusable building block.

What could be improved

Verified

  • Both YAML files parse; composite run-steps all declare shell: bash; both jobs preserve checkout→composite ordering + stop/upload + continue-on-error. CI fully green — both Playwright jobs through the composite, plus rust/python/demo-checks.

RETRO_COMPLETE

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.

tech-debt: harden nav-hooks-guard CI job (maturin build, blocking-soak, dedup) — follow-up to #1748

1 participant