Uh oh!
There was an error while loading. Please reload this page.
ci: test each Vaadin version on its exact baseline JDK - #39
Conversation
WalkthroughThe PR refactors the GitHub Actions Maven test matrix to separate runtime JDK (for executing tests) from build JDK (for compiling) and updates Surefire includes so per-module test suites (tests-v14/v23/v24 vs tests-v25) run under the intended JVM configuration. ChangesCI Test Matrix and JDK Forking
🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/maven.yml (1)
19-64: 💤 Low valueConsider adding security hardening to the workflow (optional).
The static analysis tools flag several security best practices that could be applied:
- Credential persistence: Add
persist-credentials: falseto the checkout step to prevent credentials from being accessible to later steps via the git config.- Explicit permissions: Add a
permissions:block to grant only the minimum required token permissions (likely justcontents: read).- Pin actions to commit hashes: Replace version tags like
@v3with commit SHA hashes to prevent supply-chain attacks via tag updates.These are established security best practices but may be beyond this PR's scope if not already part of the project's security policy.
🔒 Example hardening (if desired)
jobs: - build: + permissions:+ contents: read runs-on: ubuntu-latest strategy: fail-fast: false # ... matrix ... name: JDK ${{ matrix.run-java }} steps: - - uses: actions/checkout@v3+ - uses: actions/checkout@v4+ with:+ persist-credentials: false - name: Set up runtime JDK ${{ matrix.run-java }} if: matrix.run-java != matrix.build-java - uses: actions/setup-java@v3+ uses: actions/setup-java@v4 with: java-version: ${{ matrix.run-java }} distribution: 'temurin' # ... - name: Set up build JDK ${{ matrix.build-java }} - uses: actions/setup-java@v3+ uses: actions/setup-java@v4 with: java-version: ${{ matrix.build-java }} distribution: 'temurin'(Pinning to commit hashes is even more secure but harder to maintain.)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/maven.yml around lines 19 - 64, Add workflow hardening: set persist-credentials: false on the "actions/checkout@v3" step (the checkout step), add an explicit permissions block granting only contents: read for the job (or workflow) to minimize token scope, and replace mutable action tags "actions/checkout@v3" and "actions/setup-java@v3" (used in the "Set up runtime JDK" and "Set up build JDK" steps) with pinned commit SHAs; ensure the checkout step and both setup-java steps are updated and the permissions block is added near the job definition.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/maven.yml:
- Around line 48-62: Update the GitHub Actions steps that reference legacy
action versions: change the uses entries for actions/checkout@v3 to
actions/checkout@v6 and both actions/setup-java@v3 occurrences to
actions/setup-java@v5, preserving the existing with keys (java-version,
distribution) and the cache: maven setting; ensure the conditional block (if:
matrix.run-java != matrix.build-java) and the separate "Set up runtime JDK" and
"Set up build JDK" step names remain unchanged so behavior and JVM home handling
are preserved.
---
Nitpick comments:
In @.github/workflows/maven.yml:
- Around line 19-64: Add workflow hardening: set persist-credentials: false on
the "actions/checkout@v3" step (the checkout step), add an explicit permissions
block granting only contents: read for the job (or workflow) to minimize token
scope, and replace mutable action tags "actions/checkout@v3" and
"actions/setup-java@v3" (used in the "Set up runtime JDK" and "Set up build JDK"
steps) with pinned commit SHAs; ensure the checkout step and both setup-java
steps are updated and the permissions block is added near the job definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3808dbc1-e267-4c7a-a2ce-7b500af3bb03
📒 Files selected for processing (2)
.github/workflows/maven.ymltests-shared/pom.xml
Uh oh!
There was an error while loading. Please reload this page.
javier-godoy
commented
Jun 10, 2026
@coderabbitai review |
✅ Action performedReview finished.
|
javier-godoy
commented
Jun 10, 2026
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/maven.yml:
- Around line 48-58: Replace floating action tags and disable checkout
credential persistence: update the "actions/checkout" invocation to use the
immutable SHA df4cb1c069e1874edd31b4311f1884172cec0e10 and add with:
persist-credentials: false to that step, and change both "actions/setup-java"
steps (the ones named "Set up runtime JDK ${{ matrix.run-java }}" and "Set up
build JDK ${{ matrix.build-java }}") to use the immutable SHA
be666c2fcd27ec809703dec50e508c2fdc7f6654 instead of `@v5`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The previous CI workflow ran a single JDK 17 job over the whole aggregator. This broke the tests-v25 build (Vaadin 25 is compiled to Java 21 bytecode and cannot load on JDK 17), and missed verifying Vaadin 14 on Java 8 and Vaadin 23 on Java 11.
Changes
Verified locally on all four JDKs using the exact matrix commands: 76 / 77 / 77 / 83 tests pass with zero failures.
Summary by CodeRabbit