Skip to content

Fix Windows-only failures in the Electron backend integration tests - #9668

Merged
tcobbs-bentley merged 8 commits into
masterfrom
tcobbs/electron-race-fixes
Aug 28, 2026
Merged

Fix Windows-only failures in the Electron backend integration tests#9668
tcobbs-bentley merged 8 commits into
masterfrom
tcobbs/electron-race-fixes

Conversation

@tcobbs-bentley

@tcobbs-bentley tcobbs-bentley commented Aug 27, 2026

Copy link
Copy Markdown
Member

Fix Windows-only failures in the Electron backend integration tests

Two causes of the failures were introduced by #9627 and reproduced on Windows in the Node 20 and Node 24 CI legs. The fractional scaling problem has probably always existed.

Should save main window size, position and maximized flag.

Three separate problems in ElectronHost.test.ts:

  • Resize issued mid-restore. fix(core-electron): harden window state and RPC shutdown #9627 replaced the unconditional 250 ms sleep after unmaximize() with a waitUntil that returns immediately, because savedMaximized() and window.isMaximized() flip in the same tick. setSize then landed while Windows was still restoring, and the restored bounds won the last debounced write. Added waitForStableBounds() to wait for the window to settle first.
  • Fractional display scaling. On a display at non-integer scale (e.g. 250%), bounds round through physical pixels, so the realized size/position can differ from what was requested. The test now asserts that the saved state converges on whatever the window actually reports, plus a separate check that the window changed at all — instead of requiring exactly 250×251 at (50, 75).
  • Timeout too tight. waitUntil budget raised from ~1.25 s to ~5 s.

Should ignore a response after its request was removed during shutdown.

Exited with 0x80000003 (STATUS_BREAKPOINT) — a Chromium abort, not an assertion failure. This test never calls ElectronHost.startup(), so it could finish and tear down the main process while Chromium was still initializing. RunSingleTest.ts now awaits app.whenReady() and exits via app.exit() rather than process.exit().

Notes

  • Test-only; no shipped behavior change.
  • Targets master for backport to release/5.13.x. The affected files are identical on both branches, so the cherry-pick should be clean.
  • The Windows leg was run multiple times to verify the fix — the 0x80000003 failure was intermittent and passed on the Node 24 leg.

These only failed on Windows
My Windows display scale is set to 250%, and the rounding that that produced led to the window resize tests being impossible to pass as previously written.
@tcobbs-bentley
tcobbs-bentley requested a review from hl662 August 27, 2026 22:37
@tcobbs-bentley tcobbs-bentley linked an issue Aug 27, 2026 that may be closed by this pull request
2 tasks

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Stabilizes Windows Electron backend integration tests around window restoration, fractional scaling, and Chromium startup.

Changes:

  • Waits for Electron readiness before running tests.
  • Adds window-bound stabilization and longer polling.
  • Records a test-only Rush change entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
RunSingleTest.ts Uses Electron-managed startup and exit.
ElectronHost.test.ts Makes window-state tests scaling-aware.
tcobbs-electron-race-fixes_2026-08-27-22-34-10.json Adds the package change record.
Suppressed comments (1)

core/electron/src/test/backend/ElectronHost.test.ts:224

  • The position check has the same stale-baseline/all-fields issue: it compares with bounds captured before the maximize cycle and requires both coordinates to change. A prior window-manager adjustment can make this true before setPosition is reflected, while a legitimate move where one coordinate was already equal to the target will time out. Compare against bounds captured immediately before the move and accept either coordinate changing.
  assert(await waitUntil(() => window.getBounds().x !== expectedBounds.x && window.getBounds().y !== expectedBounds.y));

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread core/electron/src/test/backend/ElectronHost.test.ts Outdated
Comment thread core/electron/src/test/backend/ElectronHost.test.ts

@GytisCepk GytisCepk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These size tests are constant problem and have been fixed multiple times. I'll need to review them and figure out what is a long term solution here.

Approving for release

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread core/electron/src/test/backend/RunSingleTest.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

core/electron/src/test/backend/ElectronHost.test.ts:228

  • The initial BrowserWindow position is chosen by the OS, so it can already be (50, 75). In that case setPosition is a no-op and the new “window changed” assertion waits five seconds and fails even though persistence is working. Choose a target that is guaranteed to differ from the sampled position.
  window.setPosition(50, 75);

@hl662 hl662 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this, I ran into these flaky windows problems all the time months ago in my first MVP for vitest certa. Could never nail down the problem but you got it

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@tcobbs-bentley

Copy link
Copy Markdown
Member Author

Thanks for doing this, I ran into these flaky windows problems all the time months ago in my first MVP for vitest certa. Could never nail down the problem but you got it

Don't I know it. The last person to make fixes to this file before this PR was me. Those fixes were due to failures in my ARM64 Linux VM.

@tcobbs-bentley
tcobbs-bentley enabled auto-merge (squash) August 28, 2026 15:49
@tcobbs-bentley
tcobbs-bentley merged commit 14666eb into master Aug 28, 2026
23 checks passed
@tcobbs-bentley
tcobbs-bentley deleted the tcobbs/electron-race-fixes branch August 28, 2026 16:35
@mergify

mergify Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@tcobbs-bentley

Copy link
Copy Markdown
Member Author

@Mergifyio backport release/5.13.x

@mergify

mergify Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

backport release/5.13.x

✅ Backports have been created

Details

aruniverse pushed a commit that referenced this pull request Aug 31, 2026
…backport #9668) [release/5.13.x] (#9673)

Co-authored-by: Travis Cobbs <77415528+tcobbs-bentley@users.noreply.github.com>
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.

Some electron backend tests fail on Windows

4 participants