Skip to content

Conversation

@teknaS47
Copy link
Member

Description

Adding localization tests for other locales to the ocp-helm-nightly job

Which issue(s) does this PR fix

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

Signed-off-by: Sanket Saikia <sanketsaikia13@gmail.com>
@openshift-ci openshift-ci bot requested review from debsmita1 and elsony January 19, 2026 12:58
@openshift-ci
Copy link

openshift-ci bot commented Jan 19, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found 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

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHIDP-11153 - Partially compliant

Compliant requirements:

  • Integrate translation end-to-end (e2e) tests into the existing nightly CI workflow (not regular CI) to maintain CI speed.
  • Translation e2e tests are executed as part of the existing nightly CI workflow (acceptance criteria).

Non-compliant requirements:

(empty)

Requires further human verification:

  • Confirm via CI run history that the nightly workflow actually executes the localization Playwright project and reports results (and that regular CI remains unaffected).
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Coverage Gap

The localization runner only invokes the French Playwright project. If the intent is to cover “other locales” (per PR description/branch name), confirm whether additional locale projects (e.g., Italian/Japanese) should also be executed or whether this PR intentionally limits scope to French only.

run_localization_tests() {
  local url="https://${RELEASE_NAME}-developer-hub-${NAME_SPACE}.${K8S_CLUSTER_ROUTER_BASE}"
  log::section "Running localization tests"
  # French (fr)
  check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}" "${url}"
}
Skip Logic

The skip condition relies on pattern-matching JOB_NAME for osd-gcp. Validate that JOB_NAME is always set in this environment and that the regex matches all intended OSD-GCP job name variants; otherwise localization tests might run unexpectedly or be skipped incorrectly.

# Skip localization tests for OSD-GCP jobs
if [[ ! "${JOB_NAME}" =~ osd-gcp ]]; then
  run_localization_tests
fi
📄 References
  1. redhat-developer/rhdh-operator/tests/e2e/README.md [1-12]
  2. redhat-developer/rhdh-chart/charts/orchestrator-infra/README.md [33-71]
  3. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/README.md [33-71]
  4. redhat-developer/rhdh-operator/tests/e2e/README.md [112-122]
  5. redhat-developer/rhdh-operator/tests/e2e/README.md [51-57]
  6. redhat-developer/rhdh-operator/tests/e2e/README.md [39-50]
  7. redhat-developer/rhdh-operator/tests/e2e/README.md [123-125]
  8. redhat-developer/rhdh-operator/tests/e2e/README.md [126-129]

@rhdh-qodo-merge
Copy link

PR Type

Enhancement, Tests


Description

  • Add localization tests for French language to OCP nightly jobs

  • Implement run_localization_tests() function with conditional execution

  • Skip localization tests for OSD-GCP environments

  • Update documentation across multiple memory and rules files

  • Fix terminology from "marketplace" to "extensions" in documentation


File Walkthrough

Relevant files
Enhancement
ocp-nightly.sh
Add localization test execution to nightly pipeline           

.ibm/pipelines/jobs/ocp-nightly.sh

  • Add conditional check to skip localization tests for OSD-GCP jobs
  • Implement new run_localization_tests() function
  • Function runs French localization tests against standard deployment
  • Uses existing check_and_test helper with localization project
+12/-0   
Documentation
add_extension_metadata.md
Fix directory naming in extension metadata docs                   

.claude/memories/add_extension_metadata.md

  • Fix documentation: change "marketplace directory" to "extensions
    directory"
+1/-1     
ci-e2e-testing.md
Document French localization tests in CI/E2E guide             

.claude/memories/ci-e2e-testing.md

  • Add showcase-localization-fr to list of Playwright test projects
  • Document new Localization Tests section with French language support
  • Add yarn command for running French localization tests
  • Update config map documentation to include localization project
+13/-0   
add_extension_metadata.mdc
Update extension metadata rules documentation                       

.cursor/rules/add_extension_metadata.mdc

  • Fix directory reference from "marketplace" to "extensions"
  • Update git commit message to reference "extensions" instead of
    "marketplace"
+2/-2     
ci-e2e-testing.mdc
Add localization tests to cursor rules documentation         

.cursor/rules/ci-e2e-testing.mdc

  • Add showcase-localization-fr to Playwright projects list
  • Document Localization Tests section with French language details
  • Add yarn command for French localization tests
  • Update config map references for localization project
+13/-0   
add_extension_metadata.md
Sync extension metadata rules documentation                           

.rulesync/rules/add_extension_metadata.md

  • Fix directory naming from "marketplace" to "extensions"
  • Update git commands to use correct extensions directory paths
  • Update commit message to reference "extensions"
+4/-4     
ci-e2e-testing.md
Sync CI/E2E testing rules documentation                                   

.rulesync/rules/ci-e2e-testing.md

  • Add showcase-localization-fr to test projects list
  • Document Localization Tests section with French language support
  • Add yarn command for running French localization tests
  • Update config map documentation for localization project
+13/-0   
CI.md
Document localization testing in CI documentation               

docs/e2e-tests/CI.md

  • Add new Localization Tests section documenting French language testing
  • Document supported languages and future expansion plans
  • Explain when tests run and skip conditions for OSD-GCP
  • Reference implementation location in nightly pipeline script
+12/-0   

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Jan 19, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Improve scalability for adding more locales

The script ocp-nightly.sh hardcodes the French localization test. It should be
refactored to iterate over a list of locales to make it more scalable for adding
new languages in the future.

Examples:

.ibm/pipelines/jobs/ocp-nightly.sh [63-68]
run_localization_tests() {
  local url="https://${RELEASE_NAME}-developer-hub-${NAME_SPACE}.${K8S_CLUSTER_ROUTER_BASE}"
  log::section "Running localization tests"
  # French (fr)
  check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}" "${url}"
}

