ci: ensure test-results runs on cancellation so auto-merge blocks#24290
Merged
ci: ensure test-results runs on cancellation so auto-merge blocks#24290
Conversation
The test-results job is registered as a required status check on
release branches. With if: ${{ failure() || success() }} the job is
skipped when any matrix entry is cancelled (e.g. when an it-tests
shard hits the 30-min job timeout). GitHub treats a skipped required
check as satisfying the requirement, so the cherry-pick auto-merge
proceeds even though the matrix never completed.
Switch to if: ${{ always() }} so the job actually runs and the
existing Set Failure Status step (already if: always() && needs.*
.result != 'success') turns the check red on cancellation. Auto-merge
correctly blocks on the failed required check from then on.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mcollovati
approved these changes
May 7, 2026
vaadin-review-bot
approved these changes
May 7, 2026
Collaborator
Author
|
This PR is eligible for auto-merging policy, so it has been approved automatically. If there are pending conditions, auto merge (with 'squash' method) has been enabled for this PR [Message is sent from bot] |
vaadin-review-bot
approved these changes
May 7, 2026
Collaborator
Author
|
This PR is eligible for auto-merging policy, so it has been approved automatically. If there are pending conditions, auto merge (with 'squash' method) has been enabled for this PR[Message is sent from bot] |
ZheSun88
added
target/main
target/24.9
target/25.0
|
vaadin-bot
added a commit
that referenced
this pull request
May 8, 2026
…4290) (CP: 25.0) (#24301) This PR cherry-picks changes from the original PR #24290 to branch 25.0. --- #### Original PR description > ## Summary > > Cherry-pick PRs targeting release branches were being merged even when > a matrix `it-tests (N, ...)` shard had been cancelled (e.g. by the > 30-min job timeout when CI hung). Example: [#24286](#24286) > merged at 11:49 UTC two seconds after `it-tests (2)` was cancelled at > 11:48:58, with `test-results` showing as `SKIPPED`. > > ## Why this happened > > `test-results` is registered as a required status check on the 24.10 > branch protection. The intent is that it acts as the single chokepoint > representing "all matrix it-tests passed" (since the matrix job names > include the dynamic module list and can't be required individually). > > The job's gate today is: > > ```yaml > test-results: > ... > if: ${{ failure() || success() }} > needs: [unit-tests, it-tests] > ``` > > GitHub Actions `if:` semantics on a `needs:` chain treat > `failure() || success()` as covering only success and failure — **not > cancellation**. So when any matrix shard is cancelled, the > `test-results` job's gate evaluates to false → SKIPPED. > > A skipped required status check > [satisfies the requirement in GitHub's evaluation](https://github.com/orgs/community/discussions/13690), > so the auto-merge bot proceeds and merges the PR. > > The job's existing `Set Failure Status` step (line 268) is already > written correctly with `if: always() && (... .result != 'success')` — > it just never gets a chance to run because the whole job is skipped > first. > > ## Fix > > Change the job-level gate to `if: ${{ always() }}` so `test-results` > always runs (success / failure / cancelled / timed out). The existing > `Set Failure Status` step then explicitly fails the job when any > upstream needs entry is in a non-success state, including > `cancelled`. The required check turns **red** instead of grey, > auto-merge correctly blocks on it. > > Tiny one-liner; comment added so the next person to touch this gate > understands why `always()` is required. > > ## Test plan > > - [ ] Push a draft PR with an artificially cancelled it-tests shard > (or wait for the next natural occurrence) and confirm > `test-results` runs and reports failure rather than being > skipped. > - [ ] Confirm the auto-merge bot does *not* merge a cherry-pick PR > while `test-results` is failing. > > ## Follow-up > > This change should be cherry-picked to `25.0`, `25.1`, and `main` > once verified here. The same one-liner applies on every branch that > carries `validation.yml`. > > 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Zhe Sun <zhe@vaadin.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Zhe Sun <31067185+ZheSun88@users.noreply.github.com>
vaadin-bot
added a commit
that referenced
this pull request
May 8, 2026
…4290) (CP: 24.9) (#24302) This PR cherry-picks changes from the original PR #24290 to branch 24.9. --- #### Original PR description > ## Summary > > Cherry-pick PRs targeting release branches were being merged even when > a matrix `it-tests (N, ...)` shard had been cancelled (e.g. by the > 30-min job timeout when CI hung). Example: [#24286](#24286) > merged at 11:49 UTC two seconds after `it-tests (2)` was cancelled at > 11:48:58, with `test-results` showing as `SKIPPED`. > > ## Why this happened > > `test-results` is registered as a required status check on the 24.10 > branch protection. The intent is that it acts as the single chokepoint > representing "all matrix it-tests passed" (since the matrix job names > include the dynamic module list and can't be required individually). > > The job's gate today is: > > ```yaml > test-results: > ... > if: ${{ failure() || success() }} > needs: [unit-tests, it-tests] > ``` > > GitHub Actions `if:` semantics on a `needs:` chain treat > `failure() || success()` as covering only success and failure — **not > cancellation**. So when any matrix shard is cancelled, the > `test-results` job's gate evaluates to false → SKIPPED. > > A skipped required status check > [satisfies the requirement in GitHub's evaluation](https://github.com/orgs/community/discussions/13690), > so the auto-merge bot proceeds and merges the PR. > > The job's existing `Set Failure Status` step (line 268) is already > written correctly with `if: always() && (... .result != 'success')` — > it just never gets a chance to run because the whole job is skipped > first. > > ## Fix > > Change the job-level gate to `if: ${{ always() }}` so `test-results` > always runs (success / failure / cancelled / timed out). The existing > `Set Failure Status` step then explicitly fails the job when any > upstream needs entry is in a non-success state, including > `cancelled`. The required check turns **red** instead of grey, > auto-merge correctly blocks on it. > > Tiny one-liner; comment added so the next person to touch this gate > understands why `always()` is required. > > ## Test plan > > - [ ] Push a draft PR with an artificially cancelled it-tests shard > (or wait for the next natural occurrence) and confirm > `test-results` runs and reports failure rather than being > skipped. > - [ ] Confirm the auto-merge bot does *not* merge a cherry-pick PR > while `test-results` is failing. > > ## Follow-up > > This change should be cherry-picked to `25.0`, `25.1`, and `main` > once verified here. The same one-liner applies on every branch that > carries `validation.yml`. > > 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Zhe Sun <zhe@vaadin.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Zhe Sun <31067185+ZheSun88@users.noreply.github.com>
vaadin-bot
added a commit
that referenced
this pull request
May 8, 2026
…4290) (CP: 25.1) (#24300) This PR cherry-picks changes from the original PR #24290 to branch 25.1. --- #### Original PR description > ## Summary > > Cherry-pick PRs targeting release branches were being merged even when > a matrix `it-tests (N, ...)` shard had been cancelled (e.g. by the > 30-min job timeout when CI hung). Example: [#24286](#24286) > merged at 11:49 UTC two seconds after `it-tests (2)` was cancelled at > 11:48:58, with `test-results` showing as `SKIPPED`. > > ## Why this happened > > `test-results` is registered as a required status check on the 24.10 > branch protection. The intent is that it acts as the single chokepoint > representing "all matrix it-tests passed" (since the matrix job names > include the dynamic module list and can't be required individually). > > The job's gate today is: > > ```yaml > test-results: > ... > if: ${{ failure() || success() }} > needs: [unit-tests, it-tests] > ``` > > GitHub Actions `if:` semantics on a `needs:` chain treat > `failure() || success()` as covering only success and failure — **not > cancellation**. So when any matrix shard is cancelled, the > `test-results` job's gate evaluates to false → SKIPPED. > > A skipped required status check > [satisfies the requirement in GitHub's evaluation](https://github.com/orgs/community/discussions/13690), > so the auto-merge bot proceeds and merges the PR. > > The job's existing `Set Failure Status` step (line 268) is already > written correctly with `if: always() && (... .result != 'success')` — > it just never gets a chance to run because the whole job is skipped > first. > > ## Fix > > Change the job-level gate to `if: ${{ always() }}` so `test-results` > always runs (success / failure / cancelled / timed out). The existing > `Set Failure Status` step then explicitly fails the job when any > upstream needs entry is in a non-success state, including > `cancelled`. The required check turns **red** instead of grey, > auto-merge correctly blocks on it. > > Tiny one-liner; comment added so the next person to touch this gate > understands why `always()` is required. > > ## Test plan > > - [ ] Push a draft PR with an artificially cancelled it-tests shard > (or wait for the next natural occurrence) and confirm > `test-results` runs and reports failure rather than being > skipped. > - [ ] Confirm the auto-merge bot does *not* merge a cherry-pick PR > while `test-results` is failing. > > ## Follow-up > > This change should be cherry-picked to `25.0`, `25.1`, and `main` > once verified here. The same one-liner applies on every branch that > carries `validation.yml`. > > 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Zhe Sun <zhe@vaadin.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Zhe Sun <31067185+ZheSun88@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cherry-pick PRs targeting release branches were being merged even when
a matrix
it-tests (N, ...)shard had been cancelled (e.g. by the30-min job timeout when CI hung). Example: #24286
merged at 11:49 UTC two seconds after
it-tests (2)was cancelled at11:48:58, with
test-resultsshowing asSKIPPED.Why this happened
test-resultsis registered as a required status check on the 24.10branch protection. The intent is that it acts as the single chokepoint
representing "all matrix it-tests passed" (since the matrix job names
include the dynamic module list and can't be required individually).
The job's gate today is:
GitHub Actions
if:semantics on aneeds:chain treatfailure() || success()as covering only success and failure — notcancellation. So when any matrix shard is cancelled, the
test-resultsjob's gate evaluates to false → SKIPPED.A skipped required status check
satisfies the requirement in GitHub's evaluation,
so the auto-merge bot proceeds and merges the PR.
The job's existing
Set Failure Statusstep (line 268) is alreadywritten correctly with
if: always() && (... .result != 'success')—it just never gets a chance to run because the whole job is skipped
first.
Fix
Change the job-level gate to
if: ${{ always() }}sotest-resultsalways runs (success / failure / cancelled / timed out). The existing
Set Failure Statusstep then explicitly fails the job when anyupstream needs entry is in a non-success state, including
cancelled. The required check turns red instead of grey,auto-merge correctly blocks on it.
Tiny one-liner; comment added so the next person to touch this gate
understands why
always()is required.Test plan
(or wait for the next natural occurrence) and confirm
test-resultsruns and reports failure rather than beingskipped.
while
test-resultsis failing.Follow-up
This change should be cherry-picked to
25.0,25.1, andmainonce verified here. The same one-liner applies on every branch that
carries
validation.yml.🤖 Generated with Claude Code