Skip to content

feat: add additional orchestrator UI tests to CI #3996

Merged
gustavolira merged 1 commit into
redhat-developer:mainfrom
y-first:add-new-tests-to-ci
Feb 9, 2026
Merged

feat: add additional orchestrator UI tests to CI #3996
gustavolira merged 1 commit into
redhat-developer:mainfrom
y-first:add-new-tests-to-ci

Conversation

@y-first

@y-first y-first commented Jan 13, 2026

Copy link
Copy Markdown
Member

Description

Adds additional UI tests to the orchestrator test suite. Depends on previous PR

Which issue(s) does this PR fix

  • Fixes #?

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🔒 Security concerns

Command execution / insecure DB settings:
The Playwright test runs oc commands via execSync and interpolates process.env.RHDH_NAMESPACE into shell commands. If the environment value is malformed or attacker-controlled in CI, this could enable shell injection. Additionally, the pipeline patch sets QUARKUS_DATASOURCE_REACTIVE_TRUST_ALL=true (and SSL mode allow) which weakens TLS verification for the job service; ensure this is strictly limited to test/CI environments and cannot leak into production deployments.

⚡ Recommended focus areas for review

Test Stability

The test introduces multiple synchronous oc operations via execSync (patching, rollout restart, waiting for pods) inside a Playwright test. This can make the suite slow and flaky (cluster variance, transient failures) and also couples test results to cluster-admin-like side effects. Consider adding explicit error handling around each command, ensuring RHDH_NAMESPACE is validated, and adding cleanup/rollback guarantees if the test fails mid-way so subsequent tests are not impacted.

test("Test rerunning from failure point using failswitch workflow", async () => {
  const { execSync } = require('child_process');
  let correctValue = "quarkus.rest-client.httpbin_yaml.url=${HTTPBIN:https://httpbin.org/}";
  let patchIncorrectValue = execSync(`oc -n ${process.env.RHDH_NAMESPACE} patch sonataflow failswitch --type merge -p '{"spec": { "podTemplate": { "container": { "env": [{"name": "HTTPBIN",  "value": "https://httpbn.org/"}]}}}}'`).toString().trim();
  console.log(patchIncorrectValue);

  let reloadPod = execSync(`oc rollout restart deployment failswitch -n ${process.env.RHDH_NAMESPACE}`).toString().trim();
  console.log(reloadPod);

  let waitPodRunningResult = execSync(`oc wait --for=condition=ready pod -l app.kubernetes.io/name=failswitch -n ${process.env.RHDH_NAMESPACE} --timeout=120s`).toString().trim();
  console.log(waitPodRunningResult);

  await uiHelper.openSidebar("Orchestrator");
  await orchestrator.selectFailSwitchWorkflowItem();
  await orchestrator.runFailSwitchWorkflow("Wait");
  await orchestrator.validateWorkflowStatus("Failed");

  let patchCorrectValue = execSync(`oc -n ${process.env.RHDH_NAMESPACE} patch sonataflow failswitch --type merge -p '{"spec": { "podTemplate": { "container": { "env": [{"name": "HTTPBIN",  "value": "https://httpbin.org/"}]}}}}'`).toString().trim();
  console.log(patchCorrectValue);

  let reloadPodsResult = execSync(`oc rollout restart deployment failswitch -n ${process.env.RHDH_NAMESPACE}`).toString().trim();
  console.log(reloadPodsResult);

  let waitPodsRunningResult = execSync(`oc wait --for=condition=ready pod -l app.kubernetes.io/name=failswitch -n ${process.env.RHDH_NAMESPACE} --timeout=120s`).toString().trim();
  console.log(waitPodsRunningResult);

  await orchestrator.reRunOnFailure("From failure point");
  await orchestrator.validateWorkflowStatus("Completed");
});
Selector Robustness

validateWorkflowAllRunsStatusIcons relies on getByText(status) and asserts a specific CSS class value for icons. This is likely brittle (multiple matches, localization/case changes, CSS hash/class changes). Consider scoping selectors to the “all runs” table/row and using role/testid/aria attributes (or toBeVisible() on an icon element within the row) rather than asserting exact class names.

async validateWorkflowAllRunsStatusIcons() {
  await this.page.getByRole("tab", { name: "all runs" }).click();
  const statuses = ["Running", "Failed", "Completed", "Aborted"];
  for (const status of statuses) {
    await expect(this.page.getByText(status)).toHaveText(
      status,
    );
    await expect(this.page.getByText(status)).toHaveClass(/MuiSvgIcon-root MuiSvgIcon-fontSizeMedium css-1d3z3hw/);
  }
}
Abort Flow Bug

