Skip to content

Conversation

@zdrapela
Copy link
Member

@zdrapela zdrapela commented Dec 3, 2025

Description

This PR refactors the CI pipeline scripts to decouple the Kubernetes namespace from the Playwright project name, centralizes project names in a single source of truth, and fixes status reporting for the showcase-runtime test scenario.

This enables more flexible use of namespaces and test projects, allowing them to be configured and reused independently.

Changes

1. Decouple namespace from Playwright project name

Previously, the CI scripts implicitly tied the namespace name to the Playwright project name (e.g., namespace showcase-ci-nightly always ran project showcase). This created unnecessary coupling and prevented flexible reuse of namespaces with different test projects.

Now, check_and_test() and run_tests() accept an explicit playwright_project argument separate from the namespace, allowing:

  • Same namespace to run different test projects
  • Same test project to run in different namespaces
  • Independent configuration of deployment target and test suite

Tests are executed directly via npx playwright test --project="${playwright_project}" following Playwright's recommended approach.

2. Single source of truth for Playwright project names

Created e2e-tests/playwright/projects.json as the single source of truth for all Playwright project names. This file is consumed by:

  • TypeScript (playwright.config.ts) via e2e-tests/playwright/projects.ts import
  • CI/CD scripts via .ibm/pipelines/playwright-projects.sh which exports $PW_PROJECT_* environment variables

All CI job scripts now use these variables (e.g., ${PW_PROJECT_SHOWCASE}) instead of hardcoded strings.

3. Fix status saving for showcase-runtime

Moved the deployment status tracking (CURRENT_DEPLOYMENT increment and save_status_deployment_namespace) to the beginning of run_tests() instead of only in check_and_test(). This ensures tests like showcase-runtime that call run_tests() directly correctly report their deployment status.

4. Clean up e2e-tests/package.json

Removed redundant *-nightly and *-ci-nightly yarn script aliases that were only used for CI naming conventions.

Which issue(s) does this PR fix

  • Decouples namespace from Playwright project name for flexible reuse
  • Centralizes Playwright project names in a single source of truth
  • Fixes status reporting for showcase-runtime in the fine-grained CI reporter

PR acceptance criteria

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary

