[DO NOT MERGE] test: validate sharded e2e run - #1925
Open
paustint wants to merge 2 commits into
Open
Conversation
The e2e job was ~17m30s wall clock and gates nothing else, making it the
critical path for every PR that touches api or jetstream. 12m40s of that was
the test run itself, executing serially on a single worker.
Measured on the first sharded run: e2e 17m30s -> 11m02s, with 127 passed,
3 skipped (all pre-existing `test.skip` in source), 0 failures and 0 retries.
Test coverage is unchanged — 126 tests before and after.
E2E sharding
- Split across a 4-way matrix. Each shard is its own job with its own postgres
service, app server and seeded users, so the only resource shared between
them is the Salesforce org. Raising `workers` inside one job would instead
have them contend for the same database and logged-in user.
- Playwright runs the `setup` project in every shard, so each gets its own
authenticated storage state.
- No `needs:` on build-and-test. Downloading a prebuilt dist would avoid four
rebuilds but serialise e2e behind an 8m job; independent builds keep e2e
parallel, which is what actually shortens the PR.
- `maxFailures` drops to 1 because the budget is now per shard.
- Shards emit blob reports that merge-e2e-reports stitches into the single HTML
report the old job produced, tolerating an empty download for the case where
the affected-guard skips e2e.
e2e-required
- A matrix reports one check per shard ("e2e (1, 4)" …) and no check named
"e2e", so branch protection requiring "e2e" would block on a context that
never reports. This job aggregates the matrix behind one stable name.
Playwright browsers
- Cached by resolved version, and only chromium is installed — the config
defines a single Desktop Chrome project, so firefox and webkit were being
downloaded on every run and never used. Install dropped 82s -> 22s on a cache
miss. OS packages are not cacheable, so a hit still runs `install-deps`.
Nx computation cache
- Persist .nx/cache through the GitHub Actions cache. This is the local cache,
not Nx Cloud: nothing leaves GitHub and there is no plan to exhaust.
- Production builds opt out with --skip-nx-cache. Env vars are not declared as
Nx inputs, so a cached build can be replayed with the wrong values baked in.
build:landing already did this; build:ci and build:affected now match.
Throwaway comment in apps/api/src/main.ts so `nx affected` marks api as affected and the sharded E2E suite runs on this PR. No functional change.
Contributor
There was a problem hiding this comment.
Pull request overview
Throwaway validation PR to exercise the new sharded Playwright E2E workflow end-to-end (as in #1924), including shard fan-out, aggregate required check, merged report artifact, and cache behavior.
Changes:
- Disable Nx computation cache for production builds while persisting Nx’s local cache for typecheck/test speedups.
- Run E2E as a 4-shard matrix emitting blob reports, then merge them into a single HTML report artifact.
- Switch CI Playwright reporting to blob reports and tighten CI failure budget per shard.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Opt production CI/affected builds out of Nx cache replay via --skip-nx-cache. |
| apps/jetstream-e2e/playwright.config.ts | Adjust CI maxFailures/workers and switch CI reporter to blob output for later merging. |
| apps/api/src/main.ts | Add a throwaway comment to force api to be considered affected for this validation run. |
| .github/workflows/ci.yml | Add Nx cache restore, shard E2E into a matrix, cache Playwright browsers, merge blob reports, and add an aggregate e2e-required check. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+286
to
+289
| - name: Resolve Playwright version | ||
| if: steps.guard.outputs.should_run == 'true' | ||
| run: pnpm playwright install --with-deps | ||
| id: playwright-version | ||
| run: echo "version=$(node -p "require('@playwright/test/package.json').version")" >> "$GITHUB_OUTPUT" |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🚫 DO NOT MERGE — throwaway validation PR
Exists purely to watch the sharded E2E suite run end to end. Close it when you're done looking.
It carries the same CI changes as #1924, plus a one-line comment in
apps/api/src/main.tssonx affectedmarksapias affected. #1924 only touches CI files, so its affected-guard correctlyskips E2E and never exercises the sharding.
What to watch
1. Four shards run in parallel —
e2e (1, 4)…e2e (4, 4).Expect them to be uneven. From the last full run:
metadata.api.spec.ts26sexternal-auth-logged-in.spec.ts122steam-audit-log.api.spec.ts25steam.spec.ts156sPlaywright balances by test count (33/33/32/32), not duration. Total wall clock is set by the
slowest shard, so ~11m rather than the ~7m an even split would give.
2.
e2e-required— new aggregate check. A matrix reports one context per shard and none namede2e, so branch protection pointing ate2ewould wait on a context that never reports. This jobrolls the matrix up behind one stable name. It should go green only after all four shards pass.
3.
merge-e2e-reports— should stitch the four blob reports into a singleplaywright-reportartifact. Already verified to no-op cleanly when the guard skips E2E; this run exercises the real
merge path.
4. Playwright browser cache — should now be a hit. The previous run populated it and cut
install from 82s to 22s while still missing; a hit skips the download entirely and only runs
install-deps.5. Flakiness — the previous sharded run was 127 passed / 3 skipped / 0 failures, 0 retries,
with test coverage identical to pre-sharding (126 tests either way). Worth confirming that holds on
a second run, since sharding changes timing and flakes are probabilistic. Any retry at all is the
signal to look closer.
Known risk
All four shards hit the same Salesforce org. Shard placement of the mutating specs is incidental —
Playwright shards by file order, so it reshuffles when specs are added. A shard failing on a
session or deploy error rather than an assertion is the signature of contention.