Skip to content

test(desktop): decide the WorkHub popup close in the main process - #5918

Merged
liugddx merged 1 commit into
apache:mainfrom
liugddx:fix/workhub-e2e-popup-race
Oct 2, 2026
Merged

liugddx merged 1 commit into
apache:mainfrom
liugddx:fix/workhub-e2e-popup-race

Conversation

@liugddx

@liugddx liugddx commented Oct 2, 2026

Copy link
Copy Markdown
Member

Fixes #5917.

This is a test-only change to apps/desktop/e2e/workhub-layout.spec.ts. The spec closed the Workbar's native menu in two round trips. First it read the renderer's aria-expanded, then it called closePopup in a separate main-process evaluate. Linux can auto-dismiss that popup in between, and closePopup on a dead popup crashes the main process. The test then fails with "Target page … has been closed" or "session closed". That happened on #5906 and #5902, and both passed on rerun.

Change

  • Track the popup's own lifetime. The existing Menu.prototype.popup probe now also subscribes to the menu's menu-will-close event and records whether the popup is still open.
  • Check and close in one main-process task. The close step reads that flag and calls closePopup in the same task, so a dismissal can no longer land between the check and the call.
  • Renderer assertion unchanged. The existing expect(addPanel).not.toHaveAttribute('aria-expanded', 'true') still pins the renderer state afterwards.
  • Product code untouched. Product code never calls closePopup.

Verification

  • Static checks. Biome and format:check pass, and so does check:e2e-budget (34 tests in 20 files). Type-checking the spec reports no errors in the changed lines.
  • Not reproduced locally. The race depends on Linux native menu behaviour, so I couldn't reproduce it on Windows. Whether menu-will-close arrives in the same main-thread task as the native teardown, so that it closes the window fully rather than narrowing it, rests on Electron dispatching the event synchronously from the menu-closed callback. CI on this PR exercises the spec on Linux.

AI use

Diagnosed and written with Claude Code; the commit carries a Co-Authored-By trailer.

🤖 Generated with Claude Code

`workhub-layout.spec.ts` checked the renderer's `aria-expanded`, then
called `closePopup` in a separate main-process evaluate. Linux can
auto-dismiss the native menu between the two, and `closePopup` on a dead
popup crashes the main process, which fails the test with "Target page
... has been closed" or "session closed". It failed this way on two
unrelated PRs (apache#5906, apache#5902).

Track the popup's lifetime with its own `menu-will-close` event, and check
it in the same main-process task that closes it. The existing
`aria-expanded` assertion afterwards still pins the renderer state.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Oct 2, 2026
@liugddx
liugddx merged commit 6d19e2f into apache:main Oct 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(desktop): workhub-layout e2e crashes Electron when the native menu auto-dismisses before closePopup

2 participants