abortWorkflow now clicks the Abort button twice in a row (two .click() calls). If the UI shows a confirmation dialog, the first click may open it and the second should target the dialog’s confirm button instead of the same selector; if there is no confirmation, this double-click can create inconsistent behavior. Also, the new assertions for “Run has aborted” / “-- Aborted” should be validated against the actual UI text and scoped to avoid accidental matches.

async abortWorkflow() {
  await expect(
    this.page.getByRole("button", { name: "Abort" }),
  ).toBeEnabled();
  await this.page.getByRole("button", { name: "Abort" }).click();    
  await this.page.getByRole("button", { name: "Abort" }).click();
  await expect(this.page.getByText("Run has aborted")).toBeVisible();
  await expect(this.page.getByText("-- Aborted")).toBeVisible();
}
📄 References
  1. No matching references available

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Type

Tests, Enhancement


Description

  • Add comprehensive Playwright E2E tests for failswitch workflow scenarios

  • Implement new test helper methods for failswitch workflow interactions

  • Update orchestrator deployment to use failswitch workflow instead of user-onboarding

  • Fix PostgreSQL SSL configuration and deployment namespace handling


File Walkthrough

Relevant files
Tests
failswitch-workflow.spec.ts
Add failswitch workflow E2E test suite                                     

e2e-tests/playwright/e2e/plugins/orchestrator/failswitch-workflow.spec.ts

  • New test file with 5 comprehensive test cases for failswitch workflow
  • Tests cover workflow execution, abort functionality, status
    validation, and failure recovery
  • Includes Kubernetes CLI operations to patch workflow configurations
    and validate pod states
  • Tests validate workflow status transitions (Running, Completed,
    Failed, Aborted)
+94/-0   
orchestrator.ts
Add failswitch workflow test helper methods                           

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

  • Add validateWorkflowAllRunsStatusIcons() method to verify status icons
    in all runs tab
  • Refactor abortWorkflow() method with updated assertions for abort
    confirmation messages
  • Add selectFailSwitchWorkflowItem() method to navigate to failswitch
    workflow
  • Add runFailSwitchWorkflow() method with input parameter handling for
    workflow execution
  • Add validateWorkflowStatus() method with configurable timeout for
    status verification
  • Add reRunFailSwitchWorkflow() and reRunOnFailure() methods for
    workflow re-execution scenarios
+69/-9   
Configuration changes
utils.sh
Update orchestrator workflow deployment configuration       

.ibm/pipelines/utils.sh

  • Update PostgreSQL SSL configuration from verbose parameters to
    simplified QUARKUS_DATASOURCE_REACTIVE_POSTGRESQL_SSL_MODE
  • Change workflow repository from rhdh-orchestrator-test to
    rhdhorchestrator organization
  • Replace user-onboarding workflow with failswitch workflow in
    deployment functions
  • Add namespace parameter to oc apply command for workflow manifests
  • Add explicit deployment status checks and pod readiness waits for both
    greeting and failswitch workflows
  • Remove user-onboarding secret patching logic from operator deployment
    function
+18/-49 

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix redundant click in abort workflow

Fix the abortWorkflow function by restoring the correct logic for handling the
confirmation dialog. The current implementation clicks the "Abort" button twice
in a row, which is incorrect.

