Skip to content

Select Desktop e2e specs by what a change can reach, instead of all 103 #4478

Description

@Astro-Han

The test planner sub-selects everywhere except at its single most expensive point. Desktop e2e is 103 tests treated as one indivisible unit, it takes 651s of a 1528s test job, and 57% of commits pay for it in full.

Measurement

Replaying planTests against the last 300 first-parent commits:

lane commits share
code 283 94.3%
releaseContract 216 72.0%
cliPackage 206 68.7%
runtimeSandbox 202 67.3%
e2e 171 57.0%
storybook 155 51.7%
astryxSurface 143 47.7%
stateRootCompat 88 29.3%
appIcons 65 21.7%
asfSource 61 20.3%
full 59 19.7%
storageStress 17 5.7%

Average workspaces selected: 5.29 of 7.

The narrow gates are well aimed — storageStress at 5.7%, asfSource at 20.3%, appIcons at 21.7% — and workspace tests are already sub-selected through the dependency closure. e2e is the exception: it is all-or-nothing.

Step timings from run 33511334930 (test, 25m28s):

step seconds share
Desktop e2e 651 43%
Run affected standard workspace tests 146 10%
Run Runtime Host tests 135 9%
Storybook smoke 107 7%
Install dependencies + Build + Typecheck 45 2.9%

What actually triggers it

Of the 112 non-full commits that selected e2e, by the first path that drove the selection:

 58  apps/desktop/src/main/**                 52%
 23  apps/desktop/e2e/**                      the specs themselves
 13  apps/desktop/src/renderer/**
  9  apps/desktop/renderer-architecture.json
  5  packages/ui/**
  4  apps/desktop/electron-builder.config.mjs

The largest single driver is the Electron main process, and the 34 spec files it schedules assert renderer contracts — composer, sidebar, settings, session rail, onboarding, quote selection, prompt rail, module hub. Editing app-update-service.ts runs all 103 of them. The main process is genuinely exercised by a real-window launch, so this is not wrong; it is just far wider than the change warrants.

Suggested direction

Derive which specs a change can reach, using the closure machinery #4461 added for the Windows recovery filter. collectWorkspaceSourceClosure already computes a transitive source closure from a set of entry points; running it over the 34 spec files gives a per-spec reachable set, and a changed file then selects only the specs whose closure contains it.

Same pattern, same tool, and the same known limit: the closure walks static imports, so it cannot see process boundaries — fork()ed fixtures and bundled worker entry points are invisible to it. The Windows lane pairs its derived filter with a nightly unfiltered run for exactly this reason, and this would need the same pairing rather than being trusted alone.

Two smaller things worth settling first:

  1. Measure before cutting. Playwright currently runs with the dot reporter, so the log carries no per-test timings and the cold-start share of that 651s is unknown. Each test mkdtemps a userData dir and launches a real Electron window, waiting for the composer to mount; at 103 tests over 651s the average is 6.3s, and the distribution is flat (466s for the first 80, 180s for the last 23), which points at fixed overhead rather than a few slow cases. Adding a line reporter to apps/desktop/e2e/playwright.config.ts makes every subsequent run report this for free — better than spending a runner slot on a one-off dispatch.

  2. workers: 1 and the sharding that does not exist. The config sets one worker for a documented and sound reason: concurrent hidden windows throttle animation frames and share OS focus, invalidating geometry and focus contracts. The same comment says "CI shards run on isolated X displays, so jobs still overlap without sharing focus or a compositor" — but shard appears nowhere in .github/workflows/. Either that sharding was lost or it was never implemented, and the comment should stop describing it either way.

Note that sharding trades against what #4461 optimised for. Splitting 103 tests across 4 shards would take the step from ~10.8m to ~2.7m at the cost of three more concurrent runner slots, and #4461's whole argument is that a slot is scarcer than a minute on shared infrastructure. Selection reduces both; sharding trades one for the other. Prefer selection.

Also worth a look

full fires on 19.7% of commits. FULL_SUITE_FILES includes package-lock.json, so every dependency bump runs every surface. Whether that is the intended cost is worth confirming separately.

Not in scope

Task-level output caching (Turborepo, Nx). It was considered and does not apply: npm ci is 17s, Build 19s, Typecheck 9s — 2.9% of the job combined, already covered by setup-node's npm cache. The only thing large enough to be worth caching is e2e, and caching a required check's test results on an input hash is a protection decision, not a tooling one. The planner already makes that bet explicitly and reviewably.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions