Skip to content

ci(e2e): always report status on PRs and merge queue - #1988

Merged
ralphbean merged 1 commit into
mainfrom
ci/always-report-required-checks
Jun 9, 2026
Merged

ci(e2e): always report status on PRs and merge queue#1988
ralphbean merged 1 commit into
mainfrom
ci/always-report-required-checks

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Remove paths filters from the pull_request trigger in e2e.yml so the e2e job always runs and reports a status
  • Add merge_group trigger so e2e can be used as a required check in the merge queue
  • When no e2e-relevant files changed on a PR, the job short-circuits after checking the file list via the GitHub API — no checkout, no Go setup, no Playwright install

This unblocks making e2e a required status check without blocking docs-only or non-code PRs. Push-to-main keeps its path filters since it doesn't gate the merge queue.

Closes #1987

Test plan

  • Verify this PR's own e2e check runs and short-circuits (no e2e-relevant files changed)
  • Verify a PR with Go changes still runs the full e2e suite

🤖 Generated with Claude Code

Remove path filters from pull_request trigger and add merge_group
trigger so the e2e job always reports a status. When no e2e-relevant
files changed, the job short-circuits after checking the PR file list
via the GitHub API.

This lets the e2e check be made a required status check without
blocking docs-only or other non-code PRs from the merge queue.

Push-to-main keeps its path filters since it doesn't gate the merge
queue.

Closes #1987

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

Site preview

Preview: https://2a77d38b-site.fullsend-ai.workers.dev

Commit: 6aef818d039a88fde76133f2cd1d9d8ca3306c41

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

Low

  • [action-version-consistency] .github/workflows/e2e.yml:44 — Pre-existing: actions/checkout@v4 is used here while all other workflows in the repo use @v6 or @v6.0.2. Not introduced by this PR, but worth updating as a follow-up.

  • [implementation-detail] .github/workflows/e2e.yml:42 — The grep regex duplicating the path-relevance logic creates a maintenance burden with the push trigger's paths: filter. If e2e-relevant paths change, the grep regex must be updated separately. Consider adding a comment cross-referencing the two locations.

Info

  • [edge-case] .github/workflows/e2e.yml:36 — The GitHub REST API returns a maximum of 3000 files per PR (across all pages). For PRs exceeding that limit, the file list would be truncated and the short-circuit check could incorrectly skip tests. This is an extreme edge case unlikely to occur in practice.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 7, 2026

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At least filter out docs/ and web/

@ralphbean

Copy link
Copy Markdown
Member Author

@rh-hemartin what do you mean? Do you see some way that those will make it through the filter on line 43?

The idea here is that we always run the e2e gh workflow, but sometimes that workflow exits early if there's nothing meaningful to test in the change set.

With this, we can make the e2e test a required check because it "always runs" even though many times it won't run anything of substance.

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I just commented based on the removal of paths, but didn't go far into it to see that there was a filter. Just recheked and I looked just at the top half of my monitor lol.

@ralphbean
ralphbean added this pull request to the merge queue Jun 9, 2026
Merged via the queue into main with commit 783a78c Jun 9, 2026
12 checks passed
@ralphbean
ralphbean deleted the ci/always-report-required-checks branch June 9, 2026 17:41
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:44 PM UTC · Completed 5:49 PM UTC
Commit: ba204cb · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1988 — ci(e2e): always report status on PRs and merge queue

Workflow quality: Good. This was a clean, human-authored CI change that went through one review cycle and merged without code rework.

Timeline

  1. Jun 7 14:46 — PR created by ralphbean. Single file changed: .github/workflows/e2e.yml (+25/-15).
  2. Jun 7 14:54 — Review agent posted findings: 1 medium (protected path), 2 low (checkout v4 inconsistency, duplicated path logic), 1 info (API pagination edge case). All findings were accurate and appropriately categorized.
  3. Jun 8 08:42 — Human reviewer (rh-hemartin) requested changes, thinking path filtering was removed entirely.
  4. Jun 8 18:42 — Author explained the in-workflow grep filter handles the same paths.
  5. Jun 9 07:10 — Reviewer approved after re-reading the full diff.
  6. Jun 9 17:38 — PR merged.

Assessment

  • Review quality: Strong. The review agent's findings were all valid with no false positives. The [implementation-detail] finding about duplicated path logic was particularly useful — it identified a real maintenance burden.
  • Rework rate: Zero code rework. The human's change request was resolved by explanation, not code changes.
  • Token cost: Efficient — single file change, no unnecessary exploration.
  • One proposal: The duplicated path logic between the push trigger's paths: filter and the in-workflow grep regex is a maintenance risk worth tracking.

Proposals filed

ggallen pushed a commit to ggallen/fullsend that referenced this pull request Jun 18, 2026
Remove `paths:` filter from `pull_request_target` so the e2e workflow
triggers on all PRs. Add a "Check for e2e-relevant changes" step that
queries the PR's changed files via the API and short-circuits when no
e2e-relevant paths are touched. This ensures the `e2e` required check
always reports a status, unblocking docs-only and config-only PRs from
the merge queue.

This restores the approach from fullsend-ai#1988 which was inadvertently lost when
the e2e workflow was refactored to use pull_request_target with a
gate/e2e job split.

Fixes fullsend-ai#1989

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: conditional checks block merge queue when they don't run

2 participants