Skip to content

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

Open
zdrapela wants to merge 3 commits intoredhat-developer:release-1.8from
zdrapela:fix/e2e-combined-fixes
Open

fix(e2e): combine fixes for Settings link, bulk-import, and FAB stability#4606
zdrapela wants to merge 3 commits intoredhat-developer:release-1.8from
zdrapela:fix/e2e-combined-fixes

Conversation

@zdrapela
Copy link
Copy Markdown
Member

@zdrapela zdrapela commented Apr 16, 2026

Summary

Combined E2E test fixes from #4590, #4583, #4591, and new fixes:

Supersedes #4590, #4583, #4591

@openshift-ci openshift-ci bot requested review from gustavolira and josephca April 16, 2026 08:34
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Apr 16, 2026

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Misleading status test names 🐞
Description
In bulk-import.spec.ts, the updated assertions now check for status text "Added" in the Added
Repositories table, but the related test titles/comments still state the expected status is "Already
imported". This mismatch makes test reports misleading and increases the chance of future edits
reintroducing incorrect assertions.
Code

e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts[R129-137]

  test('Verify that the two selected repositories are listed: one with the status "Already imported" and another with the status "WAIT_PR_APPROVAL."', async () => {
-    await common.waitForLoad();
-    await bulkimport.filterAddedRepo(catalogRepoDetails.name);
-    await uiHelper.verifyRowInTableByUniqueText(catalogRepoDetails.name, [
+    await bulkimport.filterAndVerifyAddedRepo(catalogRepoDetails.name, [
      catalogRepoDetails.url,
      "Added",
    ]);
-    await bulkimport.filterAddedRepo(newRepoDetails.repoName);
-    await uiHelper.verifyRowInTableByUniqueText(newRepoDetails.repoName, [
+    await bulkimport.filterAndVerifyAddedRepo(newRepoDetails.repoName, [
      "Waiting for Approval",
    ]);
  });
Relevance

⭐⭐⭐ High

Bulk-import E2E status strings were previously updated; keeping test titles/comments consistent is
maintainability they accept.

PR-#4139
PR-#4437
PR-#4033

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test title explicitly mentions "Already imported" while the updated assertion for the same flow
now expects "Added"; the merge-status test title also still claims the status updates to "Already
imported" while the new verification expects "Added".

/e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts[129-137]
/e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts[196-220]

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

### Issue description
Some Bulk Import E2E test titles/comments still describe the expected status as **"Already imported"**, but the assertions were changed to check for **"Added"**. This makes failures and intent confusing.

### Issue Context
The assertions appear intentionally updated (per PR description), but the surrounding test names/comments were not updated accordingly.

### Fix Focus Areas
- e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts[129-137]
- e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts[196-220]

### Suggested fix
- Update the affected `test("...")` titles (and nearby comments) to match what is actually asserted (e.g., replace "Already imported" with "Added" where appropriate), or revert the assertion text if the title/comment reflects the correct product behavior.

ⓘ 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

@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: failure.

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Fix E2E tests: Settings link, bulk-import, and FAB stability

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix Settings menuitem locator in profile dropdown for precise targeting
• Add optional chaining for environment variables to prevent runtime errors
• Replace presubmit check with nightly-only check for bulk-import tests
• Add retry logic with filterAndVerifyAddedRepo method for status assertions
• Fix FAB sub-menu click stability using dispatchEvent and visibility guard
Diagram
flowchart LR
  A["E2E Test Fixes"] --> B["Settings Menuitem Locator"]
  A --> C["Bulk-Import Tests"]
  A --> D["FAB Sub-Menu Click"]
  B --> B1["Use getByRole menuitem"]
  C --> C1["Optional Chaining for env vars"]
  C --> C2["Nightly-only check"]
  C --> C3["Retry with filterAndVerifyAddedRepo"]
  D --> D1["dispatchEvent click"]
  D --> D2["toBeVisible guard"]
Loading

Grey Divider

File Changes

1. e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts 🐞 Bug fix +20/-31

Bulk-import tests: env vars, checks, and retry logic

• Add optional chaining (?.) and nullish coalescing (??) for process.env.JOB_NAME and
 process.env.JOB_TYPE to prevent runtime errors
• Replace presubmit check with nightly-only check for test execution
• Replace direct filterAddedRepo() calls with new filterAndVerifyAddedRepo() method for improved
 retry logic
• Update expected status text from "Already imported" to "Added" in assertions
• Add refresh option support in filterAndVerifyAddedRepo calls for backend processing delays

e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts


2. e2e-tests/playwright/support/page-objects/global-fab-po.ts 🐞 Bug fix +3/-1

FAB menu click stability with visibility guard

• Add toBeVisible visibility check with 15-second timeout before clicking FAB menu
• Replace standard click() with dispatchEvent("click") to bypass actionability checks during
 animation
• Add visibility assertion in verifyFabButtonByLabel method

e2e-tests/playwright/support/page-objects/global-fab-po.ts


3. e2e-tests/playwright/support/pages/bulk-import.ts ✨ Enhancement +43/-0

Add filterAndVerifyAddedRepo method with retry logic

• Add UIhelper and Common utility imports and initialization in constructor
• Implement new filterAndVerifyAddedRepo() method with retry logic using expect().toPass()
• Support optional refresh button click after filtering for backend processing
• Handle multiple expected cell texts with visibility assertions
• Configure retry intervals and timeout for robust test execution

e2e-tests/playwright/support/pages/bulk-import.ts


View more (1)
4. e2e-tests/playwright/utils/ui-helper.ts 🐞 Bug fix +7/-5

Settings menuitem locator with precise targeting

• Replace generic clickLink("Settings") with scoped getByRole("menuitem", { name: "Settings" })
 locator
• Add visibility assertion before clicking Settings menuitem
• Update comment to clarify that Settings is a menuitem, not an anchor link
• Remove outdated translation-related code comments

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


Grey Divider

Qodo Logo

@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: failure.

@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from 669b504 to 75bbebd Compare April 16, 2026 09:13
@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: failure.

@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from 75bbebd to d53c380 Compare April 16, 2026 09:32
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: failure.

@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from d53c380 to e96c7a5 Compare April 16, 2026 09:51
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

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

@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

1 similar comment
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from 648bd76 to 9373afc Compare April 17, 2026 10:54
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@github-actions
Copy link
Copy Markdown
Contributor

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

…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
@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from 9373afc to 28aa9cf Compare April 19, 2026 14:13
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from 28aa9cf to 534f6d4 Compare April 19, 2026 14:28
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@zdrapela zdrapela force-pushed the fix/e2e-combined-fixes branch from 534f6d4 to 91df2ec Compare April 19, 2026 14:49
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: cancelled.

@github-actions
Copy link
Copy Markdown
Contributor

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

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 19, 2026

@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 91df2ec link true /test e2e-ocp-helm
ci/prow/e2e-ocp-helm-nightly 91df2ec link false /test e2e-ocp-helm-nightly

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.

…ency

Wrap the search + verify in expect().toPass() with retries to handle
GitHub API latency returning empty results or transitional status.

Assisted-by: OpenCode
@zdrapela
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant