build: Gradle 9.6.1 on JDK 25, and clear the Gradle 10 deprecations - #3291
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe build is upgraded to Gradle 9.6.1 and JDK 25 while published artifacts continue targeting Java 11. Publishing logic adopts newer Gradle APIs, optional feature variants gain shared helpers, CI actions are pinned, Java matrix coverage expands, and related documentation is updated throughout. ChangesBuild and CI modernization
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant matrix_prep as matrix_prep job
participant matrix_mjs as matrix.mjs
participant test_job as test job (8x parallel)
participant build_gradle as Gradle + JDK 25
matrix_prep->>matrix_mjs: generate Java/distro matrix
matrix_mjs->>matrix_prep: return 8 job configs (Java 20–26, EA 27–28)
matrix_prep->>test_job: dispatch matrix
test_job->>build_gradle: setup Java 25 for Gradle
build_gradle->>build_gradle: run tests on matrix Java version
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
juherr
force-pushed
the
build/gradle-9-jdk-25
branch
from
July 26, 2026 13:09
9a7da66 to
d045e4e
Compare
juherr
force-pushed
the
build/gradle-9-jdk-25
branch
from
July 26, 2026 13:47
d045e4e to
5d8183b
Compare
This was referenced Jul 26, 2026
Closed
juherr
marked this pull request as ready for review
July 26, 2026 14:02
Member
Author
|
@vlsi could you review this? It moves the build to Gradle 9.6.1 on JDK 25 and clears the Gradle 10 deprecations. Bytecode target stays at Java 11. Two questions for you: 1. 2. Optional features — What would you do here? I left it untouched in this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
build-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.kts (1)
12-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope
SelfAssertionsuppression to test/fixture compilation.
tasks.withType<JavaCompile>().configureEachapplies this disable to production sources as well as fixtures, so futureSelfAssertionfindings in main code will be silently accepted. Restrict the suppression to test/fixture compile tasks or suppress only the intentional fixture assertions.🤖 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 `@build-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.kts` around lines 12 - 17, Restrict the SelfAssertion suppression in the JavaCompile configuration to test/fixture compilation tasks only, rather than applying options.errorprone.disable("SelfAssertion") to every JavaCompile task. Keep disableWarningsInGeneratedCode unchanged and ensure production compilation still reports SelfAssertion findings.
🤖 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/gradle-wrapper-validation.yml:
- Line 9: Update the actions/checkout steps in
.github/workflows/gradle-wrapper-validation.yml:9-9,
.github/workflows/label-commenter.yml:16-16, and
.github/workflows/publish-maven-central.yml:27-27 to configure
persist-credentials as false under each checkout step’s with block.
In @.github/workflows/publish-snapshot.yml:
- Line 13: Disable checkout credential persistence for the checkout steps in
.github/workflows/publish-snapshot.yml:13-13, .github/workflows/test.yml:31-33,
and .github/workflows/test.yml:49-51 by setting persist-credentials to false. In
.github/workflows/update-gradle-wrapper.yml:12-12, also disable ambient checkout
credentials while retaining the Gradle wrapper action’s explicitly scoped
repo-token configuration.
In `@build-logic-commons/gradle-plugin/build.gradle.kts`:
- Around line 18-29: Normalize jdkBuildVersion before JVM target selection in
build-logic-commons/gradle-plugin/build.gradle.kts lines 18-29 and
build-logic-commons/gradle-plugin/src/main/kotlin/build-logic.kotlin-dsl-gradle-plugin.gradle.kts
lines 13-24: when the configured value is 0, replace it with
JavaVersion.current().majorVersion, then use the normalized version for
JavaVersion.toVersion(...) and subsequent target selection; preserve existing
behavior for nonzero values.
In `@docs/BUILD_SYSTEM.md`:
- Around line 277-279: Update the matrix-size documentation consistently with
MATRIX_JOBS: 8: in docs/BUILD_SYSTEM.md, revise the later seven-job descriptions
and examples to eight; in docs/CI_TEST_WORKFLOW.md, update the “7 random
configurations” walkthrough to eight or clearly mark it as illustrative; and
update the local command default from MATRIX_JOBS=7 to 8 or document it as an
example value.
- Around line 290-296: Update the actions/setup-java references in the
BUILD_SYSTEM documentation examples, including the snippets near the setup and
publish sections, to use the same SHA-pinned v5 commit as the workflows; keep
the documented configuration unchanged.
---
Nitpick comments:
In `@build-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.kts`:
- Around line 12-17: Restrict the SelfAssertion suppression in the JavaCompile
configuration to test/fixture compilation tasks only, rather than applying
options.errorprone.disable("SelfAssertion") to every JavaCompile task. Keep
disableWarningsInGeneratedCode unchanged and ensure production compilation still
reports SelfAssertion findings.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bbe3b163-7cad-4593-a218-b3928d3000aa
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (43)
.github/CONTRIBUTING.md.github/workflows/combine-prs.yml.github/workflows/gradle-wrapper-validation.yml.github/workflows/label-commenter.yml.github/workflows/matrix.mjs.github/workflows/publish-maven-central.yml.github/workflows/publish-snapshot.yml.github/workflows/test.yml.github/workflows/update-gradle-wrapper.ymlCHANGES.txtbuild-logic-commons/gradle-plugin/build.gradle.ktsbuild-logic-commons/gradle-plugin/src/main/kotlin/build-logic.kotlin-dsl-gradle-plugin.gradle.ktsbuild-logic/basics/src/main/kotlin/testng.reproducible-builds.gradle.ktsbuild-logic/build-parameters/build.gradle.ktsbuild-logic/code-quality/build.gradle.ktsbuild-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.ktsbuild-logic/code-quality/src/main/kotlin/testng.testing.gradle.ktsbuild-logic/jvm/build.gradle.ktsbuild-logic/jvm/src/main/kotlin/testng.java-library.gradle.ktsbuild-logic/jvm/src/main/kotlin/testng.kotlin-library.gradle.ktsbuild-logic/publishing/build.gradle.ktsbuild-logic/publishing/src/main/kotlin/buildlogic/DependencyHandlerExtensions.ktbuild-logic/publishing/src/main/kotlin/buildlogic/OptionalFeaturesExtension.ktbuild-logic/publishing/src/main/kotlin/testng.local-maven-repo.gradle.ktsbuild-logic/publishing/src/main/kotlin/testng.maven-publish.gradle.ktsbuild-logic/publishing/src/main/kotlin/testng.merge-feature-jars.gradle.ktsbuild-logic/publishing/src/main/kotlin/testng.signing.gradle.ktsbuild-logic/settings.gradle.ktsbuild.gradle.ktsdocs/BUILD_SYSTEM.mddocs/CI_TEST_WORKFLOW.mddocs/JAVA_VERSIONS_QUICK_REFERENCE.mddocs/LOCAL_TESTING_GUIDE.mddocs/README.mddocs/RELEASE_PROCESS.mdgradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batsettings.gradle.ktstestng-core/testng-core-build.gradle.ktstestng-test-osgi/testng-test-osgi-build.gradle.ktstestng/testng-build.gradle.kts
💤 Files with no reviewable changes (1)
- testng-core/testng-core-build.gradle.kts
Comment on lines
+290
to
296
| - name: Set up Java ${{ matrix.non_ea_java_version }} (test) and 25 (Gradle + build) | ||
| uses: actions/setup-java@v5 | ||
| with: | ||
| java-version: | | ||
| ${{ matrix.non_ea_java_version }} # e.g., 11 | ||
| 21 # Always install Java 21 | ||
| 25 # Always install Java 25 | ||
| distribution: ${{ matrix.java_distribution }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs BUILD_SYSTEM relevant sections =="
if [ -f docs/BUILD_SYSTEM.md ]; then
nl -ba docs/BUILD_SYSTEM.md | sed -n '260,340p'
else
echo "docs/BUILD_SYSTEM.md not found"
fi
echo
echo "== workflow action setup-java usages =="
git ls-files .github/workflows | while read -r f; do
echo "--- $f"
rg -n "actions/setup-java@" "$f" || true
done
echo
echo "== docs references to SHA-pinned/mutable actions =="
rg -n "actions/setup-java|setup-java@" docs .github/workflows || trueRepository: testng-team/testng
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs BUILD_SYSTEM relevant sections =="
if [ -f docs/BUILD_SYSTEM.md ]; then
awk 'NR>=260 && NR<=340 { printf "%6d\t%s\n", NR, $0 }' docs/BUILD_SYSTEM.md
else
echo "docs/BUILD_SYSTEM.md not found"
fi
echo
echo "== workflow setup-java usages =="
git ls-files .github/workflows 2>/dev/null | sort | while IFS= read -r f; do
echo "--- $f"
grep -n "actions/setup-java@" "$f" || true
done
echo
echo "== docs and workflow references =="
find . -path ./.git -prune -o \( -path '*.md' -o -path '.github/workflows/*' \) -print 2>/dev/null | sort | while IFS= read -r f; do
grep -n "actions/setup-java@" "$f" 2>/dev/null || true
doneRepository: testng-team/testng
Length of output: 4208
Mirror the SHA-pinned workflow action in the docs.
.github/workflows/test.yml and .github/workflows/publish-maven-central.yml pin actions/setup-java@03ad4de0992f5dab5e18fcb136590ce7c4a0ac95 # v5, but docs/BUILD_SYSTEM.md still shows actions/setup-java@v5 at lines 291 and 322. Update those examples to the same SHA-pinned reference or clearly mark them as mutable illustrative snippets.
🤖 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 `@docs/BUILD_SYSTEM.md` around lines 290 - 296, Update the actions/setup-java
references in the BUILD_SYSTEM documentation examples, including the snippets
near the setup and publish sections, to use the same SHA-pinned v5 commit as the
workflows; keep the documented configuration unchanged.
krmahadevan
approved these changes
Jul 27, 2026
Member
|
@juherr LGTM. We can merge once the merge conflicts are resolved. |
Contributor
I've fixed that one and released in 4.0.0.
|
The build JDK and the bytecode target were hardcoded in three places: build-parameters, and the two build-logic-commons scripts that pick a JVM target for the build scripts themselves. Declare jdkBuildVersion and targetJavaVersion once in the repository-root gradle.properties. Included builds do not inherit it, so build-logic-commons reads the file from disk. Their fallback stays at 17 (the Kotlin DSL floor) rather than following targetJavaVersion, since the build scripts' JVM target is unrelated to TestNG's bytecode target.
Gradle 9 removes several APIs the build relied on: - AbstractArchiveTask.dirMode/fileMode -> dirPermissions/filePermissions - BuildIdentifier.isCurrentBuild() -> buildPath (the root build's path is ":") - manifest attributes must be non-null - plugins from an included build are only resolved when the build is included from pluginManagement Also bump the plugins that were not Gradle 9 compatible: vlsi gradle-extensions 1.90 -> 3.0.2 and the Shadow plugin from the unmaintained com.github.johnrengelman:8.1.1 to com.gradleup.shadow:9.6.1.
Gradle 9 runs on JDK 25, so the build toolchain moves from 21 to 25. Error Prone 2.20 crashes on JDK 25, hence error_prone_core 2.20 -> 2.50 and gradle-errorprone-plugin 3.1.0 -> 4.1.0. The new SelfAssertion check is disabled: it only flags the intentionally trivial assertions in TestNG's own sample/fixture test classes. The bytecode target stays at Java 11 (targetJavaVersion), so this is not a breaking change for TestNG users.
The comment described code that no longer exists.
Mutable tags let a compromised action publish arbitrary code into our workflows. Every action is now pinned to the commit SHA the tag currently points at, with the version kept as a trailing comment so Dependabot can still bump them.
Gradle 9 runs on JDK 25, and the build toolchain is now jdkBuildVersion=25. The test workflow provisions the matrix test JDK plus JDK 25 (last, so it becomes JAVA_HOME) instead of the previous JDK 21; the publish workflows move from zulu 21 to temurin 25. The test matrix still covers Java 11 and the artifacts still target Java 11.
Java 26 is released, so it moves out of the early-access slot; eaJava becomes a list holding 27 and 28, which are only published on jdk.java.net. Both EA pages were checked as reachable. Note the java_version axis now has 7 values for MATRIX_JOBS=7, so it is exactly at capacity: adding another version without raising MATRIX_JOBS would leave some versions untested.
The build docs still described the JDK 21 / Gradle 8 setup: the Gradle runtime and build toolchain are now JDK 25, the CI matrix covers 11-26 plus EA 27/28, and the publish workflows use temurin 25. Also corrected pre-existing inaccuracies found while updating: the claimed .sdkmanrc file does not exist, the test-runtime list mentioned Java 24 which is not in the matrix, the nmcp error message quoted "Java 21+" instead of its actual "Java 17+", and the matrix builder was renamed to matrix.mjs upstream. mise is added alongside SDKMAN/jenv in the install snippets.
The note restated the snapshot URL already spelled out two lines below.
The java_version axis now has 7 values, so 7 jobs left the matrix exactly at capacity: every row was pinned to a distinct Java version and no slack remained for the other axes to pair against. One extra row lifts pair coverage from 38.9% to 44.4%.
Gradle 10 rejects Develocity plugin versions below 4.0. The buildScan DSL used here (termsOfUseUrl, termsOfUseAgree, publishing.onlyIf) is unchanged in 4.x.
…ndencies Gradle 10 removes DependencyHandler.create(group, name, version). Verified the generated testng POM still lists the same dependencies (testng-asserts, slf4j, jcommander, jquery, plus optional guice and snakeyaml).
nmcp's publishAllProjectsProbablyBreakingProjectIsolation() feeds Project objects to the dependency handler, a notation Gradle 10 removes. Apply com.gradleup.nmcp in the maven-publish convention and list the aggregated projects with the String path notation instead. A projectsEvaluated guard fails the build if a project applies 'maven-publish' without being aggregated, so the explicit list cannot silently drift and drop a module from a release. Also bumps nmcp 1.4.1 -> 1.6.1. Verified the aggregation zip still holds the same 24 entries (testng jar, sources, javadoc, module, pom and checksums), and that the guard fires when the aggregation entry is removed.
Gradle 10 removes 'val x by configurations.creating', 'val x by tasks.registering(T::class)' and 'val x: T by project'. Rewritten as configurations.create(name), tasks.register<T>(name) and project.the<T>()/findProperty(name). Mechanical: verified the shaded testng-all jar still holds the same 464 Java 11 classes, the generated POM the same 6 dependencies, and the OSGi test module (which registers tasks through the rewritten delegates) still passes from a clean build.
Error Prone 2.46.0 onwards requires the compiler to be passed -XDaddTypeAnnotationsToSymbol=true, which the plugin only started doing in 4.4.0. The build runs error_prone_core 2.50.0 on plugin 4.1.0, so it was below what the checker expects. None of the 5.0.0 breaking changes apply here: the minimum Gradle version is 7.1 (we run 9.6.1), the minimum JDK is 11 (we build on 25), and the isEnabled/isCompilingTestOnlyCode rename touches no property this build sets.
- org.sonarqube 4.4.1.3373 (2023) -> 7.3.1.8318 - Kotlin plugin and kotlin-bom 2.3.0 -> 2.4.10 - assertj-core 3.23.1 -> 3.27.7 - foojay-resolver-convention 0.8.0 -> 1.0.0 - org.gradlex.build-parameters 1.4.4 -> 1.4.5 Autostyle is deliberately left at 4.0: 4.0.1 bundles a google-java-format that cannot reach com.sun.tools.javac.file under JPMS, and fails every module with IllegalAccessError on JDK 25. Verified with a full clean build on JDK 25 (12528 tests, 0 failures); the Java and Kotlin bytecode still targets Java 11 (major version 55).
actions/checkout writes the job token into .git/config by default, leaving it readable by anything the build runs. None of these jobs push through the checked-out remote: update-gradle-wrapper sets up its own git credentials from repo-token before pushing, and the rest only read the tree.
jdkBuildVersion=0 is documented as "use the JVM running Gradle", and the main build handles it. The build-logic scripts fed it straight to JavaVersion.toVersion(), which failed the build with 'Index -1 out of bounds for length 31'.
The check fires 16 times, all in test fixtures such as
assertThat("abc").isEqualTo("abc"), which exist only to give the runner a
passing method. Disabling it for every JavaCompile task also silenced it on
main sources for no reason.
4.0.0 replaces the AbstractTestTask.afterTest/afterSuite(Closure) calls that Gradle 10 removes, so the last deprecation coming from a third-party plugin is gone. Verified the testng POM and Gradle Module Metadata are unchanged.
The pom is assembled from feature variants plus a shaded jar, so a wiring mistake silently changes what consumers resolve; until now that was only ever checked by hand. Pin the expected set and fail the build when it drifts. Versions are ignored on purpose, so dependency bumps do not require touching the list.
Gradle 10 removes registerFeature(name) { usingSourceSet(sourceSets["main"]) },
and its prescribed replacement does not fit: these features carry no code, only
extra dependencies attached to the main artifact. A dedicated source set would
publish an empty jar under the feature capability and pull the dependencies off
main's compile classpath.
Declare the two consumable variants by hand instead, keeping the main jar as
the artifact and mapping them to optional pom dependencies. The aggregated
testng module keeps variants with no file of their own, as before.
Verified the feature variants of testng, testng-core and testng-core-api are
unchanged (same capabilities, attributes and artifacts), and that the published
pom, Gradle Module Metadata and shaded jar are identical to the baseline. The
only difference is the loss of the 'classes'/'resources' secondary variants,
which are a compile-avoidance optimisation with no consumer here.
Approach suggested by @vlsi, after pgjdbc 49d4598.
juherr
force-pushed
the
build/gradle-9-jdk-25
branch
from
July 27, 2026 09:21
d0ff5d2 to
1e0b64c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
build-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.kt (2)
16-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComments likely cite the wrong Gradle version for the
registerFeature/usingSourceSet(main)removal.Both comments say "Gradle 10 removes registerFeature(name) { usingSourceSet(main) }". Per Gradle's deprecation notes, this pattern was scheduled to "fail with an error in Gradle 9.0" (not 10) — which is consistent with this PR needing the fix now, for the Gradle 9.6.1 upgrade, rather than pre-emptively for a future major version.
build-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.kt#L16-L23: update the KDoc to reference Gradle 9 (or drop the specific version and just say "Gradle removed this form").build-logic/publishing/src/main/kotlin/buildlogic/OptionalFeaturesExtension.kt#L74-L90: update the matching inline comment likewise.🤖 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 `@build-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.kt` around lines 16 - 23, Correct the Gradle version reference for the removed registerFeature/usingSourceSet(main) form: update the KDoc near OptionalFeatureVariants to say Gradle 9 or omit the specific version, and make the matching inline comment near OptionalFeaturesExtension consistent. Modify both specified files; no code behavior changes are needed.
61-77: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse the Provider-based
addVariantsFromConfigurationoverload to stay lazy.Gradle 9.2+ adds
AdhocComponentWithVariants.addVariantsFromConfiguration(Provider<ConsumableConfiguration>, Action)specifically so configurations "are only realized when needed for publishing." HereapiElements.get()/runtimeElements.get()force eager realization every timepublishOptionalFeatureruns underplugins.withId("maven-publish"), even for tasks that never touch these variants.♻️ Proposed fix using the lazy overload
plugins.withId("maven-publish") { components.named<AdhocComponentWithVariants>("java") { - addVariantsFromConfiguration(apiElements.get()) { + addVariantsFromConfiguration(apiElements) { mapToMavenScope("compile") mapToOptional() } - addVariantsFromConfiguration(runtimeElements.get()) { + addVariantsFromConfiguration(runtimeElements) { mapToMavenScope("runtime") mapToOptional() } } }🤖 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 `@build-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.kt` around lines 61 - 77, Update publishOptionalFeature to pass apiElements and runtimeElements directly to the Provider-based addVariantsFromConfiguration overload instead of calling .get(). Preserve the existing compile/runtime scope mapping and mapToOptional actions while ensuring both configurations remain lazily realized.
🤖 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.
Nitpick comments:
In `@build-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.kt`:
- Around line 16-23: Correct the Gradle version reference for the removed
registerFeature/usingSourceSet(main) form: update the KDoc near
OptionalFeatureVariants to say Gradle 9 or omit the specific version, and make
the matching inline comment near OptionalFeaturesExtension consistent. Modify
both specified files; no code behavior changes are needed.
- Around line 61-77: Update publishOptionalFeature to pass apiElements and
runtimeElements directly to the Provider-based addVariantsFromConfiguration
overload instead of calling .get(). Preserve the existing compile/runtime scope
mapping and mapToOptional actions while ensuring both configurations remain
lazily realized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d7e27622-3fa1-43bb-bab5-da5752341590
📒 Files selected for processing (6)
build-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.ktbuild-logic/publishing/src/main/kotlin/buildlogic/OptionalFeaturesExtension.ktbuild-logic/publishing/src/main/kotlin/testng.merge-feature-jars.gradle.ktstestng-core-api/testng-core-api-build.gradle.ktstestng-core/testng-core-build.gradle.ktstestng/testng-build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (1)
- build-logic/publishing/src/main/kotlin/testng.merge-feature-jars.gradle.kts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/CI_TEST_WORKFLOW.md (1)
549-560: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the documented matrix-job defaults consistent.
Line [549] sets
MATRIX_JOBSto8, but the workflow-dispatch example still usesdefault: '7'at Line [560]. Update the dispatch default to8, or explain why the two paths intentionally differ.🤖 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 `@docs/CI_TEST_WORKFLOW.md` around lines 549 - 560, Update the workflow_dispatch input default for matrix_jobs in the MATRIX_JOBS documentation example from 7 to 8 so it matches the documented MATRIX_JOBS value.
🤖 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.
Outside diff comments:
In `@docs/CI_TEST_WORKFLOW.md`:
- Around line 549-560: Update the workflow_dispatch input default for
matrix_jobs in the MATRIX_JOBS documentation example from 7 to 8 so it matches
the documented MATRIX_JOBS value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c4d21fd-ed19-4671-bfd7-6745145f30f2
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (45)
.github/CONTRIBUTING.md.github/workflows/combine-prs.yml.github/workflows/gradle-wrapper-validation.yml.github/workflows/label-commenter.yml.github/workflows/matrix.mjs.github/workflows/publish-maven-central.yml.github/workflows/publish-snapshot.yml.github/workflows/test.yml.github/workflows/update-gradle-wrapper.ymlCHANGES.txtbuild-logic-commons/gradle-plugin/build.gradle.ktsbuild-logic-commons/gradle-plugin/src/main/kotlin/build-logic.kotlin-dsl-gradle-plugin.gradle.ktsbuild-logic/basics/src/main/kotlin/testng.reproducible-builds.gradle.ktsbuild-logic/build-parameters/build.gradle.ktsbuild-logic/code-quality/build.gradle.ktsbuild-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.ktsbuild-logic/code-quality/src/main/kotlin/testng.testing.gradle.ktsbuild-logic/jvm/build.gradle.ktsbuild-logic/jvm/src/main/kotlin/buildlogic/OptionalFeatureVariants.ktbuild-logic/jvm/src/main/kotlin/testng.java-library.gradle.ktsbuild-logic/jvm/src/main/kotlin/testng.kotlin-library.gradle.ktsbuild-logic/publishing/build.gradle.ktsbuild-logic/publishing/src/main/kotlin/buildlogic/DependencyHandlerExtensions.ktbuild-logic/publishing/src/main/kotlin/buildlogic/OptionalFeaturesExtension.ktbuild-logic/publishing/src/main/kotlin/testng.local-maven-repo.gradle.ktsbuild-logic/publishing/src/main/kotlin/testng.maven-publish.gradle.ktsbuild-logic/publishing/src/main/kotlin/testng.merge-feature-jars.gradle.ktsbuild-logic/publishing/src/main/kotlin/testng.signing.gradle.ktsbuild-logic/settings.gradle.ktsbuild.gradle.ktsdocs/BUILD_SYSTEM.mddocs/CI_TEST_WORKFLOW.mddocs/JAVA_VERSIONS_QUICK_REFERENCE.mddocs/LOCAL_TESTING_GUIDE.mddocs/README.mddocs/RELEASE_PROCESS.mdgradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batsettings.gradle.ktstestng-core-api/testng-core-api-build.gradle.ktstestng-core/testng-core-build.gradle.ktstestng-test-osgi/testng-test-osgi-build.gradle.ktstestng/testng-build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (34)
- build-logic/code-quality/src/main/kotlin/testng.testing.gradle.kts
- build-logic/publishing/src/main/kotlin/buildlogic/DependencyHandlerExtensions.kt
- build-logic/basics/src/main/kotlin/testng.reproducible-builds.gradle.kts
- build-logic/publishing/build.gradle.kts
- build-logic/jvm/build.gradle.kts
- gradle/wrapper/gradle-wrapper.properties
- build-logic/code-quality/build.gradle.kts
- CHANGES.txt
- .github/workflows/publish-maven-central.yml
- settings.gradle.kts
- .github/workflows/label-commenter.yml
- build-logic/publishing/src/main/kotlin/testng.signing.gradle.kts
- build-logic/jvm/src/main/kotlin/testng.java-library.gradle.kts
- build-logic/publishing/src/main/kotlin/testng.local-maven-repo.gradle.kts
- gradlew.bat
- build-logic/build-parameters/build.gradle.kts
- build-logic/jvm/src/main/kotlin/testng.kotlin-library.gradle.kts
- testng-core-api/testng-core-api-build.gradle.kts
- docs/README.md
- build-logic/publishing/src/main/kotlin/testng.maven-publish.gradle.kts
- build-logic-commons/gradle-plugin/src/main/kotlin/build-logic.kotlin-dsl-gradle-plugin.gradle.kts
- build-logic/publishing/src/main/kotlin/buildlogic/OptionalFeaturesExtension.kt
- testng-test-osgi/testng-test-osgi-build.gradle.kts
- .github/CONTRIBUTING.md
- build-logic-commons/gradle-plugin/build.gradle.kts
- .github/workflows/matrix.mjs
- build.gradle.kts
- .github/workflows/publish-snapshot.yml
- docs/LOCAL_TESTING_GUIDE.md
- testng/testng-build.gradle.kts
- testng-core/testng-core-build.gradle.kts
- gradlew
- build-logic/code-quality/src/main/kotlin/testng.errorprone.gradle.kts
- build-logic/publishing/src/main/kotlin/testng.merge-feature-jars.gradle.kts
Also state the Gradle versions precisely: registerFeature with the main source set is deprecated since Gradle 8 and becomes an error in Gradle 10.
Member
Author
|
@vlsi Thanks for the links. I've addressed both of your suggestions. Could you please check if everything looks good? |
This was referenced Jul 28, 2026
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.
What
Moves the build to Gradle 9.6.1 on JDK 25, and clears every Gradle 10 deprecation that originates in this repository.
The bytecode target does not change.
targetJavaVersionstays at 11, so TestNG still runs on Java 11+ and this is not a breaking change for users. Only contributors are affected: building now requires JDK 25.This is the build half of #3288, rebased on current
masterand kept deliberately free of the source refactors in that PR so it can be reviewed on its own.Build
jdkBuildVersionandtargetJavaVersionare declared once, in the rootgradle.properties. Included builds do not inherit it, sobuild-logic-commonsreads the file from disk. Their fallback stays at 17 (the Kotlin DSL floor) rather than followingtargetJavaVersion, since the JVM target of the build scripts is unrelated to TestNG's bytecode target.dirMode/fileMode→dirPermissions/filePermissions,BuildIdentifier.isCurrentBuild()→buildPath, non-null manifest attributes,build-logic-commonsincluded viapluginManagement, vlsigradle-extensions1.90 → 3.0.2, Shadowcom.github.johnrengelman:8.1.1→com.gradleup.shadow:9.6.1.error_prone_core2.20 → 2.50 andgradle-errorprone-plugin3.1.0 → 4.1.0. The newSelfAssertioncheck is disabled: it only flags the intentionally trivial assertions in TestNG's own sample/fixture classes.Plugin updates
net.ltgt.errorprone4.1.0 → 5.1.0. Not just hygiene: Error Prone 2.46.0 onwards requires-XDaddTypeAnnotationsToSymbol=true, which the plugin only started passing in 4.4.0, so runningerror_prone_core2.50.0 on plugin 4.1.0 was below what the checker expects. None of the 5.0.0 breaking changes apply (min Gradle 7.1, min JDK 11, and theisEnabled/isCompilingTestOnlyCoderename touches no property this build sets).org.sonarqube4.4.1.3373 (2023) → 7.3.1.8318kotlin-bom2.3.0 → 2.4.10assertj-core3.23.1 → 3.27.7foojay-resolver-convention0.8.0 → 1.0.0org.gradlex.build-parameters1.4.4 → 1.4.5Autostyle stays at 4.0. 4.0.1 was tried and fails every module on JDK 25:
Gradle 10 deprecations
Every remaining deprecation that comes from this repository is fixed:
DependencyHandler.create(group, name, version)→ single-string notationby creating,by registering(T::class),by project)publishAllProjectsProbablyBreakingProjectIsolation(), which feedsProjectobjects to the dependency handler (see below)CI
JAVA_HOME); the publish workflows move from zulu 21 to temurin 25.MATRIX_JOBS7 → 8: thejava_versionaxis now has 7 values, so 7 jobs left the matrix exactly at capacity with no slack for the other axes to pair against.Three points worth reviewer attention
1. The Autostyle → Spotless swap was attempted and dropped
An earlier revision of this branch replaced Autostyle with Spotless, on the grounds that Autostyle 4.0 has been unmaintained since 2021. It was removed after Spotless proved flaky in this build, and Autostyle is kept as-is.
Under
org.gradle.parallel=true,spotlessJavaCheckfails intermittently — roughly 1 run in 6 ofclean build— with the formatter classloader collapsing mid-run:The class reported missing varies between runs (
com/google/common/collect/Iterators$MergingIterator,ImmutableList$SubList,common/base/Predicate, and google-java-format's ownRemoveUnusedImports), which points at a classloader being closed while still in use rather than at any one bad step.Measured across configurations, 12 runs of
clean build -x testeach:--no-parallelremoveUnusedImports("cleanthat-javaparser-unnecessaryimport")removeUnusedImports()apply falsedeclarationmaxParallelUsages = 1build serviceSpotless 8.8.0 is the latest release, so there is no upstream fix to pick up. Disabling parallel execution or serializing every formatter task is too high a price for a formatter swap, and shipping an intermittently red
checkwould be worse than keeping an unmaintained plugin.Tracked in #3292, which records the measurements above and the leads worth trying (applying Spotless from the root project only, so there is a single set of formatter tasks and no cross-project concurrency; reporting the classloader race upstream). Keeping Autostyle also keeps this PR's diff to build files: the swap dragged 86 reformatted Java files with it, because the google-java-format version bundled with Spotless reflows javadoc differently.
Note this does not hold back the configuration cache: Gradle 9 still keeps it off by default, and Spotless'
removeUnusedImportsis not configuration-cache compatible either.2. The nmcp aggregation was rewritten — please check this one
publishAllProjectsProbablyBreakingProjectIsolation()discovers publishing projects automatically, but it does so by handingProjectobjects to the dependency handler, a notation Gradle 10 removes. nmcp 1.6.1 (bumped here from 1.4.1) still has not fixed it.The projects to aggregate are therefore listed explicitly, which required applying
com.gradleup.nmcpin thetestng.maven-publishconvention. The obvious risk is that a future publishing module gets forgotten and silently disappears from a release, so aprojectsEvaluatedguard fails the build when a project appliesmaven-publishwithout being aggregated.Verified: the aggregation zip still holds the same 24 entries, the
:testngPOM andmodule.jsonare identical to the pre-change baseline (8 variants,testng-guice/testng-yamlcapabilities, same files), and the guard does fire when the aggregation entry is removed.3. Optional features on the main source set — deliberately NOT fixed
Two deprecations are left in place:
They come from
registerFeature(name) { usingSourceSet(sourceSets["main"]) }intestng,testng-coreandtestng-core-api, and fromOptionalFeaturesExtension.I started this and then backed out, because Gradle's prescribed replacement does not fit what the code is doing:
testng-core-api'sguiceApiElementsvariant serves the main jar. That is the entire point of the arrangement: same artifact, plus optional dependencies, surfacing as<optional>true</optional>in the POM.testng-core-api-guice.jar, and the guice dependencies would leavemain's compile classpath —GuiceHelperwould stop compiling unless the dependencies are also added ascompileOnly.merge-feature-jarsresolves into the shaded jar, which cannot be validated honestly without a real Central publication.This is a redesign of the optional-feature model rather than an API replacement, so it belongs in its own PR. Happy to take direction on the preferred shape.
Note: one deprecation is not ours to fix
These are emitted by
com.github.vlsi.gradle-extensions:3.0.2(PrintTestResults.kt:184), already the latest release. Nothing to do on our side until upstream migrates toaddTestListener(TestListener); the alternative would be dropping the plugin, which would also cost the improved test logging.Verification
./gradlew clean buildgreen on JDK 25 — 12 528 tests, 0 failures.clean build -x testrepeated 10 times with no failure, after the Spotless flakiness described above.testng-<version>-all.jarare major version 55 (Java 11).:testngPOM and Gradle Module Metadata compared against the pre-refactor baseline: identical.matrix.mjsgenerates a valid matrix over 15 seeded runs with no unsatisfied requirements; both early-access JDK pages are reachable.Notes
.github/CONTRIBUTING.mdanddocs/are updated accordingly, along with several pre-existing inaccuracies found on the way (a.sdkmanrcfile that does not exist, a test-runtime list mentioning Java 24, an nmcp error quoted as "Java 21+" instead of its actual "Java 17+", andmatrix.jsrenamed tomatrix.mjs).Summary by CodeRabbit
Build & Compatibility
Documentation
CI & Release Reliability