fix: wrong existence check in getStaticResource (CP: 25.0) (#24285) (CP: 24.10)#24286
Merged
vaadin-bot merged 1 commit into24.10from May 7, 2026
Merged
Conversation
This PR cherry-picks changes from the original PR #24283 to branch 25.0. A conflict in `flow-server/src/test/java/com/vaadin/flow/server/VaadinServletServiceTest.java` was resolved manually: the new `getStaticResource_jarUrlOnJetty12_returnsUrlInsteadOfThrowing` test was rewritten in JUnit 4 idioms (`@Rule TemporaryFolder`, `Assert.*`) since 25.0 has not adopted the JUnit 5 migration that landed on `main`.
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] |
vaadin-review-bot
approved these changes
May 7, 2026
|
2 tasks
ZheSun88
added a commit
that referenced
this pull request
May 8, 2026
…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>
This was referenced May 8, 2026
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
3 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.
This PR cherry-picks changes from the original PR #24285 to branch 24.10.
Original PR description