Skip to content

Conversation

@HusneShabbir
Copy link
Contributor

@HusneShabbir HusneShabbir commented Feb 2, 2026

Adds a page load wait before assertions in adoption-insights tests to ensure data loads properly before validation.

Changes

  • Add explicit page load wait before running assertions

Why

Prevents flaky test failures caused by assertions running before data fully loads.

Resolves

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ No major issues detected
📚 Focus areas based on broader codebase context

Flaky Wait

The added await page.reload() is a broad synchronization mechanism that can introduce new flakiness (extra navigation, re-fetches, and timing differences) and may mask the real readiness condition for the assertions. Prefer waiting on the specific Adoption Insights API/panel calls (or a dedicated helper) before continuing, rather than reloading the page mid-test. (Ref 2, Ref 3)

await uiHelper.clickLink("Catalog");
await page.reload();
await testHelper.waitUntilApiCallSucceeds(page);
await uiHelper.openSidebarButton("Administration");

Reference reasoning: The existing Adoption Insights page helper includes targeted wait utilities (e.g., waiting until the API call succeeds / waiting for panel API calls) that are intended to synchronize the test with data readiness. Using these focused waits is more consistent with the current test/support design than forcing a full page.reload() to “wait for load.”

📄 References
  1. redhat-developer/rhdh/e2e-tests/playwright/support/pages/adoption-insights.ts [1-2]
  2. redhat-developer/rhdh/e2e-tests/playwright/support/pages/adoption-insights.ts [4-191]
  3. redhat-developer/rhdh/e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts [1-8]

@rhdh-qodo-merge
Copy link

PR Type

Bug fix


Description

  • Add page reload after navigating to Catalog

  • Open Administration sidebar before accessing Adoption Insights

  • Ensures proper page state and data loading before assertions


File Walkthrough

Relevant files
Bug fix
adoption-insights.spec.ts
Add page reload and sidebar navigation steps                         

e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts

  • Add page.reload() call after clicking Catalog link
  • Add uiHelper.openSidebarButton("Administration") call before Adoption
    Insights navigation
  • Ensures proper page state initialization before running assertions
+2/-0     

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2026

@rhdh-qodo-merge
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Replace page reload with explicit navigation wait

Replace the page.reload() with an explicit page.waitForURL('/catalog') to
robustly wait for navigation to complete, improving test reliability.**

e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts [149-151]

 await uiHelper.clickLink("Catalog");
-await page.reload();
+await page.waitForURL('**/catalog');
 await testHelper.waitUntilApiCallSucceeds(page);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that page.reload() is a code smell in tests and proposes a more robust alternative, page.waitForURL(), which improves test reliability by waiting for a specific navigation outcome.

Medium
Verify button visibility before click

Add an explicit expect(...).toBeVisible() check for the "Administration" button
before clicking it to prevent test flakiness.

e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts [152]

+await expect(page.getByRole('button', { name: 'Administration' })).toBeVisible();
 await uiHelper.openSidebarButton("Administration");
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion improves test robustness by adding an explicit visibility check before interacting with an element, which is a good practice to prevent flakiness, although the helper function might already include this check.

Low
  • More

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

@HusneShabbir
Copy link
Contributor Author

/retest

Copy link
Member

@karthikjeeyar karthikjeeyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/cherry-pick release-1.9

@openshift-ci
Copy link

openshift-ci bot commented Feb 3, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christoph-jerolimov, karthikjeeyar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@HusneShabbir
Copy link
Contributor Author

/retest

@HusneShabbir
Copy link
Contributor Author

@christoph-jerolimov @jrichter1 ##4166 can resolve the scorecard test failure that we just encountered in the previous run.

@christoph-jerolimov
Copy link
Member

/retest

@jrichter1
Copy link
Contributor

/test e2e-ocp-helm

@HusneShabbir
Copy link
Contributor Author

/retest

1 similar comment
@invincibleJai
Copy link
Member

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit 7c12113 into redhat-developer:main Feb 3, 2026
26 checks passed
@invincibleJai
Copy link
Member

/cherry-pick release-1.9

@openshift-cherrypick-robot
Copy link
Contributor

@invincibleJai: new pull request created: #4178

Details

In response to this:

/cherry-pick release-1.9

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.

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.

6 participants