Skip to content

Conversation

@gustavolira
Copy link
Member

Summary

This PR backports the changes from #3596 to release-1.7 to skip the Bulk Import test during PR checks (presubmit jobs) and limit execution to OCP jobs only to avoid GitHub API rate limiting.

Related Issue

Changes Made

Skip Bulk Import Test on PR Checks

Added conditional skips to bulk-import.spec.ts:

test.describe.serial("Bulk Import plugin", () => {
  test.skip(() => process.env.JOB_NAME.includes("osd-gcp")); // skipping due to RHIDP-5704 on OSD Env
  // TODO: https://issues.redhat.com/browse/RHDHBUGS-2116
+ test.fixme(() => process.env.JOB_TYPE.includes("presubmit")); // skip on PR checks
+ test.fixme(() => !process.env.JOB_NAME.includes("ocp")); // run only on OCP jobs to avoid GH rate limit
+ test.describe.configure({ retries: process.env.CI ? 5 : 0 });

Why these changes:

  1. Skip on PR checks (presubmit) - Bulk Import tests require GitHub API access and can hit rate limits during PR validation
  2. Run only on OCP jobs - Limits execution to OpenShift environments where the test is most reliable
  3. Configurable retries - Adds retry logic (5 retries on CI) to handle transient failures

Testing

Before

  • ❌ Bulk Import tests running on all PR checks
  • ❌ Tests failing due to GitHub API rate limits
  • ❌ Slowing down PR validation process

After

  • ✅ Bulk Import tests skipped on PR checks (presubmit)
  • ✅ Tests only run on OCP nightly jobs
  • ✅ Faster PR validation
  • ✅ Reduced GitHub API rate limit issues

Cherry-pick Details

This is a clean backport of commit e2f3dfe3 from #3596 to the release-1.7 branch.

Original commit: e2f3dfe3caf1be384640e50a190f9295c0c06ab5
Author: Zbyněk Drápela (@zdrapela)
Date: Oct 21, 2025

The changes were applied manually to avoid merge conflicts since the file structure differs slightly between main and release-1.7.

Verification Steps

1. PR Checks

# During PR validation, test should be skipped
process.env.JOB_TYPE === "presubmit"  # → test skipped ✅

2. OCP Nightly Jobs

# On OCP nightly jobs, test should run
process.env.JOB_NAME.includes("ocp")  # → test runs ✅

3. Other K8s Jobs

# On GKE/EKS/AKS jobs, test should be skipped
!process.env.JOB_NAME.includes("ocp")  # → test skipped ✅

Impact

  • PR validation: ⚡ Faster (bulk import test skipped)
  • Nightly OCP jobs: ✅ Test still runs (comprehensive coverage maintained)
  • GitHub API rate limits: 📉 Reduced pressure
  • Other K8s environments: ⚡ Tests skip to avoid rate limits

Checklist

Additional Notes

Why This Backport is Needed

The release-1.7 branch was experiencing issues with:

  1. Bulk Import tests consuming GitHub API quota during PR checks
  2. Tests failing intermittently due to rate limits
  3. Slower PR validation process

This backport brings the same improvements that were applied to main via #3596.

Differences from Original PR

The original PR #3596 also included changes to Quay tests, but those are not included in this backport as they are not relevant to release-1.7.

This PR only includes:

  • Bulk Import test skip logic

Original PR #3596 also included (not in this backport):

  • Quay test skip on 1.8

Files Changed:

  • e2e-tests/playwright/e2e/plugins/bulk-import.spec.ts (4 insertions)

Total: 1 file changed, 4 insertions(+)

@kim-tsao
Copy link
Member

/lgtm
/approve

@openshift-ci
Copy link

openshift-ci bot commented Nov 25, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kim-tsao

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

The pull request process is described 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

@openshift-ci openshift-ci bot removed the lgtm label Nov 26, 2025
@openshift-ci
Copy link

openshift-ci bot commented Nov 26, 2025

New changes are detected. LGTM label has been removed.

@github-actions
Copy link
Contributor

@gustavolira gustavolira force-pushed the skip-bulk-import-pr-checks-1.7 branch 2 times, most recently from 670ecf6 to 2b6b801 Compare November 27, 2025 17:57
@gustavolira
Copy link
Member Author

/test e2e-ocp-helm

@rhdh-qodo-merge
Copy link

You are above your monthly Qodo Merge usage quota. If you are a paying user, please link your GitHub/GitLab/Bitbucket account with your qodo account here to claim your seat. To allow usage organization-wide without linking, please reach to Qodo.

@gustavolira
Copy link
Member Author

/test e2e-ocp-helm

@rhdh-qodo-merge
Copy link

You are above your monthly Qodo Merge usage quota. If you are a paying user, please link your GitHub/GitLab/Bitbucket account with your qodo account here to claim your seat. To allow usage organization-wide without linking, please reach to Qodo.

@gustavolira
Copy link
Member Author

/test e2e-ocp-helm

@rhdh-qodo-merge
Copy link

You are above your monthly Qodo Merge usage quota. If you are a paying user, please link your GitHub/GitLab/Bitbucket account with your qodo account here to claim your seat. To allow usage organization-wide without linking, please reach to Qodo.

@gustavolira gustavolira force-pushed the skip-bulk-import-pr-checks-1.7 branch from 09bb439 to 02df2a4 Compare December 1, 2025 12:18
@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

Based on OpenShift CI Prow configuration analysis, JOB_TYPE is the
correct variable to detect presubmit jobs (PR checks).

Reference: ci-operator/step-registry/redhat-developer/rhdh/ocp/helm/refactored/redhat-developer-rhdh-ocp-helm-refactored-commands.sh:85
Use multiple conditions to detect PR checks:
- JOB_TYPE === 'presubmit' (Prow standard)
- PULL_NUMBER exists (fallback)
- JOB_NAME contains 'pull-ci-' (job name pattern)

Added debug logs to troubleshoot environment variable availability.
Based on actual CI logs, JOB_TYPE and PULL_NUMBER are NOT available
in the Playwright test environment. However, JOB_NAME IS available
and contains 'pull-ci-' for all PR check jobs.

Example from logs:
JOB_NAME: pull-ci-redhat-developer-rhdh-release-1.7-e2e-ocp-helm

This is the most reliable way to detect PR checks in this environment.
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

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.

@zdrapela
Copy link
Member

zdrapela commented Dec 5, 2025

/review
-i

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHDHBUGS-2116 - Partially compliant

Compliant requirements:

  • Prevent failing/flaky Bulk Import tests from breaking PR presubmit checks.
  • Limit execution to environments where the test is stable/reliable (OCP) to avoid GitHub API rate limits.
  • Add resiliency (retries) to mitigate intermittent failures.
  • Ensure CI configuration changes do not remove overall coverage (tests still run in OCP jobs).

Non-compliant requirements:

  • Reduce or eliminate flakiness in e2e/plugins/bulk-import.spec.ts causing timeouts (root-cause fix not included).

Requires further human verification:

  • Confirm that OCP nightly jobs execute the Bulk Import suite as intended post-merge.
  • Validate that PR presubmit pipelines reflect the skips and overall CI time improves.
  • Monitor for residual GitHub API rate limit issues in OCP jobs despite retries.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Environment Detection

The PR uses multiple heuristics to detect presubmit/PR jobs (JOB_NAME prefix, JOB_TYPE, PULL_NUMBER). Verify these env vars are consistently available across all CI variants to prevent false negatives/positives that could unintentionally run or skip tests.

const isPrJob =
  process.env.JOB_NAME?.startsWith("pull-ci-") ||
  process.env.JOB_TYPE === "presubmit" ||
  Boolean(process.env.PULL_NUMBER);

test.skip(isPrJob, "Skipping Bulk Import tests on PR (presubmit) jobs");
OCP-only Constraint

Skipping when JOB_NAME does not include ocp may inadvertently skip intended jobs if naming changes. Confirm job naming conventions are stable or consider a more robust flag for platform targeting.

test.skip(() => process.env.JOB_NAME?.includes("osd-gcp")); // skipping due to RHIDP-5704 on OSD Env
test.skip(() => !process.env.JOB_NAME?.includes("ocp")); // run only on OCP jobs to avoid GH rate limit
test.describe.configure({ retries: process.env.CI ? 5 : 0 });
Redundant Skips

There is a top-level test.skip(isPrJob, ...) and additional per-describe skips for certain environments. Ensure precedence does not mask needed runs or create confusion; consider centralizing skip logic to avoid redundancy.

test.skip(isPrJob, "Skipping Bulk Import tests on PR (presubmit) jobs");

// Pre-req : plugin-bulk-import & plugin-bulk-import-backend-dynamic
test.describe.serial("Bulk Import plugin", () => {
  test.skip(() => process.env.JOB_NAME?.includes("osd-gcp")); // skipping due to RHIDP-5704 on OSD Env
  test.skip(() => !process.env.JOB_NAME?.includes("ocp")); // run only on OCP jobs to avoid GH rate limit
  test.describe.configure({ retries: process.env.CI ? 5 : 0 });
📄 References
  1. No matching references available

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.

4 participants