fix(e2e): use scoped menuitem locator for Settings in profile dropdown [AI /e2e-fix] by zdrapela · Pull Request #4590 · redhat-developer/rhdh

zdrapela · 2026-04-15T09:33:42Z

Summary

Root cause: goToSettingsPage() used clickLink("Settings") which resolves to locator('a').filter({ hasText: 'Settings' }).first() — a broad selector matching any <a> with "Settings" text. Under CI conditions, hidden elements match first and the dropdown menuitem times out.
Fix: Replace with getByRole("menuitem", { name: "Settings" }) to precisely target the dropdown menuitem, matching the fix already on main (PR fix(e2e): fix custom-theme test navigation and overlay issues #4436).

Test Results

Local verification: 5/5 passes
Code quality: tsc, eslint, prettier all pass

UIhelper.goToSettingsPage() selects the profile dropdown item by the hardcoded accessible name
"Settings", so it cannot find/click the menuitem when tests run under non-English locales where the
Settings label is translated (e.g., fr uses "Paramètres"). This can break any suite that calls
goToSettingsPage() in the localization projects (e.g., custom-theme via ThemeVerifier).

Code

e2e-tests/playwright/utils/ui-helper.ts[R254-260]

+    // TODO: [RHDHBUGS-2552](https://redhat.atlassian.net/browse/RHDHBUGS-2552) - Strings not getting translated
+    // The profile dropdown renders Settings as a menuitem, not an <a> link
+    const settingsItem = this.page.getByRole("menuitem", {
+      name: "Settings",
+    });
+    await expect(settingsItem).toBeVisible();
+    await settingsItem.click();

Evidence
goToSettingsPage() now locates a menuitem by the literal name "Settings". However, other E2E tests
treat the profile dropdown Settings item as localized using
t["user-settings"][lang]["settingsLayout.title"], and the French translation for that key is
"Paramètres". The Playwright config defines a French localization project that runs
default-global-header and custom-theme, and ThemeVerifier.setTheme() calls goToSettingsPage(), so a
localized Settings label would cause this method to fail to locate the menuitem.
e2e-tests/playwright/utils/ui-helper.ts[251-261]
e2e-tests/playwright/e2e/default-global-header.spec.ts[116-132]
translations/core-plugins_v1.8_s3281-fr-C.json[309-317]
e2e-tests/playwright.config.ts[197-210]
e2e-tests/playwright/utils/custom-theme/theme-verifier.ts[13-24]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`UIhelper.goToSettingsPage()` hardcodes the menuitem accessible name as `"Settings"`. This conflicts with the repo’s localization support and other tests that use `t["user-settings"][lang]["settingsLayout.title"]` (e.g., fr: `"Paramètres"`), causing failures when running with `LOCALE!=en`.

### Issue Context
- Other tests already use the localized key for the Settings menu item.
- The Playwright config defines a French localization project that runs suites which indirectly call `goToSettingsPage()`.

### Fix Focus Areas
- e2e-tests/playwright/utils/ui-helper.ts[251-261]

### Suggested fix
- Replace the hardcoded `name: "Settings"` with the localized string used elsewhere, ideally with a safe fallback:
 - `const settingsLabel = t["user-settings"][lang]["settingsLayout.title"] ?? "Settings";`
 - Then use `name: settingsLabel` for the menuitem locator.
- Keep the new role-based approach (menuitem) to avoid hidden `<a>` matches.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

Previous review results

Review updated until commit d691a0a

Results up to commit 29c7f6b

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

zdrapela · 2026-04-15T09:34:01Z

/test e2e-ocp-helm-nightly

github-actions · 2026-04-15T10:25:04Z

Image was built and published successfully. It is available at:

The goToSettingsPage() method used clickLink('Settings') which resolves to locator('a').filter({ hasText: 'Settings' }).first() — a broad selector that can match hidden <a> elements elsewhere in the page. Under CI conditions (3 workers, slower rendering), the dropdown menuitem is found but not visible in time, causing TimeoutError. Replace with getByRole('menuitem', { name: 'Settings' }) which precisely targets the profile dropdown menuitem. This matches the fix already on main (PR redhat-developer#4436). Assisted-by: OpenCode

zdrapela · 2026-04-15T14:14:02Z

/agentic_review

rhdh-qodo-merge · 2026-04-15T14:14:10Z

Persistent review updated to latest commit d691a0a

github-actions · 2026-04-15T15:04:20Z

Image was built and published successfully. It is available at:

zdrapela · 2026-04-15T15:07:35Z

/test ?

zdrapela · 2026-04-15T15:07:50Z

/test e2e-ocp-helm-nightly

rhdh-qodo-merge · 2026-04-15T15:07:52Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

🐞\ ≡ Correctness (1) ☼ Reliability (1) ⭐ New (1)

1. Hardcoded Settings label 🐞 ≡

Description

UIhelper.goToSettingsPage() selects the profile dropdown item by the hardcoded accessible name
"Settings", so it cannot find/click the menuitem when tests run under non-English locales where the
Settings label is translated (e.g., fr uses "Paramètres"). This can break any suite that calls
goToSettingsPage() in the localization projects (e.g., custom-theme via ThemeVerifier).

Code

e2e-tests/playwright/utils/ui-helper.ts[R254-260]

+    // TODO: [RHDHBUGS-2552](https://redhat.atlassian.net/browse/RHDHBUGS-2552) - Strings not getting translated
+    // The profile dropdown renders Settings as a menuitem, not an <a> link
+    const settingsItem = this.page.getByRole("menuitem", {
+      name: "Settings",
+    });
+    await expect(settingsItem).toBeVisible();
+    await settingsItem.click();

Evidence
goToSettingsPage() now locates a menuitem by the literal name "Settings". However, other E2E tests
treat the profile dropdown Settings item as localized using
t["user-settings"][lang]["settingsLayout.title"], and the French translation for that key is
"Paramètres". The Playwright config defines a French localization project that runs
default-global-header and custom-theme, and ThemeVerifier.setTheme() calls goToSettingsPage(), so a
localized Settings label would cause this method to fail to locate the menuitem.
e2e-tests/playwright/utils/ui-helper.ts[251-261]
e2e-tests/playwright/e2e/default-global-header.spec.ts[116-132]
translations/core-plugins_v1.8_s3281-fr-C.json[309-317]
e2e-tests/playwright.config.ts[197-210]
e2e-tests/playwright/utils/custom-theme/theme-verifier.ts[13-24]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`UIhelper.goToSettingsPage()` hardcodes the menuitem accessible name as `"Settings"`. This conflicts with the repo’s localization support and other tests that use `t["user-settings"][lang]["settingsLayout.title"]` (e.g., fr: `"Paramètres"`), causing failures when running with `LOCALE!=en`.

### Issue Context
- Other tests already use the localized key for the Settings menu item.
- The Playwright config defines a French localization project that runs suites which indirectly call `goToSettingsPage()`.

### Fix Focus Areas
- e2e-tests/playwright/utils/ui-helper.ts[251-261]

### Suggested fix
- Replace the hardcoded `name: "Settings"` with the localized string used elsewhere, ideally with a safe fallback:
 - `const settingsLabel = t["user-settings"][lang]["settingsLayout.title"] ?? "Settings";`
 - Then use `name: settingsLabel` for the menuitem locator.
- Keep the new role-based approach (menuitem) to avoid hidden `<a>` matches.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Missing settings page wait 🐞 ☼ ⭐

Description

UIhelper.goToSettingsPage() clicks the Settings menuitem and returns without waiting for navigation
or a Settings-page element, so callers can run Settings-page interactions before the page is ready
and intermittently fail. This is observable in flows that call common.signOut() immediately after
goToSettingsPage().

Code

e2e-tests/playwright/utils/ui-helper.ts[R256-261]

+    const settingsItem = this.page.getByRole("menuitem", {
+      name: "Settings",
+    });
+    await expect(settingsItem).toBeVisible();
+    await settingsItem.click();
  }

Evidence

goToSettingsPage ends right after clicking the Settings menuitem, with no URL/element wait.
common.signOut then immediately clicks Settings page-specific selectors (user-settings-menu,
sign-out), and at least one test calls goToSettingsPage() followed directly by common.signOut()
without any intermediate assertion/wait, so the sign-out clicks can race navigation/rendering.

e2e-tests/playwright/utils/ui-helper.ts[251-261]
e2e-tests/playwright/utils/common.ts[51-55]
e2e-tests/playwright/e2e/guest-signin-happy-path.spec.ts[37-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`goToSettingsPage()` clicks the Settings dropdown item but does not wait for navigation or for any Settings-page-specific UI to appear. This allows subsequent steps (e.g., `Common.signOut()` clicking `user-settings-menu`) to run before the Settings page has finished loading, causing intermittent test failures.

### Issue Context
Multiple tests call `goToSettingsPage()` and then immediately interact with Settings page elements. At minimum, `guest-signin-happy-path.spec.ts` calls `goToSettingsPage()` followed by `common.signOut()` with no other wait.

### Fix Focus Areas
- e2e-tests/playwright/utils/ui-helper.ts[251-261]

### Suggested change
After clicking the Settings menu item, add an explicit wait that proves the Settings page is ready, e.g.:
- `await expect(this.page.getByTestId('user-settings-menu')).toBeVisible();` (best, because it’s page-specific), and/or
- `await this.page.waitForURL(/\/settings(\/|$)/);`

Optionally wrap click + wait in `Promise.all` to avoid missing fast navigations:
```ts
await Promise.all([
 this.page.waitForURL(/\/settings(\/|$)/),
 settingsItem.click(),
]);
await expect(this.page.getByTestId('user-settings-menu')).toBeVisible();
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

rhdh-qodo-merge · 2026-04-15T15:12:24Z

Review Summary by Qodo

Fix Settings link selector in profile dropdown for CI stability

🐞 Bug fix

Walkthroughs

Description

• Replace broad link selector with scoped menuitem locator in Settings navigation
• Use getByRole("menuitem") to precisely target dropdown Settings item
• Add visibility check before clicking to prevent timeout errors
• Aligns with existing fix on main branch (PR #4436)

Diagram

flowchart LR
  A["goToSettingsPage()"] -->|"Old: clickLink('Settings')"| B["Broad selector<br/>matches hidden elements"]
  B -->|"CI conditions"| C["TimeoutError"]
  A -->|"New: getByRole('menuitem')"| D["Scoped dropdown selector"]
  D -->|"+ visibility check"| E["Reliable click"]

File Changes

1. e2e-tests/playwright/utils/ui-helper.ts 🐞 Bug fix +7/-5

Replace broad link selector with scoped menuitem locator

• Replace clickLink("Settings") with getByRole("menuitem", { name: "Settings" })
• Add explicit visibility check via expect(settingsItem).toBeVisible()
• Change from generic link selector to role-based menuitem selector
• Improves reliability under CI conditions with slower rendering

e2e-tests/playwright/utils/ui-helper.ts

zdrapela · 2026-04-15T15:38:55Z

The hardcoded "Settings" is intentional and matches the current implementation on main. The TODO: [RHDHBUGS-2552](https://redhat.atlassian.net/browse/RHDHBUGS-2552) comment documents that the global header strings are not getting translated yet — until that product bug is fixed, using t["user-settings"][lang]["settingsLayout.title"] would not work correctly. Once RHDHBUGS-2552 is resolved, this should be updated to use the localized string.

zdrapela · 2026-04-15T19:27:22Z

/retest

zdrapela · 2026-04-16T08:22:03Z

/test e2e-ocp-helm

github-actions · 2026-04-16T08:31:19Z

The container image build workflow finished with status: failure.

…lity - Use scoped menuitem locator for Settings in profile dropdown (redhat-developer#4590) - Fix bulk-import tests: optional chaining for env vars, nightly-only check, retry with filterAndVerifyAddedRepo, correct status text (redhat-developer#4583) - Fix FAB sub-menu click stability with dispatchEvent and toBeVisible guard (redhat-developer#4591) Assisted-by: OpenCode

github-actions · 2026-04-16T10:41:03Z

Image was built and published successfully. It is available at:

openshift-ci · 2026-04-16T11:20:47Z

@zdrapela: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ocp-helm-nightly	`d691a0a`	link	false	`/test e2e-ocp-helm-nightly`
ci/prow/e2e-ocp-helm	`e17ef49`	link	true	`/test e2e-ocp-helm`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci bot added the do-not-merge/work-in-progress label Apr 15, 2026

zdrapela force-pushed the fix/e2e-guest-signin-settings-link branch from 29c7f6b to d691a0a Compare April 15, 2026 14:13

zdrapela marked this pull request as ready for review April 15, 2026 15:07

openshift-ci bot removed the do-not-merge/work-in-progress label Apr 15, 2026

openshift-ci bot requested review from albarbaro and psrna April 15, 2026 15:07

rhdh-qodo-merge bot added Enhancement Bug fix labels Apr 15, 2026

Merge branch 'release-1.8' into fix/e2e-guest-signin-settings-link

03ef4e0

zdrapela mentioned this pull request Apr 16, 2026

fix(e2e): combine fixes for Settings link, bulk-import, and FAB stability #4606

Open

Merge branch 'release-1.8' into fix/e2e-guest-signin-settings-link

e17ef49

Conversation

zdrapela commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Results

Related

Uh oh!

openshift-ci bot commented Apr 15, 2026

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

rhdh-qodo-merge bot commented Apr 15, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Great, no issues found!

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

rhdh-qodo-merge bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 15, 2026

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

rhdh-qodo-merge bot commented Apr 15, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

rhdh-qodo-merge bot commented Apr 15, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

zdrapela commented Apr 15, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zdrapela commented Apr 15, 2026

Uh oh!

zdrapela commented Apr 16, 2026

Uh oh!

github-actions bot commented Apr 16, 2026

Uh oh!

github-actions bot commented Apr 16, 2026

Uh oh!

openshift-ci bot commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zdrapela commented Apr 15, 2026 •

edited

Loading

rhdh-qodo-merge bot commented Apr 15, 2026 •

edited by openshift-ci bot

Loading

rhdh-qodo-merge bot commented Apr 15, 2026 •

edited

Loading

rhdh-qodo-merge bot commented Apr 15, 2026 •

edited by openshift-ci bot

Loading

zdrapela commented Apr 15, 2026 •

edited by openshift-ci bot

Loading