Repository navigation
Conversation
Browser Tests ran the admin, embeds and release-service tests and then the playground tests, with a build before each, so a full run waited about 12.5 minutes on that one job. The playground tests now run as Playground E2E, and Browser Tests keeps its required-check name.
|
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://daniel-ci-playground-job.try.emdashcms.com, https://daniel-ci-playground-job-emdash-playground.emdash-cms.workers.dev (commit 61e65d8)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
There was a problem hiding this comment.
The approach is sound: the playground E2E suite moves out of the serial tail of Browser Tests into its own Playground E2E job. That runs the suites in parallel and keeps the required check names. Both reviewers approved and raised no findings. They checked the following:
Browser Testsnow gates onbrowser == 'true'alone, and the removed step guard was redundant.- The new job reuses the same
e2e_playgroundcondition, build command, Playwright cache key and failure artifacts. - The
Testsaggregator now includestest-e2e-playground, so a failed, cancelled or wrongly skipped playground run still fails the required check. BROWSER_SELECTEDno longer countse2e_playground, so a playground-only plan expectsBrowser Teststo be skipped.
The cost is one extra install and Chromium restore, which the description states. Concurrent jobs sharing a Playwright cache key may produce a harmless cache-save warning.
This was a static review only. No tests or builds were run, and the reported timing gains were not verified.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
What does this PR do?
Runs the playground tests in a job of their own,
Playground E2E, instead of after the admin, embeds and release-service tests inBrowser Tests.Browser Teststook about 12.5 minutes on a full run, nearly 4 of them playground tests. In this PR's CI run the two jobs ran side by side and took 6.8 and 5.4 minutes.Browser Testsis a required check, so it keeps its name. It now runs only when the test plan selects the admin, embeds or release-service tests. The requiredTestsjob checks the new job as a lane of its own, so a failing playground test still fails a required check. The tests themselves don't change.#3393 put the playground tests into
Browser Teststo use fewer runners while jobs queued. Jobs no longer queue, so the second job adds about 1.2 runner minutes for its setup and install, and no wait. Moving them out again is Matt's call. The PR doesn't depend on any other open PR. The run gets shorter once the E2E shards drop below the oldBrowser Tests: in the CI run of #4447 the slowest shard took 14.0 minutes andBrowser Tests13.1.Part of #4449
Type of change
Your connection to this change
Checklist
pnpm typecheckpasses (n/a: workflow file only)pnpm lintpasses (n/a: workflow file only)pnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (n/a: no admin UI change)AI assistance disclosure
Screenshots / test output
Browser Teststook 12.5 minutes, of which install 37 s, the two builds 53 s and 57 s, admin tests 340 s and playground tests 225 s.Browser Teststook 408 s (install 35 s, build 47 s, admin tests 280 s) andPlayground E2E322 s (build 52 s, playground tests 196 s, and 74 s for everything else, the 42 s install included).Testspassed with the new lane.Playground E2Ewas the only test job that ran, it failed on that test, andTestsfailed with "Playground E2E was failure; test plan expected success". A failing admin test (run):Browser Testsfailed on that test,Playground E2Ewas skipped, andTestsfailed with "Browser tests was failure; test plan expected success".pnpm run --filter "@emdash-cms/playground^..." build, then the playground tests underCI=1: both specs pass.