Solution Walkthrough:

Before:

# .ibm/pipelines/jobs/ocp-nightly.sh

run_localization_tests() {
  local url="..."
  log::section "Running localization tests"

  # French (fr)
  check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}" "${url}"
}

After:

# .ibm/pipelines/jobs/ocp-nightly.sh

run_localization_tests() {
  local url="..."
  log::section "Running localization tests"

  declare -A LOCALES_TO_TEST=(
    ["fr"]="${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}"
    # Future locales can be added here
  )

  for locale in "${!LOCALES_TO_TEST[@]}"; do
    log::info "Running localization tests for ${locale}"
    check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${LOCALES_TO_TEST[$locale]}" "${url}"
  done
}
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a design limitation where the French locale test is hardcoded, and proposes a scalable, loop-based alternative that would significantly ease the addition of future languages, which is mentioned as a possibility in the PR's documentation.

Medium
General
Use wildcard matching for skip check

Replace the regex match !~ with the wildcard pattern != for checking the
JOB_NAME variable.

.ibm/pipelines/jobs/ocp-nightly.sh [37-39]

-if [[ ! "${JOB_NAME}" =~ osd-gcp ]]; then
+if [[ "${JOB_NAME}" != *osd-gcp* ]]; then
   run_localization_tests
 fi
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly proposes using a shell wildcard pattern, which is more idiomatic and readable for this simple substring check, though the existing regex is also correct.

Low
  • Update

@teknaS47
Copy link
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Contributor

The image is available at:

/test e2e-ocp-helm

@teknaS47
Copy link
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Contributor

The image is available at:

/test e2e-ocp-helm

@invincibleJai
Copy link
Member

@teknaS47
Copy link
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Contributor

The image is available at:

/test e2e-ocp-helm

@teknaS47
Copy link
Member Author

/test e2e-ocp-helm

Copy link
Member

@zdrapela zdrapela left a comment

Choose a reason for hiding this comment

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

The logic to run the tests looks good. We need to have CI fixed to verify everything works smoothly. I've seen failed tests in https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/redhat-developer_rhdh/4020/pull-ci-redhat-developer-rhdh-main-e2e-ocp-helm-nightly/2013461772190617600/artifacts/e2e-ocp-helm-nightly/redhat-developer-rhdh-ocp-helm-nightly/artifacts/showcase/index.html, but I'm not sure if it's caused by your test.

Anyway, I found some issues that are not a problem of this PR, but more of the code base. Please see my inline comments and let me know if you are up for fixing them or if I should rather take a look. I hope I explained myself well, but let me know if something is not clear 😉

### Step 4: Validate Files

```bash
# Navigate to marketplace directory
Copy link
Member

@zdrapela zdrapela Jan 21, 2026

Choose a reason for hiding this comment

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

Thank you for fixing the inconsistencies caused by the force merge of #3992!
The RuleSync check is now broken because of the update in #3793.
Feel free to fix it by running yarn rulesync:generate and committing the files generated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I’ve already regenerated the RuleSync files and committed the updates.

Copy link
Member

Choose a reason for hiding this comment

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

Please push the updates. I don't see them here yet

local url="https://${RELEASE_NAME}-developer-hub-${NAME_SPACE}.${K8S_CLUSTER_ROUTER_BASE}"
log::section "Running localization tests"
# French (fr)
check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}" "${url}"
Copy link
Member

Choose a reason for hiding this comment

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

I found an issue caused by my oversight in #3796 😅
The run_tests function is using namespace as a hardcoded folder to store artifacts in the root ARTIFACTS_DIR. This leads to overriding the results from the showcase project by the results of your localization tests. See https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/redhat-developer_rhdh/4020/pull-ci-redhat-developer-rhdh-main-e2e-ocp-helm-nightly/2013461772190617600/artifacts/e2e-ocp-helm-nightly/redhat-developer-rhdh-ocp-helm-nightly/artifacts/showcase/index.html

To fix that, I suggest doing something like this: zdrapela@faa4103 We want to keep the default value bound to the namespace, but in your case, we need to override it with PW_PROJECT_SHOWCASE_LOCALIZATION_FR - see my suggestion.

Suggested change
check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}" "${url}"
check_and_test "${RELEASE_NAME}" "${NAME_SPACE}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}" "${url}" "${PW_PROJECT_SHOWCASE_LOCALIZATION_FR}"

Are you up for fixing it in your PR, or should I open a separate one to fix it?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can fix it here, or another PR, anything is fine by me.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, please try it here and we will see if that works well

@github-actions
Copy link
Contributor

🚫 Image Push Skipped.

The container image push was skipped because the build was skipped (either due to [skip-build] tag or no relevant changes with existing image)

@teknaS47
Copy link
Member Author

/test e2e-ocp-helm-nightly

@teknaS47
Copy link
Member Author

/test e2e-ocp-helm-nightly

@github-actions
Copy link
Contributor

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

@zdrapela
Copy link
Member

/test e2e-ocp-helm-nightly

@openshift-ci
Copy link

openshift-ci bot commented Jan 22, 2026

@teknaS47: 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 8c54675 link true /test e2e-ocp-helm
ci/prow/e2e-ocp-helm-nightly 8c54675 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.

Copy link
Member

@zdrapela zdrapela left a comment

Choose a reason for hiding this comment

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

The PR looks good, but we need to wait for the fix of the PR checks to verify it.

Copy link
Member

@zdrapela zdrapela left a comment

Choose a reason for hiding this comment

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

The PR looks good, but we need to wait for the fix of the PR checks to verify it.

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