How to test changes / Special notes to the reviewer

  1. Key decoupling: Namespace and Playwright project are now independent:

    # Old: namespace implicitly determined test project
    check_and_test "${RELEASE_NAME}" "${NAMESPACE}" "${URL}"
    
    # New: namespace and project are explicitly separate
    check_and_test "${RELEASE_NAME}" "${NAMESPACE}" "${PW_PROJECT_SHOWCASE}" "${URL}"
  2. Files modified:

    • .ibm/pipelines/utils.sh - Core functions updated with explicit project argument
    • .ibm/pipelines/jobs/*.sh - All job handlers updated
    • e2e-tests/playwright/projects.json - Single source of truth for project names
    • e2e-tests/playwright/projects.ts - TypeScript exports
    • .ibm/pipelines/playwright-projects.sh - Shell script to load project names as env vars
    • e2e-tests/package.json - Removed redundant script aliases

@openshift-ci
Copy link

openshift-ci bot commented Dec 3, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@zdrapela
Copy link
Member Author

zdrapela commented Dec 3, 2025

/review
-i

@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
⚡ Recommended focus areas for review

Possible Issue

The function signature for check_and_test changed to accept playwright_project and shifted argument positions. Verify all call sites were updated consistently across the repo; a missed site would swap URL/attempts args or omit the project, causing failures or incorrect wait timings.

check_and_test() {
  local release_name=$1
  local namespace=$2
  local playwright_project=$3
  local url=$4
  local max_attempts=${5:-30} # Default to 30 if not set
  local wait_seconds=${6:-30} # Default to 30 if not set

  if check_backstage_running "${release_name}" "${namespace}" "${url}" "${max_attempts}" "${wait_seconds}"; then
    echo "Display pods for verification..."
    oc get pods -n "${namespace}"
    run_tests "${release_name}" "${namespace}" "${playwright_project}" "${url}"
  else
    echo "Backstage is not running. Exiting..."
    CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))
    save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace"
    save_status_failed_to_deploy $CURRENT_DEPLOYMENT true
    save_status_test_failed $CURRENT_DEPLOYMENT true
    save_overall_result 1
  fi
Artifact Paths

Artifact directories switched from using project to namespace. Ensure namespaces are unique per run and filesystem-safe; otherwise artifacts from concurrent runs can collide or be overwritten.

# Use namespace for artifact directory to keep artifacts organized by deployment
mkdir -p "${ARTIFACT_DIR}/${namespace}/test-results"
mkdir -p "${ARTIFACT_DIR}/${namespace}/attachments/screenshots"
cp -a "${e2e_tests_dir}/test-results/"* "${ARTIFACT_DIR}/${namespace}/test-results" || true
cp -a "${e2e_tests_dir}/${JUNIT_RESULTS}" "${ARTIFACT_DIR}/${namespace}/${JUNIT_RESULTS}" || true

cp -a "${e2e_tests_dir}/screenshots/"* "${ARTIFACT_DIR}/${namespace}/attachments/screenshots/" || true

ansi2html < "/tmp/${LOGFILE}" > "/tmp/${LOGFILE}.html"
cp -a "/tmp/${LOGFILE}.html" "${ARTIFACT_DIR}/${namespace}" || true
cp -a "${e2e_tests_dir}/playwright-report/"* "${ARTIFACT_DIR}/${namespace}" || true

save_data_router_junit_results "${namespace}"

echo "Playwright project '${playwright_project}' in namespace '${namespace}' RESULT: ${RESULT}"
if [ "${RESULT}" -ne 0 ]; then
📚 Focus areas based on broader codebase context

Namespace Logging

The updated test runner organizes artifacts by namespace but does not log or validate namespace existence before usage. Add a check/creation step and explicit logging, similar to other scripts that capture namespaces up-front, to avoid mkdir/copy failures when namespaces are unset or invalid. (Ref 6, Ref 8)

prepare_operator_app_config() {
  local config_file=$1
  yq e -i '.permission.rbac.conditionalPoliciesFile = "./rbac/conditional-policies.yaml"' ${config_file}
}

run_tests() {
  local release_name=$1
  local namespace=$2
  local playwright_project=$3
  local url="${4:-}"

  CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))
  save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace"
  save_status_failed_to_deploy $CURRENT_DEPLOYMENT false

  BASE_URL="${url}"
  export BASE_URL
  echo "BASE_URL: ${BASE_URL}"
  echo "Running Playwright project '${playwright_project}' against namespace '${namespace}'"

  cd "${DIR}/../../e2e-tests"
  local e2e_tests_dir
  e2e_tests_dir=$(pwd)

  yarn install --immutable > /tmp/yarn.install.log.txt 2>&1

  INSTALL_STATUS=$?
  if [ $INSTALL_STATUS -ne 0 ]; then
    echo "=== YARN INSTALL FAILED ==="
    cat /tmp/yarn.install.log.txt
    exit $INSTALL_STATUS
  else
    echo "Yarn install completed successfully."
  fi

  yarn playwright install chromium

  Xvfb :99 &
  export DISPLAY=:99

  (
    set -e
    echo "Using PR container image: ${TAG_NAME}"
    # Run Playwright directly with --project flag instead of using yarn script aliases
    npx playwright test --project="${playwright_project}"
  ) 2>&1 | tee "/tmp/${LOGFILE}"

  local RESULT=${PIPESTATUS[0]}

  pkill Xvfb

  # Use namespace for artifact directory to keep artifacts organized by deployment
  mkdir -p "${ARTIFACT_DIR}/${namespace}/test-results"
  mkdir -p "${ARTIFACT_DIR}/${namespace}/attachments/screenshots"
  cp -a "${e2e_tests_dir}/test-results/"* "${ARTIFACT_DIR}/${namespace}/test-results" || true
  cp -a "${e2e_tests_dir}/${JUNIT_RESULTS}" "${ARTIFACT_DIR}/${namespace}/${JUNIT_RESULTS}" || true

  cp -a "${e2e_tests_dir}/screenshots/"* "${ARTIFACT_DIR}/${namespace}/attachments/screenshots/" || true

  ansi2html < "/tmp/${LOGFILE}" > "/tmp/${LOGFILE}.html"
  cp -a "/tmp/${LOGFILE}.html" "${ARTIFACT_DIR}/${namespace}" || true
  cp -a "${e2e_tests_dir}/playwright-report/"* "${ARTIFACT_DIR}/${namespace}" || true

  save_data_router_junit_results "${namespace}"

  echo "Playwright project '${playwright_project}' in namespace '${namespace}' RESULT: ${RESULT}"
  if [ "${RESULT}" -ne 0 ]; then
    save_overall_result 1

Reference reasoning: Reference bash scripts consistently capture and validate namespace/workspace variables before proceeding and use clear, structured output and cleanup. Align by ensuring namespace variables are validated early and used consistently for file operations and logs.

📄 References
  1. redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/plugin-infra.sh [1-49]
  2. redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/plugin-infra.sh [50-75]
  3. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [1-55]
  4. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [71-103]
  5. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [159-190]
  6. redhat-developer/rhdh-chart/hack/merge_secrets.sh [1-47]
  7. redhat-developer/rhdh-chart/hack/merge_secrets.sh [49-52]
  8. redhat-developer/rhdh-chart/hack/orchestrator-templates-setup.sh [1-57]

@zdrapela
Copy link
Member Author

zdrapela commented Dec 3, 2025

/test e2e-ocp-helm

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

@zdrapela
Copy link
Member Author

zdrapela commented Dec 3, 2025

/test e2e-ocp-helm

@gustavolira
Copy link
Member

Consider updating documentation in .cursor/rules/ci-e2e-testing.mdc that references the old script names (showcase-ci-nightly, etc.) in a follow-up PR.

local playwright_project=$3
local url="${4:-}"

CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))
Copy link
Member

Choose a reason for hiding this comment

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

nit: 💡 Suggestion: Consider adding a comment explaining when CURRENT_DEPLOYMENT is incremented:

CURRENT_DEPLOYMENT tracking:

  • Incremented here (run_tests) for successful test executions
  • Incremented in check_and_test() error path for deployment failures
  • Incremented in check_upgrade_and_test() error path for upgrade failures
    CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))

This helps future maintainers understand the reporting flow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, you're right. I'm now thinking that it probably makes sense to open a Jira to refactor how the deployments are done, and that would result in the deployment and status saving being self-explanatory 🤔 Rather than describing the current juggling 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Can this be incorporated in https://issues.redhat.com/browse/RHIDP-11006 ? 🤔


initiate_aks_helm_deployment
check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "https://${K8S_CLUSTER_ROUTER_BASE}" 50 30
check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "showcase-k8s" "https://${K8S_CLUSTER_ROUTER_BASE}" 50 30
Copy link
Member

Choose a reason for hiding this comment

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

The Playwright project names are now hardcoded across multiple job files. Consider defining them as constants in env_variables.sh:

PLAYWRIGHT_PROJECT_K8S="showcase-k8s"
PLAYWRIGHT_PROJECT_RBAC_K8S="showcase-rbac-k8s"
PLAYWRIGHT_PROJECT_OPERATOR="showcase-operator"
PLAYWRIGHT_PROJECT_OPERATOR_RBAC="showcase-operator-rbac"
PLAYWRIGHT_PROJECT_RUNTIME="showcase-runtime"
PLAYWRIGHT_PROJECT_UPGRADE="showcase-upgrade"

Then use something like:
check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PLAYWRIGHT_PROJECT_K8S}"

Copy link
Member Author

@zdrapela zdrapela Dec 4, 2025

Choose a reason for hiding this comment

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

I considered doing this, but then I thought that it might be more readable to have it configured directly.
Having too many global variables makes it less transparent 🤔
But I don't have a strong opinion here 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway, I refactored it further to use the variables for both Bash and Playwright Typescript.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

@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
⚡ Recommended focus areas for review

Possible Issue

In check_and_test() and check_upgrade_and_test(), CURRENT_DEPLOYMENT and deployment namespace are saved only on failure paths; run_tests() now increments CURRENT_DEPLOYMENT and saves namespace on success paths. Verify that each call site results in exactly one increment and consistent namespace/status saving in both success and failure cases, avoiding skipped increments or double-counting across mixed code paths.

check_and_test() {
  local release_name=$1
  local namespace=$2
  local playwright_project=$3
  local url=$4
  local max_attempts=${5:-30} # Default to 30 if not set
  local wait_seconds=${6:-30} # Default to 30 if not set

  if check_backstage_running "${release_name}" "${namespace}" "${url}" "${max_attempts}" "${wait_seconds}"; then
    echo "Display pods for verification..."
    oc get pods -n "${namespace}"
    run_tests "${release_name}" "${namespace}" "${playwright_project}" "${url}"
  else
    echo "Backstage is not running. Exiting..."
    CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))
    save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace"
    save_status_failed_to_deploy $CURRENT_DEPLOYMENT true
    save_status_test_failed $CURRENT_DEPLOYMENT true
    save_overall_result 1
  fi
  save_all_pod_logs $namespace
}

check_upgrade_and_test() {
  local deployment_name="$1"
  local release_name="$2"
  local namespace="$3"
  local playwright_project="$4"
  local url=$5
  local timeout=${6:-600} # Timeout in seconds (default: 600 seconds)

  if check_helm_upgrade "${deployment_name}" "${namespace}" "${timeout}"; then
    check_and_test "${release_name}" "${namespace}" "${playwright_project}" "${url}"
  else
    echo "Helm upgrade encountered an issue or timed out. Exiting..."
    CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))
    save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace"
    save_status_failed_to_deploy $CURRENT_DEPLOYMENT true
    save_status_test_failed $CURRENT_DEPLOYMENT true
    save_overall_result 1
  fi
Robustness

The grep-based validation of the Playwright project assumes a specific formatting in playwright.config.ts and returns 1 immediately on mismatch, which marks deployment as failed in callers. Consider making this check non-fatal or more resilient to formatting/whitespace differences, or log a warning while still running to avoid false negatives.

if ! grep -q "name: \"${playwright_project}\"" "${DIR}/../../e2e-tests/playwright.config.ts" 2> /dev/null; then
  logging::warn "Playwright project '${playwright_project}' may not exist in config"
  return 1
fi

CURRENT_DEPLOYMENT=$((CURRENT_DEPLOYMENT + 1))
save_status_deployment_namespace $CURRENT_DEPLOYMENT "$namespace"
save_status_failed_to_deploy $CURRENT_DEPLOYMENT false
📄 References
  1. redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/plugin-infra.sh [1-49]
  2. redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/plugin-infra.sh [50-75]
  3. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [1-55]
  4. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [104-145]
  5. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [146-157]
  6. redhat-developer/rhdh-operator/hack/validate-image-digests.sh [159-190]
  7. redhat-developer/rhdh-chart/hack/merge_secrets.sh [1-47]
  8. redhat-developer/rhdh-chart/hack/merge_secrets.sh [49-52]

@rhdh-qodo-merge
Copy link

PR Type

Enhancement


Description

  • Decouple Kubernetes namespace from Playwright project name in CI pipeline

    • check_and_test() and run_tests() now accept explicit playwright_project argument
    • Tests executed directly via npx playwright test --project=<name> instead of yarn aliases
  • Fix status reporting for showcase-runtime test scenario

    • Move deployment status tracking to beginning of run_tests() function
    • Ensures all test scenarios correctly report deployment status to fine-grained reporter
  • Update all CI job handlers with explicit Playwright project names

    • 10 job files updated with new function signatures across AKS, EKS, GKE, OCP deployments
  • Clean up redundant yarn script aliases in e2e-tests/package.json

    • Remove *-nightly and *-ci-nightly aliases no longer needed by CI pipeline
  • Update documentation and rules to reflect new CI execution approach


File Walkthrough

Relevant files
Enhancement
12 files
utils.sh
Refactor test execution functions with explicit Playwright project
argument
+38/-23 
aks-helm.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
aks-operator.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
auth-providers.sh
Update run_tests call with explicit Playwright project name
+1/-1     
eks-helm.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
eks-operator.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
gke-helm.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
gke-operator.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
ocp-nightly.sh
Update test execution calls with explicit Playwright project names
+4/-4     
ocp-operator.sh
Update test execution calls with explicit Playwright project names
+3/-3     
ocp-pull.sh
Update check_and_test calls with explicit Playwright project names
+2/-2     
upgrade.sh
Update check_upgrade_and_test call with explicit Playwright project
name
+1/-1     
Configuration changes
1 files
package.json
Remove redundant nightly and ci-nightly yarn script aliases
+5/-7     
Documentation
4 files
ci-e2e-testing.md
Document new CI pipeline execution approach and function signatures
+28/-10 
ci-e2e-testing.mdc
Document new CI pipeline execution approach and function signatures
+28/-10 
ci-e2e-testing.md
Document new CI pipeline execution approach and function signatures
+28/-10 
CI.md
Update documentation to reflect direct Playwright execution in CI
+4/-4     

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Dec 4, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Improve robustness of Playwright project check

Improve the robustness of the Playwright project check by using a more flexible
regular expression that handles variations in quoting and spacing in the
configuration file.

.ibm/pipelines/utils.sh [631-634]

-if ! grep -q "name: \"${playwright_project}\"" "${DIR}/../../e2e-tests/playwright.config.ts" 2> /dev/null; then
+if ! grep -qE "^\s*name:\s*['\"]${playwright_project}['\"],?\s*$" "${DIR}/../../e2e-tests/playwright.config.ts" 2> /dev/null; then
   logging::warn "Playwright project '${playwright_project}' may not exist in config"
   return 1
 fi
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the new grep check is brittle and provides a more robust regex, preventing potential failures from minor formatting changes in the config file.

Low
  • Update

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

@zdrapela zdrapela force-pushed the namespace-project-decouple branch from 4a1362b to ec72a2b Compare December 5, 2025 10:50
@zdrapela
Copy link
Member Author

zdrapela commented Dec 5, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

@gustavolira
Copy link
Member

/approve
/gtml

@gustavolira
Copy link
Member

/approve
/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Dec 5, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gustavolira
Once this PR has been reviewed and has the lgtm label, please assign jessicajhee for approval. For more information see the Code Review Process.

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

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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

@openshift-ci openshift-ci bot removed the lgtm label Dec 5, 2025
@openshift-ci
Copy link

openshift-ci bot commented Dec 5, 2025

New changes are detected. LGTM label has been removed.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

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.

2 participants