TEST: fix flaky CreateTargetDialog accessibility test - #2335
Open
Roman Lutz (romanlutz) wants to merge 1 commit into
Open
TEST: fix flaky CreateTargetDialog accessibility test#2335Roman Lutz (romanlutz) wants to merge 1 commit into
Roman Lutz (romanlutz) wants to merge 1 commit into
Conversation
Fluent's Dropdown focus modalizer (tabster) puts aria-hidden on the dialog's ancestors while the listbox popover is open, and under jsdom it is not reliably restored when the listbox closes. The existing restoreDialogAccessibility() helper stripped it as a one-shot snapshot at the end of selectTargetType(), so any aria-hidden re-applied afterwards (e.g. on a later focus change) left every subsequent *ByRole query blind - *ByRole defaults to hidden: false. That is what made "should keep full long registry names accessible after selecting RoundRobin targets" fail intermittently in CI while the *ByLabelText assertion just above it passed. Replace the point-in-time cleanup with a MutationObserver installed per test that re-runs the cleanup whenever aria-hidden reappears above the dialog. The helper is now a no-op while a listbox is open, so the modalizer keeps its real behavior and only the missing restore is compensated for. Assertions are unchanged - the Remove button is still required to be reachable by role. Verified by re-adding aria-hidden to a dialog ancestor from a timer before the failing assertion: reproduces the exact CI error without the observer, passes with it. Suite run 15x with no failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48ec2fc7-3fd7-4188-a3ff-df3cc2a64790
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
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.
Description
CreateTargetDialog > should keep full long registry names accessible after selecting RoundRobin targetsfails intermittently in CI with:Seen on PR #2325 (a uv.lock-only dependabot bump, so unrelated to any product change); a plain re-run of the same commit passed.
Root cause. Fluent's Dropdown focus modalizer (tabster) sets
aria-hidden="true"on the dialog's ancestors while the listbox popover is open. In a real browser it removes that on close; under jsdom it is not reliably restored. BecausegetByRolesearches the accessibility tree and defaults tohidden: false, anything under anaria-hiddenancestor becomes invisible to it.The test file already had a
restoreDialogAccessibility()workaround, but it was a one-shot snapshot called once at the end ofselectTargetType(). If tabster re-applied the attribute afterwards (for example on a later focus change from a subsequent click), nothing stripped it again, and every later*ByRolequery in that test went blind. Under CI load the timing shifted just enough for that to happen.The failure signature matches exactly: the
getByLabelText("Selected target: ...")assertion immediately above passed (label queries do no accessibility filtering) while thegetByRole("button", { name: "Remove ..." })assertion failed, even though both elements render from the same JSX block inCreateTargetDialog.tsxand always appear together.Fix. Replace the point-in-time cleanup with a
MutationObserver(watchDialogAccessibility()) that re-runs the cleanup wheneveraria-hiddenreappears above the dialog, installed inbeforeEachand disconnected inafterEach. This is centralized, so future*ByRoleassertions in this file do not need to remember a workaround.The helper is now a no-op while a listbox is actually open, so tabster keeps its real behavior and only the restore that jsdom skips is compensated for.
Notably, the assertions are unchanged. No swap to
getByLabelTextand no{ hidden: true }escape hatch, since the point of the assertion is that the Remove button is genuinely reachable in the accessibility tree.Scope check: this is the only test file that opens a Fluent listbox popover inside a modal Dialog, so no other file has the same latent race. ChatWindow's converter Combobox is not inside a dialog and therefore has no modalizer.
Tests and Documentation
Test-only change; no product code and no documentation affected, so JupyText was not applicable.
Verification:
aria-hidden="true"to a dialog ancestor from asetTimeoutright before the failing assertion reproduces the exact CI error with the observer disabled, and passes with it enabled. The temporary sabotage was reverted before committing.npm run lintandnpm run type-checkboth clean.