Skip to content

test(admin): pick the role option directly to deflake the allowed-domains dialog test - #2398

Merged
ascorbic merged 1 commit into
emdash-cms:mainfrom
danielmlr:test/allowed-domains-role-select-determinism
Aug 10, 2026
Merged

ascorbic merged 1 commit into
emdash-cms:mainfrom
danielmlr:test/allowed-domains-role-select-determinism

Conversation

@danielmlr

Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #2387

AllowedDomainsSettings.test.tsx > updates the default role from the edit dialog
is flaky on CI — #2387 has the full analysis and a failing run. In short: the
test drives the Default Role select blind:

const roleSelect = screen.getByLabelText("Default Role").element() as HTMLButtonElement;
roleSelect.focus();
await userEvent.keyboard("{ArrowDown}{ArrowDown}{Enter}");

The option list for this dialog is Subscriber (10) / Contributor (20) /
Author (30) / Editor (40) — useAllowedDomainsRolesConfig filters
ROLE_ENTRIES to value <= MAX_SELF_SIGNUP_DEFAULT_ROLE. The fixture domain
starts at Author (30) and the assertion expects Editor (40), which is exactly
one step down the list. The test sends two {ArrowDown}s — so it
passes only when exactly one keystroke is absorbed by opening the popup and
the highlight starts on the currently selected item. When that doesn't hold,
{Enter} commits Subscriber (10), the first item in the list — the exact
failure in #2387, and the same flip shows up in the CI history of #2374, where
the admin package is byte-identical across three SHAs yet Browser Tests goes
fail/pass/fail (details in
#2374 (comment)).

The change: open the select and click the Editor option directly. Kumo's
Select.Option renders Base UI's select item with role="option", so the
option can be addressed by its accessible name. The test awaits the option's
visibility before clicking, which removes the popup-mount race and makes the
test independent of the ordering of ROLE_ENTRIES.

Why not userEvent.click, as suggested in #2387

The issue suggests the pattern from Settings.test.tsxuserEvent.click on
the combobox, then on the option. That pattern works there because the language
combobox sits on the settings page; inside this modal dialog it times out on
the very first click, on the trigger, because Base UI's inert overlay
intercepts the pointer events:

TimeoutError: locator.click: Timeout 14813ms exceeded.
  - <div aria-hidden="true" role="presentation" data-base-ui-inert=""></div> intercepts pointer events

The clicks are therefore programmatic (element().click()), following the
delete-confirmation tests in the same file, which already use a programmatic
confirmButton.click() for the same reason. That interception is also the
likely reason this test used blind keyboard navigation in the first place.

On the sweep suggested in #2387: this is the only keyboard-driven Select
interaction of this shape in the admin tests. The {ArrowDown} uses in
slash-menu.test.tsx drive the editor's slash menu, and
SortableContentSettingsSections.test.tsx drives drag-reorder — neither is a
Select.

Why there is no failing test in this PR

CONTRIBUTING.md asks bug fixes to include a failing test. That does not apply
cleanly here, because the test is the defect: it asserts a correct
expectation through an unreliable interaction, and the race cannot be pinned
deterministically without changing the very timing under test. The evidence is
the failing run linked in #2387 and the fail/pass/fail CI history of #2374
across byte-identical admin sources.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change) — targeted file plus full admin browser suite, see below
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation — n/a, test-only change
  • I have added a changeset — n/a, test-only change with no user-facing effect
  • New features link to an approved Discussion — n/a, not a feature

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 5 (diagnosis, draft), Claude Fable 5 (implementation, review pass)

Drafted and reviewed with Claude (Opus 5: diagnosis and draft; Fable 5: implementation and review pass, separate sessions).

Screenshots / test output

Targeted file, six consecutive green runs during drafting, re-verified in
review (two further targeted runs plus the full admin browser suite):

✓ |chromium| tests/components/settings/AllowedDomainsSettings.test.tsx (10 tests)

 Test Files  1 passed (1)
      Tests  10 passed (10)
 Test Files  115 passed (115)
      Tests  1384 passed (1384)

The userEvent.click variant from #2387, tried on the same branch, fails
deterministically with the inert-overlay timeout quoted above.

…ains dialog test

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 7780a21

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added review/needs-review No maintainer or bot review yet area/admin size/XS labels Aug 9, 2026

@emdashbot emdashbot Bot 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.

This is a narrowly scoped test-only fix for the flaky AllowedDomainsSettings browser test. The approach is sound: the failing interaction relied on two {ArrowDown} keystrokes to move exactly one step down in a Base UI/Kumo Select, which only works when one keystroke is swallowed by opening the popup and the highlight starts on the selected item. Replacing that with an explicit open + await for the role="option" element + programmatic click removes the ordering and popup-mount races and makes the test independent of ROLE_ENTRIES order. The fallback to programmatic clicks is consistent with sibling tests that already do the same inside modal dialogs (e.g., MenuEditor.test.tsx, delete confirmations in this file), and the PR author’s reasoning for not using userEvent.click (Base UI inert overlay intercepts pointer events) matches the patterns already present in the repo.

Nothing in the change touches runtime code, logged-out routes, content-table queries, localization, SQL, or authorization, so the usual AGENTS.md cross-cutting checks do not apply. The labels used in the test (Default Role, Editor) are the same English-source Lingui strings used in the component and fixtures, so the test is appropriately locale-locked like the other admin tests.

I read the diff, the full test file, the component under test, the roles config hook, and several sibling tests. No AGENTS.md convention violations or correctness issues; this is a clean deflake.

@pkg-pr-new

pkg-pr-new Bot commented Aug 9, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@2398

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@2398

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@2398

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@2398

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@2398

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@2398

emdash

npm i https://pkg.pr.new/emdash@2398

create-emdash

npm i https://pkg.pr.new/create-emdash@2398

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@2398

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@2398

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@2398

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@2398

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@2398

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@2398

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@2398

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@2398

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@2398

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@2398

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@2398

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@2398

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@2398

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@2398

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@2398

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@2398

commit: 7780a21

@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-review No maintainer or bot review yet labels Aug 10, 2026
@ascorbic
ascorbic merged commit 317eb3e into emdash-cms:main Aug 10, 2026
49 checks passed
@danielmlr
danielmlr deleted the test/allowed-domains-role-select-determinism branch August 10, 2026 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/admin review/approved Approved; no new commits since size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky Browser Test: AllowedDomainsSettings "updates the default role from the edit dialog" (keyboard-driven Select race)

3 participants