Skip to content

fix(e2e): add explicit visibility wait before clicking workflow links [release-1.9]#4530

Open
gustavolira wants to merge 4 commits intoredhat-developer:release-1.9from
gustavolira:fix/orchestrator-failswitch-race-condition-release-1.9
Open

fix(e2e): add explicit visibility wait before clicking workflow links [release-1.9]#4530
gustavolira wants to merge 4 commits intoredhat-developer:release-1.9from
gustavolira:fix/orchestrator-failswitch-race-condition-release-1.9

Conversation

@gustavolira
Copy link
Copy Markdown
Member

@gustavolira gustavolira commented Apr 2, 2026

Summary

  • Fixes race condition in selectFailSwitchWorkflowItem and selectGreetingWorkflowItem where the workflow table becomes visible but the specific workflow link hasn't rendered yet
  • Adds await expect(link).toBeVisible({ timeout: 30000 }) before clicking — same pattern introduced in main via test(e2e): add orchestrator RHDH entity workflow tests #4230
  • Addresses the failswitch-workflow.spec.ts tests timing out at ~50s in release-1.9

Root cause

After openSidebar("Orchestrator") navigates to the page, selectFailSwitchWorkflowItem verifies the Workflows heading and table are visible, but immediately clicks the workflow link before it finishes rendering. This causes a silent miss and the test times out waiting for the next step.

Test plan

  • Verify failswitch-workflow.spec.ts tests pass on release-1.9 CI runs
  • Confirm no regression in greeting-workflow.spec.ts

🤖 Generated with Claude Code

Race condition in selectFailSwitchWorkflowItem and selectGreetingWorkflowItem:
the table becomes visible but the specific workflow link may not yet be rendered,
causing the click to fail silently and tests to timeout after 50+ seconds.

Add explicit `await expect(link).toBeVisible({ timeout: 30000 })` before
clicking, matching the fix applied to main in redhat-developer#4494.

Ref: https://issues.redhat.com/browse/RHDHBUGS-2671

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rhdh-qodo-merge
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHDHBUGS-2671 - Partially compliant

Compliant requirements:

  • Address the race condition where the workflows table is visible but the specific workflow link is not yet rendered.

Non-compliant requirements:

  • Fix the E2E failure where the test times out waiting for the Workflows page heading to be visible.
  • Ensure navigation/selection of workflows is stable so the test does not end up on a workflow detail page unexpectedly.

Requires further human verification:

  • Verify e2e/plugins/orchestrator/orchestrator-rbac.spec.ts (global workflow read-only access / Run button disabled flow) passes reliably in CI.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Wait strategy

Waiting for toBeVisible({ timeout: 30000 }) helps, but a visible link may still be covered/disabled during re-render. A more robust pattern is to wait for the table to contain the row/link and then click with an assertion like toBeEnabled or use locator.click() with Playwright's built-in actionability checks plus an explicit await expect(locator).toBeAttached()/toBeEnabled() if needed.

  const failSwitchLink = this.page.getByRole("link", {
    name: "FailSwitch workflow",
  });
  await expect(failSwitchLink).toBeVisible({ timeout: 30000 });
  await failSwitchLink.click();
}
📚 Focus areas based on broader codebase context

Selector Robustness

The new locators use an exact accessible name match (name: "Greeting workflow") which is more brittle than patterns used elsewhere (case-insensitive regex). Consider switching to a regex-based name (e.g., /Greeting workflow/i) to better tolerate minor UI text/casing changes while keeping the explicit visibility wait. (Ref 1, Ref 2)

const greetingLink = this.page.getByRole("link", {
  name: "Greeting workflow",
});
await expect(greetingLink).toBeVisible({ timeout: 30000 });
await greetingLink.click();

Reference reasoning: Existing orchestrator e2e tests frequently locate the same UI elements using regex-based accessible names (e.g., /Greeting workflow/i, /greeting/i) before asserting visibility and clicking. Aligning the page-object locator strategy with those patterns improves consistency and reduces flakiness from small text variations.

📄 References
  1. redhat-developer/rhdh/e2e-tests/playwright/e2e/plugins/orchestrator/failswitch-workflow.spec.ts [9-139]
  2. redhat-developer/rhdh/e2e-tests/playwright/e2e/plugins/orchestrator/orchestrator-entity-workflows.spec.ts [29-299]

@rhdh-qodo-merge
Copy link
Copy Markdown

PR Type

Bug fix


Description

  • Fixes race condition in workflow link selection by adding explicit visibility waits

  • Applies 30-second timeout before clicking workflow links in two methods

  • Prevents test timeouts caused by premature clicks on unrendered elements


File Walkthrough

Relevant files
Bug fix
orchestrator.ts
Add visibility waits before workflow link clicks                 

