test(debates): route the last two device-menu opens through the connection guard - #2479
Merged
Merged
Conversation
…ction guard #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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2477 routed the device-menu tests through
openDeviceSettings, which waits forroomConnectto have been called and for the trigger to be enabled — exactlydevicesLocked === false. Two sites in the same file still open a menu with a barefireEvent.click, and one of them is genuinely exposed to the same race.The exposed one
ignores stale device enumeration results during rapid hardware changes(line 1521) opens the Audio settings menu as its first interaction, with a synchronousgetByRole. Nothing before it waits on the connection: it awaits the ready button andcreateLocalTracks, both of which are local-preview concerns and independent ofroomState. So if the intro connection is still in flight at that point, the effect atdebate-pre-join-screen.tsx:188closes the menu underneath it and every radio disappears.Reproduced with the same technique #2477 used, and it gives the same error:
A controlled A/B, same test, same condition —
roomConnectresolving after 200ms so the connection lands mid-test rather than never:fireEvent.click(getByRole('button', …))(today)Unable to find role="radio"await openDeviceSettings('Audio settings')Holding
roomConnectpermanently pending reproduces it deterministically too, which is how it was found.The other one
Line 1149, inside
lets participants choose microphone and camera devices from desktop settings menus, switches Audio → Video with a bare click. That test opens Audio through the helper first, sodevicesLockedis already false by the time it switches and I could not get it to fail. Changed for consistency rather than because it was breaking — it is the same shape, and leaving one bare click in the file invites the next one.Why the file still passed
openDeviceSettingscovers 20 call sites; these two were the remainder. With only one exposed site left the hit rate is far below the roughly 1-in-6 the file used to show, which is why 10/10 under contention did not catch it — the win from #2477 is real, this is the tail of it.Validation
Tests only; no product code touched.