Skip to content

ci: run full e2e tests automatically on every PR#881

Closed
lionelvillard wants to merge 1 commit intollm-d:mainfrom
lionelvillard:ci-auto-e2e-full
Closed

ci: run full e2e tests automatically on every PR#881
lionelvillard wants to merge 1 commit intollm-d:mainfrom
lionelvillard:ci-auto-e2e-full

Conversation

@lionelvillard
Copy link
Copy Markdown
Collaborator

Summary

  • Remove the /trigger-e2e-full comment-gated mechanism for running full e2e tests
  • Run e2e-tests-full automatically alongside e2e-tests-smoke on every PR with code changes
  • Remove check-full-tests job, report-status job, issue_comment trigger, and all associated fork-PR workarounds
  • Simplify concurrency group and check-code-changes job

Test plan

  • Verify e2e-tests-smoke and e2e-tests-full both trigger on a PR with code changes
  • Verify neither e2e job triggers on a docs-only PR
  • Verify workflow_dispatch still works for manual runs

🤖 Generated with Claude Code

Remove the /trigger-e2e-full comment-gated mechanism and run e2e-tests-full
alongside e2e-tests-smoke on every PR with code changes. This removes the
check-full-tests job, report-status job, issue_comment trigger, and all
associated fork-PR workarounds.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Lionel Villard <villard@us.ibm.com>
Copilot AI review requested due to automatic review settings March 11, 2026 23:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the PR CI workflow to always run the full Kind e2e suite automatically (in addition to smoke) on PRs that include code changes, removing the prior comment-gated “full e2e” trigger mechanism and associated jobs/logic.

Changes:

  • Removes /trigger-e2e-full comment trigger, validation, and status-reporting jobs.
  • Runs e2e-tests-full automatically alongside e2e-tests-smoke when code changes are detected.
  • Simplifies workflow concurrency grouping and PR-change detection wiring.

Comment on lines 49 to 54
FILTER_OUTPUT="${{ steps.filter.outputs.code }}"
if [ "${{ github.event_name }}" == "issue_comment" ] && [ -z "$FILTER_OUTPUT" ]; then
echo "has_code_changes=true" >> $GITHUB_OUTPUT
elif [ -n "$FILTER_OUTPUT" ]; then
if [ -n "$FILTER_OUTPUT" ]; then
echo "has_code_changes=$FILTER_OUTPUT" >> $GITHUB_OUTPUT
else
echo "has_code_changes=true" >> $GITHUB_OUTPUT
fi
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

check-code-changes defaults has_code_changes to true when paths-filter produces no output (e.g., if the step fails; it's currently continue-on-error: true). That can unintentionally run the e2e jobs on docs-only PRs, defeating the optimization. Consider failing fast when change detection fails, or defaulting to false for pull_request events (and explicitly forcing true for workflow_dispatch if you want manual runs to always execute e2e).

Copilot uses AI. Check for mistakes.
@lionelvillard lionelvillard enabled auto-merge (squash) March 11, 2026 23:46
@lionelvillard
Copy link
Copy Markdown
Collaborator Author

/ok-to-test

@github-actions
Copy link
Copy Markdown
Contributor

🚀 OpenShift E2E — approve and run (/ok-to-test)

View the OpenShift E2E workflow run

@lionelvillard
Copy link
Copy Markdown
Collaborator Author

/trigger-e2e-full

@github-actions
Copy link
Copy Markdown
Contributor

🚀 Kind E2E (full) triggered by /trigger-e2e-full

View the Kind E2E workflow run

@github-actions
Copy link
Copy Markdown
Contributor

GPU Pre-flight Check ✅

GPUs are available for e2e-openshift tests. Proceeding with deployment.

Resource Total Allocated Available
GPUs 50 15 35
Cluster Value
Nodes 16 (7 with GPUs)
Total CPU 993 cores
Total Memory 10383 Gi
GPUs required 4 (min) / 6 (recommended)

Copy link
Copy Markdown
Contributor

@clubanderson clubanderson left a comment

Choose a reason for hiding this comment

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

A few concerns about removing the /trigger-e2e-full gate entirely:

  1. Abuse/cost exposure: Any fork contributor can now trigger 60-minute full Kind e2e runs on every push. The old comment-gate intentionally required write access — that was a deliberate security/cost decision, not accidental complexity.

  2. Kind alone is not sufficient: With the OpenShift nightly e2e tests currently failing, settling for Kind-only validation on PRs is risky. Kind doesn't match the real deployment environment described in the guides. Maintainers need to be confident that /ok-to-test (OpenShift e2e) or /trigger-e2e-full are being used regularly before merge — removing the trigger mechanism makes it easier to skip that step.

  3. Smoke already runs on every PR: If smoke tests are catching regressions on every PR, the full Kind suite was designed as an escalation path that maintainers invoke when they want extra confidence. Running it unconditionally on every push adds cost without a clear benefit over the existing smoke + on-demand full model.

Suggestion: if the goal is to run full e2e more frequently, consider auto-running it only on non-fork PRs (from contributors with write access) while keeping the comment gate for forks. That simplifies the common case without removing the abuse protection.

@lionelvillard
Copy link
Copy Markdown
Collaborator Author

lionelvillard commented Mar 12, 2026

@clubanderson what about merging both /trigger-e2e-full and /ok-to-test together? 99% of the time we always want to both full kind tests and openshift tests and it is quite cumbersome to have type both commands. Can /ok-to-test triggers both tests?

@clubanderson
Copy link
Copy Markdown
Contributor

That sounds a bit better. Sure.

@lionelvillard
Copy link
Copy Markdown
Collaborator Author

will open a new PR.

auto-merge was automatically disabled March 17, 2026 18:19

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants