Skip to content

test(debates): stop the device settings menu closing underneath its own tests - #2477

Merged
jwalkingjew merged 3 commits into
masterfrom
preston/stabilise-device-settings-tests
Sep 20, 2026
Merged

jwalkingjew merged 3 commits into
masterfrom
preston/stabilise-device-settings-tests

Conversation

@jwalkingjew

Copy link
Copy Markdown
Collaborator

The device settings menu closes underneath its own tests

debate-room-page-client.test.tsx fails intermittently on CI — on master as well as on branches — with Unable to find role="radio", naming a different device each time. Four examples:

Run Branch Failing test
35473004072 master preserves valid device selections…
35473942744 master lets participants choose microphone and camera devices…
35479390432 #2467 changes speaker output…
35479873272 #2467 ignores an older speaker authorization…

Cause

The intro screen opens its own LiveKit connection. While roomState is 'connecting' or 'reconnecting' — or a participant has ready_at — devicesLocked is true, and debate-pre-join-screen.tsx force-closes any open settings menu:

Closing, not just disabling the trigger: an open popover or sheet keeps its radios clickable, and picking one there restarts the preview underneath the connection that is publishing it.

That behaviour is deliberate and has its own passing test. The bug is the race around it: these tests render and open the menu immediately, so a connection landing mid-interaction shuts the menu underneath them and every device radio disappears.

Measured, not reasoned about

Two earlier readings of this failure were wrong, so this one is backed by a reproduction:

  • Deterministic — holding roomConnect pending across the interaction reproduces the exact error every time.
  • Statistical — under CPU contention the file fails 1 run in 6 locally, which is the rate CI shows and why it passes 12/12 on an idle machine.

Why the previous attempt didn't hold

#2458 read this as a re-render between two clicks and switched to findByRole. Waiting longer cannot help — the menu is closed and stays closed. Its own comment records it still failing "about one run in four, on this branch and on master alike".

The fix

Every site that opens a device settings menu now goes through a helper that waits for the connection to have been attempted and for the trigger to be enabled — which is exactly devicesLocked === false:

async function openDeviceSettings(name: 'Audio settings' | 'Video settings') {
  await waitFor(() => expect(mocks.roomConnect).toHaveBeenCalled());
  const trigger = await screen.findByRole('button', { name });
  await waitFor(() => expect(trigger).toBeEnabled());
  fireEvent.click(trigger);
  return trigger;
}

All 19 call sites, not just the two that happened to fail.

Verification

181/181 on the file, and 10 runs / 10 passes under the same CPU contention that reproduced the failure at 1-in-6. Tests only — no product code changed.

Unblocks #2467, and should stop this costing unrelated PRs a red Test job.

…wn tests

`debate-room-page-client.test.tsx` fails intermittently on CI — on master as
well as on branches — with `Unable to find role="radio"`, naming a different
device each time. It has cost several PRs a red Test job.

The intro screen opens its own LiveKit connection, and `devicesLocked` — a
participant's `ready_at`, or `roomState` being 'connecting'/'reconnecting' —
force-closes any open settings menu. That is deliberate: "an open popover or
sheet keeps its radios clickable, and picking one there restarts the preview
underneath the connection that is publishing it", and it has its own passing
test. The bug is the race around it. These tests render and open the menu
immediately, so when the intro connection lands mid-interaction the menu is shut
underneath them and every device radio disappears.

Measured rather than reasoned about, because two earlier readings of this
failure were wrong: holding `roomConnect` pending across the interaction
reproduces the exact error deterministically, and under CPU contention the file
fails 1 run in 6 locally — the rate CI shows, and the reason it passes 12 times
out of 12 on an idle machine.

An earlier attempt (#2458) read it as a re-render between two clicks and moved
to `findByRole`. Waiting longer cannot help: the menu is closed and stays
closed, which is why its own comment records it still failing "about one run in
four".

So every site that opens a device menu now goes through a helper that waits for
the connection to have been attempted and for the trigger to be enabled — which
is exactly `devicesLocked === false`. All 19 of them, not the two that happened
to fail.

Verified under the contention that reproduced it: 10 runs, 10 passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
geogenesis Ready Ready Preview Sep 20, 2026 2:17am UTC

Request Review

@jwalkingjew
jwalkingjew merged commit 0fc4ce3 into master Sep 20, 2026
4 checks passed
@jwalkingjew
jwalkingjew deleted the preston/stabilise-device-settings-tests branch September 20, 2026 02:26
@ohohoreilly

Copy link
Copy Markdown
Contributor

Confirmed this from the outside before building on it — the diagnosis holds exactly.

All three master failures in the last 200 runs are this file, each a different device test, which is the pattern you described:

  • preserves valid device selections and restarts capture when a selected device is removed (22:19)
  • lets participants choose microphone and camera devices from desktop settings menus (22:39)
  • ignores an older speaker authorization that finishes after the latest choice (02:07)

All three now go through openDeviceSettings. 10/10 here under 16 spinners on 16 cores, and 181/181 idle.

One thing the sweep missed, though: two sites in the file still open a menu with a bare fireEvent.click, and one is genuinely exposed. ignores stale device enumeration results during rapid hardware changes (line 1521) opens Audio settings as its first interaction and waits on nothing that tracks roomState — the awaits above it are createLocalTracks and the ready button, both local-preview concerns. With roomConnect resolving after 200ms it fails with the same Unable to find role="radio", and passes through the helper. Follow-up in #2479.

That is the tail of this, not a dent in it — one exposed site instead of nineteen is why 10/10 under contention did not surface it.

ohohoreilly added a commit that referenced this pull request Sep 20, 2026
…ction guard (#2479)

#2477 moved the device tests onto `openDeviceSettings`, which waits for the
intro connection to settle before opening a settings menu. Two bare
`fireEvent.click` opens were left in the file.

`ignores stale device enumeration results during rapid hardware changes`
opens Audio settings as its first interaction and waits on nothing that
tracks `roomState`, so a connection landing mid-test closes the menu
underneath it and the radios vanish — the same `Unable to find
role="radio"` this file has been failing with. With `roomConnect`
resolving after 200ms it fails through the bare click and passes through
the helper.

The switch to Video settings at line 1149 follows a guarded open, so
`devicesLocked` is already false and it could not be made to fail; changed
for consistency.

181/181 idle, 10/10 under CPU contention.

Co-authored-by: Patrick O'Reilly <patrick@geobrowser.io>

This branch was successfully deployed

1 active deployment
Preview — 63229587 Deployed Sep 20, 2026 by vercel[bot]
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.

2 participants