Skip to content

Conversation

@msridhar
Copy link
Collaborator

@msridhar msridhar commented Jan 1, 2026

Our build scripts now create testJdk21 and testJdk17 tasks for testing on older JDKs, assuming that Gradle itself (and hence the test task) runs on JDK 25. Configure CI to use JDK 25 for builds. Do some related build script cleanup. No behavior changes.

Fixes #1399

Summary by CodeRabbit

  • Chores
    • Updated CI/CD workflow dependencies to latest versions.
    • Updated Java toolchain configuration for build and test processes.

✏️ Tip: You can customize this high-level summary in your review settings.

@msridhar msridhar marked this pull request as ready for review January 1, 2026 00:36
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026

Walkthrough

This PR updates the build infrastructure and CI configuration across multiple files. The GitHub Actions workflow is updated to use newer action versions (actions/checkout v6 and gradle/actions/setup-gradle v5) and simplifies the Java version specification from a multiline format to a single value. The Gradle build scripts are refactored to modify the JDK test matrix—changing the version range from [25] to [21]—and consolidate JVM argument configurations by using a centralized commonJavacOpens reference. Additionally, toolchain versions are updated from 24 to 25 in jdk-recent-unit-tests, and test task configurations are adjusted across jar-infer-lib and jdk-recent-unit-tests.

Possibly related PRs

Suggested reviewers

  • lazaroclapp
  • yuxincs

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Update CI to run builds on JDK 25' directly reflects the main objective of the PR: configuring CI to use JDK 25 and updating build scripts to support testing on this version.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfdd5cc and f87fbba.

📒 Files selected for processing (4)
  • .github/workflows/continuous-integration.yml
  • buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
  • jar-infer/jar-infer-lib/build.gradle
  • jdk-recent-unit-tests/build.gradle
💤 Files with no reviewable changes (1)
  • jar-infer/jar-infer-lib/build.gradle
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Applied to files:

  • jdk-recent-unit-tests/build.gradle
  • buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
📚 Learning: 2025-11-25T22:43:06.446Z
Learnt from: CR
Repo: uber/NullAway PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T22:43:06.446Z
Learning: Run only the tests for the main NullAway module using `./gradlew :nullaway:test` unless specifically asked to run tests in a different module

Applied to files:

  • jdk-recent-unit-tests/build.gradle
  • buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
📚 Learning: 2025-10-09T19:59:16.543Z
Learnt from: msridhar
Repo: uber/NullAway PR: 1243
File: jdk-annotations/astubx-generator/build.gradle:22-22
Timestamp: 2025-10-09T19:59:16.543Z
Learning: When disabling testJdk17 tasks for modules requiring JDK 21, use `onlyIf { false }` to skip the task:
```gradle
tasks.named("testJdk17").configure {
    onlyIf { false }
}
```
Do not use `doFirst { throw new GradleException(...) }` as it will cause CI failures when the task is executed.

Applied to files:

  • jdk-recent-unit-tests/build.gradle
  • buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle
🔇 Additional comments (7)
.github/workflows/continuous-integration.yml (3)

27-27: LGTM! Java version correctly set to 25.

This change aligns with the PR objective to configure CI to use JDK 25 for builds. The simplification from a multiline format to a single value also improves readability.


76-76: gradle/actions/setup-gradle@v5 is a valid release; ensure runner compatibility.

v5.0.0 (released Oct 1, 2025) is available and properly used on both line 30 and line 76. Note that v5 upgraded Node to v24 and requires GitHub Actions runner v2.327.1 or newer—verify the runner versions in your environment support this.


23-23: The update to actions/checkout@v6 is safe to proceed. Version 6 is available and stable. While it includes a breaking change regarding credential handling (credentials written to $RUNNER_TEMP instead of .git/config), this change is handled automatically by the action and does not affect this workflow, which does not rely on the credentials location or run authenticated git commands inside Docker containers.

jdk-recent-unit-tests/build.gradle (2)

21-25: LGTM! JDK 25 toolchain correctly configured.

The explicit toolchain configuration for JDK 25 is appropriate for this module, which tests recent JDK features. This aligns with the PR objective where the default test task runs on JDK 25.


57-62: LGTM! Test task disabling is correct.

The configuration correctly disables both testJdk17 and testJdk21 tasks using onlyIf { false }, which aligns with the learned practice. Since this module tests recent JDK features and requires JDK 25, it should only run the default test task on JDK 25.

The change from testJdk25 to testJdk21 is consistent with the update in the test conventions file, which now creates testJdk21 and testJdk17 tasks for testing on older JDKs.

Based on learnings, using onlyIf { false } to skip tasks is the correct approach to avoid CI failures.

buildSrc/src/main/groovy/nullaway.java-test-conventions.gradle (2)

81-101: LGTM! Per-version test task correctly creates testJdk21.

The change from [25].each to [21].each aligns with the PR objective to create testJdk21 tasks for testing on older JDKs. With the CI now using JDK 25 for the default test task, this provides backward compatibility testing on JDK 21.

The task configuration correctly:

  • Uses a toolchain launcher for JDK 21
  • Copies test configuration from the main test task
  • Applies commonJavacOpens for JDK 16+ compatibility

104-122: LGTM! JDK 17 test task simplified and improved.

The changes improve this task:

  1. Description simplified (line 109): Removes the mention of "oldest Error Prone version" for better clarity.
  2. JVM arguments refactored (line 121): Uses commonJavacOpens instead of an explicit list, eliminating code duplication and improving maintainability.

The task continues to correctly use the Error Prone JDK 17 classpath configuration for compatibility testing.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@msridhar msridhar enabled auto-merge (squash) January 1, 2026 00:43
@msridhar msridhar requested a review from yuxincs January 1, 2026 00:43
@codecov
Copy link

codecov bot commented Jan 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.40%. Comparing base (bfdd5cc) to head (f87fbba).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #1422   +/-   ##
=========================================
  Coverage     88.40%   88.40%           
  Complexity     2669     2669           
=========================================
  Files            97       97           
  Lines          8872     8872           
  Branches       1775     1775           
=========================================
  Hits           7843     7843           
  Misses          511      511           
  Partials        518      518           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@msridhar msridhar merged commit 22aa129 into master Jan 1, 2026
11 checks passed
@msridhar msridhar deleted the ci-jdk-25 branch January 1, 2026 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update CI jobs to run on JDK 25

3 participants