Skip to content

ci: ensure test-results runs on cancellation so auto-merge blocks (#24290) (CP: main)#24303

Merged
ZheSun88 merged 1 commit intomainfrom
cherry-pick-24290-to-main-1778234164513
May 8, 2026
Merged

ci: ensure test-results runs on cancellation so auto-merge blocks (#24290) (CP: main)#24303
ZheSun88 merged 1 commit intomainfrom
cherry-pick-24290-to-main-1778234164513

Conversation

@vaadin-bot
Copy link
Copy Markdown
Collaborator

This PR cherry-picks changes from the original PR #24290 to branch main.

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
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:

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,
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

…4290)

## 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>
@ZheSun88 ZheSun88 enabled auto-merge May 8, 2026 09:57
@github-actions github-actions Bot added the +0.0.1 label May 8, 2026
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 8, 2026

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 8, 2026

Test Results

 1 404 files  ±0   1 404 suites  ±0   1h 18m 49s ⏱️ - 2m 13s
10 128 tests ±0  10 058 ✅ ±0  70 💤 ±0  0 ❌ ±0 
10 603 runs  ±0  10 524 ✅ ±0  79 💤 ±0  0 ❌ ±0 

Results for commit 423f170. ± Comparison against base commit 8822cb1.

@ZheSun88 ZheSun88 added this pull request to the merge queue May 8, 2026
Merged via the queue into main with commit 09ccde4 May 8, 2026
31 checks passed
@ZheSun88 ZheSun88 deleted the cherry-pick-24290-to-main-1778234164513 branch May 8, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants