Skip to content

tracking(desktop): move every test out of Electron E2E that does not need it #4761

Description

@Astro-Han

tracking(desktop): move every test out of Electron E2E that does not need it

#4741 removed transcript-scroll.spec.ts (774 lines), prompt-rail.spec.ts (622) and one test from streaming-remount.spec.ts — 22 layout-tier tests — because they blocked main. Transcript scrolling and the prompt rail now have no automated coverage at all.

This tracker restores that coverage and removes the condition that produced the failures, which is not the tests' placement alone. Follows #4727.

Root cause

E2E holds a large majority of tests that never needed Electron
  → 110 tests, ~6 minutes, on the required check
    → #4523 buys 40% with four Xvfb workers and a warm, reused WebContents
      → the premises those tests were written against — a fresh renderer per
        test, exclusive OS focus, unthrottled frames — stop holding
        → six flake reports in 43 hours

Evidence for the third link: the playwright.config.ts comment #4523 deleted said "Keep the default at one worker: concurrent hidden windows throttle animation frames and share OS focus, invalidating geometry and focus contracts." The 19 spec files asserting computed style, bounding boxes or scroll offsets are exactly those contracts. For the fourth: #4707's trace shows the between-test readiness gate waitForSelector('[data-turn-id]') completing on the previous session's turn-fixture-1, with the assertion then observing turn-prompt-rail-111.

Fixing the fourth link is treating a symptom — #4714 did exactly that and was closed unmerged; #4741 then removed the victim, and the gate is still at fixtures.ts:560. Fixing the first link makes the other three unnecessary.

How far the composition has drifted, in one number: after #4741, the warm-reuse machinery serves one test that runs in CI.

Target shape

83 tests in 34 files → roughly 4 to 8 that genuinely need Electron in CI, plus a manual perf probe. Everything else moves down a tier, and the harness that exists to make a large Electron suite fast is deleted with it.

Counts below are a first pass from what each file asserts; each item confirms its own per-file before moving.

Order

The sections below rank by what is at stake, not by what to do first. Only one dependency is real — P3 needs tier one to be small — so the rest is a choice, and this is it:

  1. P0 transcript scrolling, including the Turn-internal item. Its second bullet is a correction to its first, not a follow-up: landing the verbatim rebuild alone ships a known blind spot for a later PR to explain.
  2. P0 prompt rail. Finishes the only part of this tracker where coverage is currently negative. Everything after it is relocation.
  3. slash-command-menu out of P1, ahead of the rest. flaky e2e: slash-command-menu inline menu shows the 命令 group for a non-leading slash #2948 and flaky(e2e): slash-command-menu specs time out intermittently on the Linux e2e lane #3289 predate ci: cut single-job validation time by up to 40% #4523 and are both this file, so it is a standing flake source rather than only a misplaced test — and test(desktop): sink the slash-command submit rule to its routing function #4752 / test(desktop): sink the /compact menu rule to a tested predicate #4762 already opened the seam it needs.
  4. P2 in batches. The largest block, mostly relocation without extraction, and test(desktop): rebuild transcript and prompt rail coverage as stories #4766 has the play-plus-geometry pattern working.
  5. P1 remainder. Extraction first (test(desktop): sink the slash-command submit rule to its routing function #4752 is the worked example), so the highest unit cost; some seams are still held by app-shell.tsx (refactor(desktop): make AppShell a composition root, R2 (migrate by call site) #4582).
  6. P3.

Two things this order depends on:

  • P3 triggers on a number, not on a finished checklist. The counts here are a first pass, and 27 + 42 leaves 17 rather than 4–8. Re-count tier one after each batch instead of discovering the gap at the end.
  • P3 can be harvested in pieces. The warm fixture already serves exactly one test that runs in CI; the PR that moves that test is the PR that deletes resetPromptRailWindow, promptRailWorker and releaseRendererObservations, because removing a mechanism and removing its last consumer are one change. Only workers: 1 and the Xvfb displays have to wait for the count.

Progress

P0 — rebuild what is currently uncovered

The deleted specs are the specification: git show 376a06f245^:apps/desktop/e2e/transcript-scroll.spec.ts (and the same for prompt-rail.spec.ts) recovers what each guarded.

  • Transcript scrolling (11) — test(desktop): rebuild transcript and prompt rail coverage as stories #4766. Tail following during streaming, growth outside turn wrappers, reader scroll-up not pulled back, nested-scroller gesture consumption, history request bands, the dock affordance, where earlier history lands relative to the reader.

  • Turn-internal geometry stabilitytest(desktop): rebuild transcript and prompt rail coverage as stories #4766, new rather than a restoration. The deleted specs asserted turn-level anchoring (Math.abs(turnTop(anchor) - anchorTop) <= 4, growth arriving below) and tail-following lag while pinned. Every one of them watches content arriving at a reader who stays put; none watches a reader travelling up through materialising content-visibility placeholders, because under perf(desktop): bound transcript rendering and eliminate scroll LoAF #4206 one boundary per Turn made that stable enough not to notice. perf(desktop): bound rendering within oversized turns #4259 puts boundaries inside the Turn; measured there, one upward traversal moves scrollHeight +63%.

    Measuring first changed what this should assert. The traversal is not still, and cannot be. A Turn off screen is laid out at contain-intrinsic-block-size: auto 280px and swaps to its real height on the way past, so unless every Turn is 280px tall, travelling through them moves things by construction: fifteen upward steps drift by up to 163px, every step in the same direction, and the transcript converges about 8% shorter. That is the cost perf(desktop): bound transcript rendering and eliminate scroll LoAF #4206 accepted, not a regression — so UpwardTraversalHoldsTurnGeometry bounds it instead:

    The bound is the property perf(desktop): bound transcript rendering and eliminate scroll LoAF #4206 actually buys: one estimate to correct per Turn, so the correction scales with Turns crossed rather than with what is inside them. Written against today's layout, so perf(desktop): bound rendering within oversized turns #4259 has a check to land against rather than a regression to discover. The constants are calibrated from measurement rather than derived, so 15% is a number to disagree with if it looks wrong; the shape of the assertion is what matters.

  • Prompt rail (11) — test(desktop): rebuild transcript and prompt rail coverage as stories #4766, 10 of 11. Tick geometry, pointer/tick correspondence through elementFromPoint occlusion, observer stability across streaming deltas, turn DOM identity while scrolling, focus and selection retention. Only switching sessions reconstructs only the Host active range stays, on refactor(desktop): make AppShell a composition root, R2 (migrate by call site) #4582.

    Two expectations recorded here before the work turned out not to need them. The PROMPT_RAIL_PROMPT_COUNT-scale conversation was not needed: ChatView reads the Host's bounded active range (DESKTOP_TRANSCRIPT_ACTIVE_RANGE_MAX_TURNS = 10) plus transcriptTurnIndex for the remaining landmarks, so the rail gets its full 64 ticks against 10 mounted Turns — production's own arrangement. A tick for a Turn outside the range comes back out as onLoadTranscriptTurn, which is how the story reaches the jump that used to look dead. And scrollMotion needed no story-side equivalent: apps/desktop/src/renderer/scroll-motion-policy.ts is product code with its own unit test and was never at risk. What actually lost its consumer when test(desktop): remove the layout-tier E2E specs that block main #4741 removed promptRailMotionWindow is the scrollMotion argument in e2e/fixtures.ts — a dead parameter, to be removed with the rest of the fixture under P3.

    One thing a green run here does not mean: the fix(ui): keep the prompt rail clickable on macOS #2338 guard is inert on CI. PromptRailHasNoGapsBetweenTicks walks elementFromPoint down the rail, but headless Chromium paints no platform scrollbar at all and Linux's in-flow one moves the content column left instead of overlaying it, so occlusion only bites headed on macOS. That was already true in E2E. test(desktop): rebuild transcript and prompt rail coverage as stories #4766 adds SMOKE_HEADED=1 so the note is something you can act on.

  • StreamingDeltasKeepThePromptRailObserver, as a component test — the eleventh rail test. It was written as a story in test(desktop): rebuild transcript and prompt rail coverage as stories #4766 and taken back out under review, for the reason this tracker exists to apply: it asserts no geometry. It counts IntersectionObserver constructions and matches a rootMargin string, so a real layout engine buys it nothing, and as a story it was the slowest of the set (10.9s at 8x CPU throttle against a 15s budget). It also needs a positive control the story form made awkward — the probe has to be installed before the rail's observer exists, or nothing proves the patched subclass is ever the one the rail builds, and the story passes green when the rootMargin literal it matches on changes. A component test gets both properties for free. This regression is uncovered until it lands.

P1 — tests that need no layout engine at all (~26)

No geometry assertion, no genuine cross-process assertion. These are state machines against the deterministic fixture backend; a real window gives them nothing and costs a cold start.

Where the target module and its test file do not already exist, the move is an extraction first — the part @Phoenix500526 expected to dominate the cost, and #4752 is the worked example.

P2 — tests that need a real layout engine, not Electron (~42)

Storybook + Chromium. scripts/storybook-visual-smoke.mjs already runs every story in Playwright Chromium and fails on storyFinished with status === 'error'; 133 play functions exist; session-workbar.stories.tsx already compares getBoundingClientRect() and asserts scrollWidth <= clientWidth.

  • Whole files: agent-graph-layout (1), link-color-contract (1), sidebar-geometry (1), transcript-measure (1), partial-history-notice (1), request-header-row-contract (1), settings-row-focus-ring (4), sidebar-titlebar-actions (2), sidebar-project-row (4), quote-selection (2), composer-plus-menu-stability (7)
  • Geometry cases only, file stays: session-workbar (18 geometry references), settings (10), workhub-layout (2), onboarding-viewport (its minimum-window case is genuinely Electron and stays)

The E2E test is deleted in the same PR as the story that replaces it.

P3 — delete the harness that only a large Electron suite needed

Blocked on P1 and P2. Once tier one is 4–8 tests, none of this has a consumer:

  • --workers="$worker_count" and the four Xvfb displays (ci.yml)
  • resetPromptRailWindow, promptRailWorker and the warm fixture (e2e/fixtures.ts, ~87 lines)
  • releaseRendererObservations() preload bridge and the sessions:e2e:release-renderer-observations channel
  • RuntimeHostSessionObservationRegistry.releaseTarget() — added by ci: cut single-job validation time by up to 40% #4523 for this and nothing else; the registry itself is production infrastructure and stays
  • playwright.config.ts back to workers: 1, with the deleted invariant restored — this time somewhere changing it is a reviewable change to every test that depends on it, not a comment that leaves with the change that invalidates it

#4523's 40% is not given back: what is saved is the ~68 tests that no longer start Electron at all.

What stays in Electron

  • proxy-password-editing — real HTTP server, proxy-authorization header the main process actually sent, across two reloads
  • fixture-thread-search — a search hit's session key surviving the bridge intact
  • new-task-reload — reload persistence
  • onboarding-viewport — minimum window size
  • streaming-remount (4) — judgement: it injects failures through the bridge with rejectNextSessionObservation / rejectNextTranscriptOpen, which is genuinely cross-process even though the assertion is a React sequence
  • native-transcript-perf — CDP-driven, real compositor, manual probe behind MAKA_TRANSCRIPT_PERF. Note it currently rides the warm fixture and would need a plain per-test window under P3.

Fixture data

Scenario data lives in apps/desktop/src/main/e2e-fixture (1197 lines). It is test scaffolding rather than main-process logic, so the intent is to share it with the story side rather than write a second copy — settled per PR as each item needs it.

Relationship to other work

Non-goals

  • Not restoring the deleted specs to E2E.
  • Not changing retries: 0.
  • Not patching the readiness gate: P3 removes it.
  • Not moving assertions that genuinely need Electron — native input, window-boundary drags, real folders, reload persistence, cross-process contracts.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

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