Port public server e2e speedups#299
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bda8701. Configure here.
| return | ||
| case <-time.After(1 * time.Second): | ||
| } | ||
| } |
There was a problem hiding this comment.
Install wait skips final poll
Medium Severity
In waitForExtensionInstalled, the loop checks remaining <= 0 before each attempt and then sleeps a full second after failures. When the deadline falls during that sleep, the next iteration exits immediately with the previous lastErr and never re-runs chromeExtensionsCheckOutput, so the test can fail even if the extension became visible during the backoff.
Reviewed by Cursor Bugbot for commit bda8701. Configure here.
## Summary - fix the enterprise extension install wait so it does not sleep across its deadline before failing - add focused coverage for the retry-delay calculation - speed up public e2e startup by restoring the Go build cache for the e2e job and pre-pulling headful/headless images in parallel before the Go e2e suite - keep server e2e files, Go test files, and testdata out of Docker image build contexts so test-only edits do not invalidate runtime image build layers ## Timing Before this PR on the rebased public branch: - test-server-e2e job: 4m20s - Run server e2e tests step: 3m50s - Go e2e package runtime: 186.989s - first display headless container start: 25.4s - first display headful container start: 28.0s After this PR: - build-headful / docker: 50s - build-headless / docker: 52s - test-server-e2e job: 3m28s - Pull e2e images step: 32s - Run server e2e tests step: 2m29s - Go e2e package runtime: 129.429s - first measured container starts are now about 3-4s instead of paying the serial image pull during the display tests The Go e2e package runtime is now under 2.5m. The total e2e job is still above 2.5m because GitHub Actions setup plus the explicit image pull are counted around the Go test process. The Docker context cleanup makes image builds more stable for test-only PRs by preventing e2e/test-only files from invalidating runtime image builder layers. ## Validation - git diff --check - yq . .github/workflows/server-test.yaml >/dev/null - cd server && go test ./e2e -run TestNextExtensionInstallPollDelay -count=1 - autoreview clean after Docker context cleanup - CI green after Docker context cleanup: build-headful, build-headless, test-server-unit, test-server-e2e, scan, chromium-launcher test, socket checks Follow-up for Cursor Bugbot feedback on #299. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Changes are limited to e2e test timing, GitHub Actions workflow, and Docker ignore rules with no production runtime or auth logic touched. > > **Overview** > Fixes a **deadline race** in enterprise extension install polling: `waitForExtensionInstalled` now uses `nextExtensionInstallPollDelay` (shorter sleeps when little time remains, skip sleep when past deadline) instead of a fixed 1s wait, with unit tests for the delay helper. > > **CI / build speed:** the server e2e job re-enables the Go module cache (`server/go.sum`) and **pre-pulls** headful and headless Chromium images in parallel before `make test-e2e`. > > **Docker context:** `.dockerignore` excludes `server/e2e`, `server/**/*_test.go`, and `server/**/testdata` so test-only changes do not invalidate runtime image build layers. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 46007d2. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->


Summary
Port the public-safe parts of the merged private server e2e speedup/stability work into one public PR.
What changed:
test-server-unitandtest-server-e2ecdpmonitorsubtests so they do not share monitor stateexample.com/google.comtransfer setup with deterministic browser profile fixture statePublic/private scope
This intentionally ports only changes whose target files/tests already exist in the public repo.
Skipped private-only work:
server/e2e/e2e_persist_login_test.go, which does not exist in publicWhy
The private test-speedup stack is merged and proved useful for separating real image failures from unrelated e2e flakes. Public currently has the same broad server e2e shape and several of the same tests, so this PR brings the public suite up to parity without changing production image/API behavior.
This follows the same testfast/startfast direction as the private work:
Timing
Baseline is latest successful public
mainserver workflow before this PR: run28205858762on5af656d.After is this PR's successful server workflow: run
28245446048onbda8701.testjobtest-server-e2ejobtest-server-unitImage build timings are shown in validation but should not be treated as the primary test-speedup signal because Docker cache state varies by run. The most comparable signal is the e2e package runtime and the server test critical path.
Validation
Local:
git diff --check origin/main...HEADyq '.' .github/workflows/server-test.yaml >/dev/nullcd server && go test ./e2e -run 'TestExtensionDownloadObservedRequiresUpdateXMLAndCRX|TestExtensionDownloadLogSince' -count=1cd server && go test ./lib/cdpmonitor -run TestBindingAndTimeline -count=1cd server && go test ./e2e -run TestDoesNotExist -count=0cd server && make test-unitcd server/e2e/playwright && corepack pnpm install --frozen-lockfilecd server/e2e/playwright && corepack pnpm exec tsx index.ts(usage/parse check)~/.agents/skills/autoreview/scripts/autoreview --mode local --prompt-file /tmp/public-e2e-speedups-review-notes.mdcleanGitHub Actions on this PR:
28245446048: passedbuild-headful / docker: 1m15sbuild-headless / docker: 2m49stest-server-unit: 1m52stest-server-e2e: 4m38stest: passed in 21sNotes:
typescript, so there is no localtsc --noEmitgate to runReview notes
Note
Low Risk
Test and CI workflow changes only; assertions stay strict and production runtime code is untouched. Main residual risk is heavier parallel e2e load on CI runners, which this PR already exercised successfully.
Overview
Ports public-safe server e2e speed and stability work: faster teardown, less fixed sleeping, clearer CI split, and more parallel safe tests—without changing production images or API handlers.
CI (
server-test.yaml) adds workflow concurrency (cancel in-progress), splits the old combined job intotest-server-unit(make test-unit, Go cache on) andtest-server-e2e(make test-e2eafter image builds, explicit pnpm PATH + Playwright install, Go cache off).E2e harness forces Docker container stop with
StopTimeout(0)and logs[e2e-timing]for start/stop and readiness phases.Flake reduction replaces fixed sleeps with bounded polls: enterprise extension (policy, download logs,
chrome://extensions), Playwright daemon recovery after Chromium restart (WaitDevTools+ retry), and MV3 service-worker checks in Playwright (waitForFunction).Other test changes:
t.Parallel()on selected configure/restart/recording cases; zip transfer bench uses a deterministic S3 fixture instead of external sites; recording audio analysis runs ffmpeg/ffprobe inside the running container;cdpmonitorbinding/timeline subtests each get an isolated monitor viawithMonitor.Reviewed by Cursor Bugbot for commit bda8701. Bugbot is set up for automated code reviews on this repo. Configure here.