Skip to content

fix(scorecard): fix race condition - #2558

Merged
jrichter1 merged 1 commit into
redhat-developer:mainfrom
HusneShabbir:scorecard-catalog-page-update
Mar 16, 2026
Merged

fix(scorecard): fix race condition#2558
jrichter1 merged 1 commit into
redhat-developer:mainfrom
HusneShabbir:scorecard-catalog-page-update

Conversation

@HusneShabbir

@HusneShabbir HusneShabbir commented Mar 16, 2026

Copy link
Copy Markdown
Member

Summary

Stabilize scorecard e2e by explicitly setting the Catalog entity picker to "All" in openCatalog(), removing a race between navigation and the user-picker being ready.

Solution

In CatalogPage.openCatalog(), after clicking the Catalog nav link we now wait for and click the "All" option in the user-picker:

await this.page.getByTestId('user-picker-all').getByText('All').click();

This:

  • Ensures the user-picker has rendered and is interactive before the test continues.
  • Puts the Catalog in a known state (filter = "All") so later steps (e.g. search, open component) behave consistently.
  • Eliminates the race between "Catalog opened" and "user-picker ready / filter applied", so the suite passes reliably on CI.

@rhdh-qodo-merge

Copy link
Copy Markdown

Review Summary by Qodo

Fix CatalogPage e2e openCatalog flow with user picker

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Fix CatalogPage e2e test openCatalog flow
• Add user picker 'All' selection step
• Ensure proper catalog page initialization
Diagram
flowchart LR
  A["openCatalog method"] -- "clicks Catalog link" --> B["Catalog page loads"]
  B -- "selects All from user picker" --> C["Catalog fully initialized"]
Loading

Grey Divider

File Changes

1. workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts 🐞 Bug fix +1/-0

Add user picker selection to openCatalog flow

• Added user picker 'All' selection in openCatalog method
• Ensures catalog page is fully loaded before proceeding
• Fixes flaky e2e test by waiting for user picker element

workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts


Grey Divider

Qodo Logo

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Mar 16, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Locale-locked All selector 🐞 Bug ⛯ Reliability
Description
CatalogPage.openCatalog() now requires an element containing the literal text "All", which
hard-codes an English UI label even though the Playwright suite runs under multiple locale projects.
In non-English locale runs where the Catalog filter label is localized, this step will not find the
element and the test will time out/fail at navigation.
Code

workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts[50]

+    await this.page.getByTestId('user-picker-all').getByText('All').click();
Evidence
The new locator chain explicitly matches text "All" inside the element, while the test suite is
configured to run as separate Playwright projects with browser locale set to fr/it/ja/de/es as well
as en, and the test setup reads navigator.language and switches the app locale accordingly.
Therefore the new selector is not locale-robust and can break locale projects when UI strings are
translated.

workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts[48-51]
workspaces/scorecard/playwright.config.ts[19-59]
workspaces/scorecard/packages/app/e2e-tests/scorecard.test.ts[62-73]
workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts[39-46]

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

## Issue description
`CatalogPage.openCatalog()` uses `getByText('All')`, hard-coding an English label in a test suite that runs across multiple locales. This can fail when UI text is localized.

## Issue Context
Playwright projects run with different `locale` settings, and the test setup reads `navigator.language` and switches the app locale. The selector should not depend on translated visible text if a stable selector (test id) exists.

## Fix Focus Areas
- workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts[48-51]

### Suggested change
Replace:
- `await this.page.getByTestId('user-picker-all').getByText('All').click();`

With something locale-agnostic, e.g.:
- `const allPicker = this.page.getByTestId('user-picker-all');`
- `await expect(allPicker).toBeVisible();`
- `await allPicker.click();`

If disambiguation is needed, prefer role-based selection within the testId scope (e.g., `getByRole('button')`) rather than matching translated text.

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


Grey Divider

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

Grey Divider

Qodo Logo

@rhdh-gh-app

rhdh-gh-app Bot commented Mar 16, 2026

Copy link
Copy Markdown

Changed Packages

Package Name Package Path Changeset Bump Current Version
app workspaces/scorecard/packages/app none v0.0.0

@HusneShabbir HusneShabbir changed the title fix(scorecard): CatalogPage e2e - openCatalog flow and prettier fix(scorecard): fix race condition Mar 16, 2026
@sonarqubecloud

Copy link
Copy Markdown

Comment thread workspaces/scorecard/packages/app/e2e-tests/pages/CatalogPage.ts
@HusneShabbir

Copy link
Copy Markdown
Member Author

Thanks @jrichter1, for looking into the issue and debugging it.

@jrichter1
jrichter1 merged commit 29a48ac into redhat-developer:main Mar 16, 2026
15 checks passed
asmasarw pushed a commit to asmasarw/rhdh-plugins that referenced this pull request Mar 17, 2026
…at-developer#2558)

Made-with: Cursor

Co-authored-by: HusneShabbir <husneshabbir447@gmail.com>
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