This repository was archived by the owner on May 29, 2026. It is now read-only.
ci(e2e): exclude @perf tests from PR-time job#242
Merged
Conversation
The 60s budget added in #238 was calibrated against chromium (~20s observed locally) before the cross-browser matrix landed in #239. WebKit on ubuntu-latest against the Vite dev server consistently runs 108-120s; firefox 62-68s. Both blow the budget on every run, and the 3-attempt retry cycle has been pushing the e2e job past its 15min cap (#240 was cancelled by the GHA timeout for this reason). Skip the perf assertion on non-chromium browsers as a short-term fix so PR CI can stay green. The longer-term plan, already documented in the file header, is to move @Perf tests to a nightly schedule against a production bundle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the E2E perf smoke spec to skip the 10k-paragraph setContent budget assertion on Firefox and WebKit while keeping it enforced on Chromium, reducing CI runtime and avoiding job timeouts after the cross-browser matrix was introduced.
Changes:
- Inject
browserNameinto the perf test and add a conditionaltest.skip(...)for non-Chromium browsers. - Add inline rationale documenting the current CI/runtime constraints and the planned move of
@perfto a nightly job.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+40
to
+50
| test('setContent with 10k paragraphs finishes within the budget and scroll is reachable', async ({ page, browserName }) => { | ||
| // Short-term skip on webkit + firefox: the 60s budget was calibrated | ||
| // against chromium (~20s observed) before the cross-browser matrix | ||
| // landed. WebKit on ubuntu-latest against the Vite dev server | ||
| // consistently runs 108-120s; firefox 62-68s. Both blow the budget | ||
| // every run, with retries pushing the suite past the 15min job cap. | ||
| // Until the @perf job moves to a nightly schedule against a | ||
| // production bundle (see file header), keep the regression guard | ||
| // on chromium only. | ||
| test.skip(browserName !== 'chromium', 'perf budget calibrated against chromium only'); | ||
|
|
The per-browser skip added in the first iteration of this PR turned out to be incomplete: chromium on GHA ubuntu-latest also overshoots the 60s budget (~70s observed) once running against the Vite dev server, not just the webkit ~115s and firefox ~65s seen in the prior runs. All three browsers blow the budget; only the local M-class macOS Chromium baseline (~20s) actually fits. Switch to the project's already-documented plan (see file header): exclude @perf-tagged tests from PR-time CI via --grep-invert. Future @Perf tests automatically pick up the same treatment, and the test file stays free of skip logic. The follow-up to move @Perf onto a nightly schedule against a production bundle becomes a small workflow addition rather than a rewrite. Revert the per-browser test.skip in perf.spec.ts so the local `pnpm e2e` invocation still runs the regression guard on all three browsers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Jocs
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Exclude
@perf-tagged tests from PR-time e2e CI via Playwright's--grep-invert "@perf". This unblocks PR CI without touching the regression-guard code ine2e/tests/stability/perf.spec.ts.Root cause
e2e/tests/stability/perf.spec.ts:60assertssetContent(10_000)< 60s. That budget was set when the matrix was chromium-only and the baseline was the local M-class macOS dev box (~20s). After #239 added firefox + webkit to the matrix, every browser on GHAubuntu-latestrunning against the Vite dev server overshoots:With Playwright's 3-attempt retry cycle in CI, webkit perf alone burns ~6 min, which pushed PR #240's job past the 15 min GHA cap (
The operation was canceled).Why grep-invert over
test.skipThe earlier draft of this PR skipped non-chromium browsers via
test.skip(browserName !== 'chromium', ...). It turned out chromium also overshoots on GHA (~70s) — a per-browser skip in the test body would need to skip all three, leaving inert skip logic in the file.Using
--grep-invert "@perf"in the workflow:perf.spec.tsalready foreshadows moving@perfto a nightly schedule.@perftests automatically inherit the same treatment.pnpm e2estill runs the perf regression guard on all three browsers; only the GHA PR job skips it.Follow-up (out of scope)
Move
@perf-tagged tests into a nightly workflow that runs against a production bundle (pnpm build+ serve). At that point per-browser budgets can be calibrated against bundled-output timings and the grep-invert can be removed from the PR-time job.Test plan
@perffailures, all other tests still run and pass.pnpm e2e:chromiumstill runs the perf spec (no skip in code).Generated with Claude Code