e2e-tests/playwright/support/pages/orchestrator.ts [196-204]

 async abortWorkflow() {
   await expect(
     this.page.getByRole("button", { name: "Abort" }),
   ).toBeEnabled();
-  await this.page.getByRole("button", { name: "Abort" }).click();    
   await this.page.getByRole("button", { name: "Abort" }).click();
+  await expect(
+    this.page
+      .getByRole("dialog")
+      .filter({ hasText: "Are you sure you want to abort this run?" }),
+  ).toBeVisible();
+  await this.page.getByRole("dialog").getByRole("button", { name: "Abort" }).click();
   await expect(this.page.getByText("Run has aborted")).toBeVisible();
   await expect(this.page.getByText("-- Aborted")).toBeVisible();
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug introduced in the PR where the "Abort" button is clicked twice consecutively, which will likely cause the test to fail, and proposes a fix that restores the correct logic of handling a confirmation dialog.

Medium
General
Check icon presence not CSS classes

In validateWorkflowAllRunsStatusIcons, replace the assertion against brittle,
auto-generated CSS class names with a more stable check for the presence of an
SVG icon within the status cell.

e2e-tests/playwright/support/pages/orchestrator.ts [165-174]

 async validateWorkflowAllRunsStatusIcons() {
   await this.page.getByRole("tab", { name: "all runs" }).click();
   const statuses = ["Running", "Failed", "Completed", "Aborted"];
   for (const status of statuses) {
-    await expect(this.page.getByText(status)).toHaveText(
-      status,
-    );
-    await expect(this.page.getByText(status)).toHaveClass(/MuiSvgIcon-root MuiSvgIcon-fontSizeMedium css-1d3z3hw/);
+    const row = this.page.locator("tbody tr").filter({ hasText: status });
+    await expect(row.getByText(status)).toBeVisible();
+    // Verify an icon SVG is present in the status cell
+    await expect(row.locator("svg")).toBeVisible();
   }
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that testing against auto-generated CSS class names is brittle and proposes a more robust alternative of checking for the SVG element's presence, improving test stability.

Medium
Refactor shell commands into helper

Refactor the execSync calls in the test into a reusable helper function to
improve code organization. Also, remove the unused correctValue variable.

e2e-tests/playwright/e2e/plugins/orchestrator/failswitch-workflow.spec.ts [65-93]

+import { execSync } from 'child_process';
+
+function patchAndReload(httpbinUrl: string) {
+  const namespace = process.env.RHDH_NAMESPACE;
+  execSync(`oc -n ${namespace} patch sonataflow failswitch --type merge -p '{"spec": { "podTemplate": { "container": { "env": [{"name": "HTTPBIN",  "value": "${httpbinUrl}"}]}}}}'`);
+  execSync(`oc rollout restart deployment failswitch -n ${namespace}`);
+  execSync(`oc wait --for=condition=ready pod -l app.kubernetes.io/name=failswitch -n ${namespace} --timeout=120s`);
+}
+
 test("Test rerunning from failure point using failswitch workflow", async () => {
-  const { execSync } = require('child_process');
-  let correctValue = "quarkus.rest-client.httpbin_yaml.url=${HTTPBIN:https://httpbin.org/}";
-  let patchIncorrectValue = execSync(`oc -n ${process.env.RHDH_NAMESPACE} patch sonataflow failswitch --type merge -p '{"spec": { "podTemplate": { "container": { "env": [{"name": "HTTPBIN",  "value": "https://httpbn.org/"}]}}}}'`).toString().trim();
-  console.log(patchIncorrectValue);
-
-  let reloadPod = execSync(`oc rollout restart deployment failswitch -n ${process.env.RHDH_NAMESPACE}`).toString().trim();
-  console.log(reloadPod);
-
-  let waitPodRunningResult = execSync(`oc wait --for=condition=ready pod -l app.kubernetes.io/name=failswitch -n ${process.env.RHDH_NAMESPACE} --timeout=120s`).toString().trim();
-  console.log(waitPodRunningResult);
+  patchAndReload("https://httpbn.org/");
 
   await uiHelper.openSidebar("Orchestrator");
   await orchestrator.selectFailSwitchWorkflowItem();
   await orchestrator.runFailSwitchWorkflow("Wait");
   await orchestrator.validateWorkflowStatus("Failed");
 
-  let patchCorrectValue = execSync(`oc -n ${process.env.RHDH_NAMESPACE} patch sonataflow failswitch --type merge -p '{"spec": { "podTemplate": { "container": { "env": [{"name": "HTTPBIN",  "value": "https://httpbin.org/"}]}}}}'`).toString().trim();
-  console.log(patchCorrectValue);
-
-  let reloadPodsResult = execSync(`oc rollout restart deployment failswitch -n ${process.env.RHDH_NAMESPACE}`).toString().trim();
-  console.log(reloadPodsResult);
-
-  let waitPodsRunningResult = execSync(`oc wait --for=condition=ready pod -l app.kubernetes.io/name=failswitch -n ${process.env.RHDH_NAMESPACE} --timeout=120s`).toString().trim();
-  console.log(waitPodsRunningResult);
+  patchAndReload("https://httpbin.org/");
 
   await orchestrator.reRunOnFailure("From failure point");
   await orchestrator.validateWorkflowStatus("Completed");
 });
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion proposes a valid refactoring to improve code quality by encapsulating shell commands in a helper function, which enhances readability and maintainability.

Low
  • More

@github-actions

Copy link
Copy Markdown
Contributor

The image is available at:

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

The image is available at:

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days.

@y-first

y-first commented Jan 22, 2026

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot removed the Stale label Jan 23, 2026
@y-first

y-first commented Jan 26, 2026

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm

@y-first y-first force-pushed the add-new-tests-to-ci branch from 155fc82 to c41e4bd Compare February 9, 2026 14:12
@y-first

y-first commented Feb 9, 2026

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@rhdh-qodo-merge

Copy link
Copy Markdown
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan
ⓘ Paying users. Check that your Qodo account is linked with this Git user account

@y-first y-first force-pushed the add-new-tests-to-ci branch 2 times, most recently from bc1b60d to 1ba2a4a Compare February 9, 2026 15:09
@gustavolira

Copy link
Copy Markdown
Member

/approve
/lgtm

@y-first y-first force-pushed the add-new-tests-to-ci branch from 1ba2a4a to 3af8e85 Compare February 9, 2026 15:18
@y-first

y-first commented Feb 9, 2026

Copy link
Copy Markdown
Member Author

/test e2e-ocp-helm-nightly

@rhdh-qodo-merge

Copy link
Copy Markdown
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan
ⓘ Paying users. Check that your Qodo account is linked with this Git user account

@sonarqubecloud

sonarqubecloud Bot commented Feb 9, 2026

Copy link
Copy Markdown

@y-first y-first requested a review from gustavolira February 9, 2026 15:30
@github-actions

github-actions Bot commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

@openshift-ci

openshift-ci Bot commented Feb 9, 2026

Copy link
Copy Markdown

@y-first: 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 3af8e85 link unknown /test e2e-ocp-helm
ci/prow/e2e-ocp-helm-nightly 3af8e85 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.

@gustavolira

Copy link
Copy Markdown
Member

/approve
/lgtm

@openshift-ci

openshift-ci Bot commented Feb 9, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gustavolira

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

@gustavolira gustavolira enabled auto-merge (squash) February 9, 2026 17:47
@gustavolira gustavolira merged commit 5afc370 into redhat-developer:main Feb 9, 2026
10 of 13 checks passed
@y-first y-first deleted the add-new-tests-to-ci branch February 9, 2026 18:20
@y-first y-first restored the add-new-tests-to-ci branch February 12, 2026 14:10
y-first added a commit to y-first/rhdh that referenced this pull request Feb 12, 2026
@y-first

y-first commented Feb 12, 2026

Copy link
Copy Markdown
Member Author

/cherry-pick release-1.9

@openshift-cherrypick-robot

Copy link
Copy Markdown
Contributor

@y-first: only redhat-developer org members may request cherry picks. If you are already part of the org, make sure to change your membership to public. Otherwise you can still do the cherry-pick manually.

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.

gustavolira added a commit that referenced this pull request Feb 12, 2026
Backport of #3738 and #3996 from main, adapted to the release-1.9 CI
structure (keeping utils.sh instead of lib/orchestrator.sh).

Changes:
- Migrate orchestrator from user-onboarding to failswitch workflow
- Update workflow repo from rhdh-orchestrator-test to rhdhorchestrator
- Add failswitch-workflow.spec.ts with comprehensive UI tests
- Update orchestrator page object with new failswitch methods
- Re-enable greeting workflow tests
- Add executeCommandWithRetries to log-utils
- Add orchestrator test skip logic for OSD-GCP and PR OCP helm jobs
- Update sfp patch env vars in rbac_deployment

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gustavolira added a commit that referenced this pull request Feb 13, 2026
…4246)

* feat: backport orchestrator failswitch workflow tests to release-1.9

Backport of #3738 and #3996 from main, adapted to the release-1.9 CI
structure (keeping utils.sh instead of lib/orchestrator.sh).

Changes:
- Migrate orchestrator from user-onboarding to failswitch workflow
- Update workflow repo from rhdh-orchestrator-test to rhdhorchestrator
- Add failswitch-workflow.spec.ts with comprehensive UI tests
- Update orchestrator page object with new failswitch methods
- Re-enable greeting workflow tests
- Add executeCommandWithRetries to log-utils
- Add orchestrator test skip logic for OSD-GCP and PR OCP helm jobs
- Update sfp patch env vars in rbac_deployment

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: wait for SonataFlow operator reconciliation before checking rollout

After patching a SonataFlow CR, the operator needs time to reconcile and
create the deployment. Without waiting, `oc rollout status` fails with
"deployments.apps not found".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use lower_case naming for local variables in shell functions

Fixes Sonar code smell violations for local variable naming convention.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

3 participants