e2e-tests/playwright/support/pages/orchestrator.ts

  • Added explicit await expect(greetingLink).toBeVisible({ timeout: 30000
    }) before clicking in selectGreetingWorkflowItem()
  • Added explicit await expect(failSwitchLink).toBeVisible({ timeout:
    30000 }) before clicking in selectFailSwitchWorkflowItem()
  • Refactored link selection to store reference in variable before
    visibility check and click
  • Ensures workflow links are fully rendered before interaction to
    prevent silent failures
+10/-2   

@gustavolira
Copy link
Copy Markdown
Member Author

/test ?

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Apr 2, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Refactor to reduce code duplication

Refactor the selectGreetingWorkflowItem and selectFailSwitchWorkflowItem methods
by extracting their common logic into a single, parameterized helper method to
reduce code duplication.

e2e-tests/playwright/support/pages/orchestrator.ts [20-269]

 async selectGreetingWorkflowItem() {
+  await this.selectWorkflowByName("Greeting workflow");
+}
+...
+async selectFailSwitchWorkflowItem() {
+  await this.selectWorkflowByName("FailSwitch workflow");
+}
+
+private async selectWorkflowByName(workflowName: string) {
   const workflowHeader = this.page.getByRole("heading", {
     name: "Workflows",
   });
   await expect(workflowHeader).toBeVisible();
   await expect(workflowHeader).toHaveText("Workflows");
   await expect(Workflows.workflowsTable(this.page)).toBeVisible();
-  const greetingLink = this.page.getByRole("link", {
-    name: "Greeting workflow",
+  const workflowLink = this.page.getByRole("link", {
+    name: workflowName,
   });
-  await expect(greetingLink).toBeVisible({ timeout: 30000 });
-  await greetingLink.click();
-}
-...
-async selectFailSwitchWorkflowItem() {
-  const workflowHeader = this.page.getByRole("heading", {
-    name: "Workflows",
-  });
-  await expect(workflowHeader).toBeVisible();
-  await expect(workflowHeader).toHaveText("Workflows");
-  await expect(Workflows.workflowsTable(this.page)).toBeVisible();
-  const failSwitchLink = this.page.getByRole("link", {
-    name: "FailSwitch workflow",
-  });
-  await expect(failSwitchLink).toBeVisible({ timeout: 30000 });
-  await failSwitchLink.click();
+  await expect(workflowLink).toBeVisible({ timeout: 30000 });
+  await workflowLink.click();
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies significant code duplication between two methods and proposes a valid refactoring using a helper method, which improves code maintainability and follows the DRY principle.

Low
Use click timeout instead of expect

Simplify the code by removing the expect(greetingLink).toBeVisible() call and
passing the timeout directly to greetingLink.click(), leveraging Playwright's
built-in waiting mechanism.

e2e-tests/playwright/support/pages/orchestrator.ts [27-31]

-const greetingLink = this.page.getByRole("link", {
-  name: "Greeting workflow",
-});
-await expect(greetingLink).toBeVisible({ timeout: 30000 });
-await greetingLink.click();
+const greetingLink = this.page.getByRole("link", { name: "Greeting workflow" });
+await greetingLink.click({ timeout: 30000 });
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out that an explicit expect().toBeVisible() is redundant as Playwright's click() action auto-waits for visibility, and proposes a valid simplification.

Low
  • Update

@gustavolira
Copy link
Copy Markdown
Member Author

/test e2e-ocp-v4-20-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

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

@gustavolira
Copy link
Copy Markdown
Member Author

/test e2e-ocp-v4-20-helm-nightly
/test e2e-ocp-helm

Add waitForWorkflowToAppear helper that uses short auto-wait checks
(3s) before reloading the page, avoiding unnecessary reloads while
handling the race condition where the backend hasn't registered
the workflow yet.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gustavolira
Copy link
Copy Markdown
Member Author

/test e2e-ocp-v4-20-helm-nightly

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

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

The SonataFlow operator + Data Index Service can take over a minute
to register workflows after a fresh deploy. Increase retry window
to 10 attempts with 5s auto-wait each + 30s final assertion, and
bump test timeout to 180s to accommodate the longer wait.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

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

@gustavolira
Copy link
Copy Markdown
Member Author

/test e2e-ocp-v4-20-helm-nightly

The Workflows.workflowsTable locator uses nth(2) on generic divs,
which breaks during page loading states after reload. Replace with
the stable "Workflows" heading locator inside waitForWorkflowToAppear
to prevent the retry loop from failing on transient DOM states.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

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

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 7, 2026

@gustavolira
Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 7, 2026

@gustavolira: 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-v4-20-helm-nightly 7d785a7 link false /test e2e-ocp-v4-20-helm-nightly
ci/prow/e2e-ocp-helm 00401a9 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.

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