Repository navigation
Fail CI on a committed test.only, and keep a failed E2E's screenshots - #649
Conversation
This comment has been minimized.
This comment has been minimized.
8ff713a to
834ad0b
Compare
3c17074 to
e25c7b8
Compare
This comment has been minimized.
This comment has been minimized.
|
Rebased with the stack onto This PR's premises hold under the runner's Chrome:
What changed:
Verification on this head: the backend suite with CI's flags passes (5451 passed, 3 skipped). Listing production's suite under |
Playwright had no forbidOnly, so a test.only committed by mistake would quietly shrink the suite that decides whether production rolls back. In CI it now fails the run instead; staging's E2E, which runs first, is where it fires. Production's E2E job collects the suite in a step of its own before running it, so a suite that cannot load, from a test.only or a spec that throws at module scope, fails the job without failing the step the rollback reads: only tests that ran and failed roll production back. A failed staging or production E2E left nothing to inspect. Both E2E jobs now upload the failed tests' screenshots. Only the screenshots, although the ticket asks for traces: this repository's artifacts are public and GitHub masks secrets only in logs. A trace records every request's headers, and error-context.md holds a call log that lists them too; both would publish the node API key and the Cloudflare Access token. The error text is already in the job log, masked. With nothing to publish them, CI records no traces. The upload is bounded and allowed to fail, so collecting evidence cannot cancel e2e-prod before it publishes the verdict the rollback reads. The artifact name carries the run attempt, because a name cannot repeat within a run. e2e-prod's cap rises from 30 to 35 minutes, so the new steps' caps do not come out of the ten minutes its setup steps had. test_deploy_workflows.py holds every upload from a job that runs Playwright, and every upload of test-results in any workflow, to screenshots, with hidden files off, and to the bound. It pins production's collect step ahead of its test step, and each E2E job's cap above its steps' own caps with room for the setup steps, since a job timeout cancels the test step and a cancelled step is no verdict. ClickUp 123zgec4mxy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e25c7b8 to
3dbf964
Compare
This comment has been minimized.
This comment has been minimized.
|
On the review's cap finding: taken. e2e-prod's cap is now 35 minutes, as the description says, which supersedes "stays at 30" in my rebase note above. With 30, the new collect (3) and upload (2) caps came out of the ten minutes main's job leaves its setup steps, down to five. A slow Head: 3dbf964. The workflow, pin, permission and deploy-workflow tests and their neighbours pass (346) under the suite lock, a 30-minute test step and a 9-minute collect step each fail the cap test, and pre-commit is clean. Only this workflow's value and two comments changed since e25c7b8, whose full backend suite passed locally. |
|
Claude finished @jehanazad's task in 2m 37s —— View job Code review
I diffed
The other three files ( Previously flagged cap issue stays fixed. Traced by hand and found sound on this head:
I could not execute No new issues found. This PR already went through one review round (cap fix applied, confirmed above) and is sound as of Branch · |
ClickUp: 2.8 Small CI and test hygiene fixes, its two Playwright items:
forbidOnly, and evidence from a failed E2E.Stacked on #648 (1.1), itself on #590 (1.4). This PR's own change is the tip commit.
Why
test.onlywould go unnoticed. Playwright had noforbidOnly, so atest.onlycommitted by mistake would quietly shrink the suite that decides whether production rolls back.What changes
forbidOnlyin CI. A committedtest.onlyfails the run instead of narrowing it. Staging's E2E runs first, so that's where it fires.test.onlyor from a spec that throws at module scope, then fails e2e-prod without failing the step the rollback reads. Only tests that ran and failed roll production back.Traces are excluded
The ticket asks for traces. This PR uploads only screenshots, and CI records no traces at all, until Alexander decides otherwise. The reasons:
error-context.mdholds a call log that lists them as well. Either would publish the node API key and the Cloudflare Access token.Local runs trace as they did before, on the first retry.
What the tests pin
In
test_deploy_workflows.py:npm ci. A job timeout cancels the test step, and a cancelled step is no verdict, so a production wedged enough to hang the suite would never roll back. A test step raised to 30 minutes now fails the test.Overlap with #625
#625 (web/strict-e2e-shared) edits
e2e/playwright.config.tstoo, on different lines, and git merges it with this stack cleanly in either order, so neither has to wait for the other.It also carries this ticket's stale-reference item (
e2e/specs/env.d.ts), so this PR leaves that file alone.If #625 lands first, its pre-merge listing runs under
CI=true, andforbidOnlythen fails a PR's typecheck step before merge rather than staging's E2E after it. Until then, staging's E2E is where a committedtest.onlyfirst fails.Known limits
nodes.spec.tsfailures leave no screenshot. Their error is in the masked log.Verification
CI=trueexits 0 with 98 tests, and exits 1 with forbidOnly's error once a trialtest.onlyis added. This ran under the shared suite lock, and the trial file is gone.main, so the upload's first real run is the first failed E2E after this lands.🤖 Generated with Claude Code