Skip to content

[Bug]: E2E regression tests can merge red — PR CI never runs the E2E suite #10518

Closed

Description

@MumuTW

Summary

A PR that adds an E2E regression test never runs that test. pr.yml does not invoke the E2E suite — e2e.yml is reachable only via schedule (on main) and via release-cut.yml's workflow_call. So a fix PR can add a regression test, go green, merge, and only then have its own test run for the first time — on main, where nobody is gating on it.

This has already happened at least twice, and both tests have never passed on main.

Environment: Linux (ubuntu-latest, electron-headless project), main.

Case 1 — headless-serve-desktop-activation.spec.ts (11 days red)

Added 2026-07-13 by 1d2aaf1bf ("Fix recipe serve desktop promotion", #8646). It landed after the last green scheduled run, so the first scheduled run that ever contained it was 29353814223 (e0edc8ef7, 07-14) — where it failed at :80. It has failed in every scheduled run since, and still fails today at :89 in 30130776139.

TimeoutError: electronApplication.firstWindow: Timeout 60000ms exceeded while waiting for event "window"

Case 2 — markdown-explorer-find-focus.spec.ts

Added 2026-07-23 23:18 by 65f245f07 ("fix(editor): focus Explorer-opened Markdown for find", #8083), which closes #8078. The first scheduled run containing it, 30113308910 (17959a22b, 07-24), failed — as did every run since.

The failure is at the assertion that encodes the bug. The earlier steps all pass: the README.md row is visible, the click lands, and .rich-markdown-editor becomes visible. Only the find box never appears:

Error: expect(locator).toBeVisible() failed
Locator: getByRole('textbox', { name: 'Find in rich markdown editor' })
Expected: visible
Error: element(s) not found

  > 23 |   ).toBeVisible()
    at tests/e2e/markdown-explorer-find-focus.spec.ts:23:5

That is the original #8078 symptom: open a Markdown file from Explorer, press Find, nothing happens. #8078 should not be considered verified fixed — either the fix does not hold under electron-headless on Linux, or the test is wrong. Nothing in CI has ever distinguished the two.

Reproduction

I re-ran the affected shards twice, independently, on a fork at 1367094bb (a strict subset of upstream main, no local commits). Both specs failed identically in both runs and match main's own runs — these are not runner flake. A test in the same shards did move between the two runs (terminal-tab-close-restart-persistence → terminal-wedged-write-pipeline-recovery), which is what flake actually looks like here.

Suggested fix

Give PRs some E2E signal, so a regression test cannot merge red:

  • run the E2E suite (or a sharded subset) on PRs that touch tests/e2e/** or the relevant src/** paths, or
  • at minimum run the specific spec files a PR adds or modifies.

Related: the standing main E2E failures are tracked separately in #10519.

Activity

  1. nwparker commented on Jul 27, 2026

    @nwparker
    Contributor

    Repro status: has_repro (CI config, conclusive)

    Summary

    E2E suite is not required on pull requests. .github/workflows/e2e.yml only triggers on workflow_call, workflow_dispatch, and a schedule cron — not pull_request. pr.yml does not call the E2E workflow.

    Evidence

    # .github/workflows/e2e.yml
    on:
      workflow_call: ...
      workflow_dispatch: ...
      schedule:
        - cron: '0 17,22 * * *'

    No pull_request: trigger. No uses: .../e2e.yml from pr.yml.

    Impact

    E2E regressions can merge red; failures only show up on scheduled/default-branch runs.

  2. added
    has_reproConclusive reproduction exists (test/script/CLI evidence). See docs/bug-reproductions/
    on Jul 27, 2026
  3. MumuTW commented on Jul 27, 2026

    @MumuTW
    ContributorAuthor

    Opened #10868 for the narrow half of this: a pull_request job that runs exactly the E2E specs a PR touches, so a PR that adds a regression test can no longer merge without that test ever having run.

    Deliberately not in scope there: making the full sharded suite required on PRs. That is 22 minutes of wall clock, and main is red today (2 specs, #10621 open for one), so flipping it on would block every merge until #10519 closes. Worth deciding separately once main is green.

    Both paths are demonstrated on fork runs, since this PR's own checks are action_required: run path selected and ran exactly the one touched spec (2m53s), skip path finished in 16s with every setup/build step skipped.

    Also worth recording here: both cases cited in this issue are now green on main as of the 07-26 scheduled runs — markdown-explorer-find-focus.spec.ts:6 ✓ and headless-serve-desktop-activation.spec.ts:89 ✓, neither spec file having been modified since it landed. So #8078 does look genuinely fixed after all; my "should not be considered verified fixed" note above no longer holds. The gate gap this issue is actually about is unchanged.

  4. added a commit that references this issue on Jul 28, 2026
    1fa9ffb
  5. added a commit that references this issue on Jul 31, 2026
    f138986
  6. added 2 commits that reference this issue on Oct 15, 2026
    2934628
    5625167
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinghas_reproConclusive reproduction exists (test/script/CLI evidence). See docs/bug-reproductions/

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions