Skip to content

test(e2e): scale the per-call timeouts the shared ones did not cover - #32

Merged
brentyates-swx merged 2 commits into
swxtchfrom
fix/e2e-remaining-timeouts
Sep 22, 2026
Merged

brentyates-swx merged 2 commits into
swxtchfrom
fix/e2e-remaining-timeouts

Conversation

@brentyates-swx

Copy link
Copy Markdown

Refs #10, #26

Found by CI, not by reading

#10's fix scaled the config's expect.timeout and APP_READY_TIMEOUT. CI then showed that was not the whole set. On PR #27 — a branch touching only packages/opencode/src/plugin/ — four e2e specs failed, and the log carries both halves of the story:

Timeout: 30000ms                                          <- scaled, working
Timeout 10000ms exceeded while waiting on the predicate   <- not scaled

The first is the config's 10 s expect bound × 3 under CI, so the earlier change is doing its job. The second is a per-call bound written inline that nothing scaled — review-line-comment.spec.ts:60's .toPass({ timeout: 10_000 }) — and that spec was among the four failures.

The sweep

file bound
review-line-comment.spec.ts toPass 10 s
session-timeline-collapse-state.spec.ts three waits, 10 s each
scroll-interaction.spec.ts 20 s
subagent-child-navigation.spec.ts 15 s
session-tab-switch-probe.ts 30 s
reproduction/timeline-suspense/playwright.config.ts 30 s test, 10 s expect

There are now no unscaled timeouts left under packages/app/e2e — verified by grep, which is how these were found in the first place.

What this does not fix

One of those four failures was a toBeVisible that timed out at the already-scaled 30 s. Thirty seconds is not a marginally slow render, so that one is either a genuine defect or a much deeper stall, and scaling will not touch it. It needs its own investigation against a failing run's Playwright trace, which is what #26 is for.

Saying so explicitly because the easy mistake here is to scale everything, watch the failure rate drop, and assume the remainder is the same problem.

🤖 Generated with Claude Code

https://claude.ai/code/session_017iG7xA9wRvNyFYZSXm8inU

@brentyates-swx

Copy link
Copy Markdown
Author

The lint gate caught my own bug in this PR

Worth recording, because it is the first time a control added in this triage pass has caught a defect that nothing else would have.

CI failed on unit → Lint for correctness bugs, which was surprising on a branch that touches only packages/app/e2e. The cause was mine: the script that added the deadline import to each file inserted it before the first line not starting with import . In scroll-interaction.spec.ts the second line is import { — a multi-line import — so the next line is an indented member, and the new import landed inside the braces:

import {
import { deadline } from "../../utils/deadline"
  defineVisualRegions,

A syntax error, in a file I had just described as verified.

Nothing else would have caught it

  • tsc --noEmit on packages/app did not flag it.
  • playwright test --list on that exact file reports "Total: 0 tests in 0 files" — the performance specs are not in the default project that test:e2e:local runs, so CI's e2e job never parses this file at all.

My earlier verification listed two regression specs, which do run, and I generalised from them to a file that does not. That is the mistake, and it is the same shape as several others this session: verifying the thing that was convenient to verify rather than the thing that was changed.

The other four files were inserted correctly — checked each rather than assuming the failure was isolated.

On the gate itself

It now has two catches: the duplicate sessionStatus key that motivated it, and this. Both were in files that the existing checks did not cover — one a rule violation, one a parse failure. The typeAware: false decision matters here too: a parse error is found in 1.3 seconds across 3,364 files.

One incidental finding, not fixed here: the performance e2e specs are not run by CI. They can therefore rot without anyone noticing, which is worth a look separately from #26.

@brentyates-swx
brentyates-swx force-pushed the fix/e2e-remaining-timeouts branch from 247364c to 51809a9 Compare September 22, 2026 17:47
#10's fix scaled the config's expect.timeout and APP_READY_TIMEOUT. CI then
showed that was not the whole set. On a branch touching only
packages/opencode/src/plugin, four e2e specs failed, and the log carries both
halves of the story:

    Timeout: 30000ms                                     <- scaled, working
    Timeout 10000ms exceeded while waiting on the predicate  <- not scaled

The first is the config's 10s expect bound multiplied by 3 under CI, so the
earlier change is doing its job. The second is a per-call bound written
inline, which nothing scaled - review-line-comment.spec.ts:60's
`.toPass({ timeout: 10_000 })`, and that spec was among the four failures.

Swept the rest with the same helper, so there are now no unscaled timeouts
left under packages/app/e2e:

    review-line-comment.spec.ts              toPass 10s
    session-timeline-collapse-state.spec.ts  three waits, 10s each
    scroll-interaction.spec.ts               20s
    subagent-child-navigation.spec.ts        15s
    session-tab-switch-probe.ts              30s
    reproduction/timeline-suspense config    30s test, 10s expect

Worth stating what this does NOT fix: one of those four failures was a
`toBeVisible` that timed out at the already-scaled 30s. Thirty seconds is not
a marginally slow render, so that one is either a genuine defect or a much
deeper stall, and scaling will not address it. It needs its own investigation
against a failing run's trace - which is what #26 is for.

Refs #10, #26

Claude-Session: https://claude.ai/code/session_017iG7xA9wRvNyFYZSXm8inU
The script that added the deadline import to each file inserted it before the
first line not starting with "import ". In scroll-interaction.spec.ts the
second line IS `import {` - a multi-line import - so the next line is an
indented member, and the new import landed INSIDE the braces:

    import {
    import { deadline } from "../../utils/deadline"
      defineVisualRegions,

A syntax error, in a file I had just claimed was verified.

Two things caught it, and one of them is the point.

The lint gate added earlier in this triage pass failed the build with a parse
error. That gate exists because a duplicate key in an e2e mock survived
undetected; this is the second defect it has caught, and the first of my own.

Nothing else would have. `tsc` did not flag it - and `playwright test --list`
on that file reports "0 tests in 0 files", because the performance specs are
not part of the default project that `test:e2e:local` runs. So CI's e2e job
never parses this file at all. My earlier verification listed two REGRESSION
specs, which do run, and I generalised from them to a file that does not.

The other four files were inserted correctly; checked each rather than
assuming the failure was isolated.

Refs #10, #26

Claude-Session: https://claude.ai/code/session_017iG7xA9wRvNyFYZSXm8inU
@brentyates-swx
brentyates-swx force-pushed the fix/e2e-remaining-timeouts branch from 51809a9 to cf473af Compare September 22, 2026 18:11
@brentyates-swx
brentyates-swx merged commit b3f2691 into swxtch Sep 22, 2026
3 checks passed
@brentyates-swx
brentyates-swx deleted the fix/e2e-remaining-timeouts branch September 22, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant