Repository navigation
Set the TestKit directory apart from TestKit coverage - #834
Merged
Merged
Conversation
`enableTestKitCoverage()` no longer passes the custom `io.spine.tools.gradle.testkit.coverage.home` property. It deals with the JaCoCo agent and the exec directory only. The new `useSharedTestKitDir()` sets Gradle's own `org.gradle.testkit.dir` on every `Test` task, at execution time, pointing it at `<Gradle user home>/caches/spine-testkit`. `module-testing` and `kmp-module` apply it, so every module gets it, whether it collects TestKit coverage or not. The tests of `buildSrc` itself use the same directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approved
There are no active review issues for the change separating shared TestKit directory configuration from coverage collection.
No open issues 🎉
What changed in this PR
This PR separates TestKit directory configuration from coverage collection in the shared Gradle build conventions, allowing tests to reuse caches independently of coverage settings.
Changes:
- Adds
useSharedTestKitDir()to configure Gradle’s standard TestKit directory property at execution time, preserving task-specific overrides. - Applies the shared directory to JVM and Kotlin Multiplatform test tasks, and separately to
buildSrctests. - Removes directory selection and the custom home property from
enableTestKitCoverage().
| File | Description |
|---|---|
buildSrc/src/main/kotlin/module-testing.gradle.kts |
Enables the shared TestKit directory for JVM module tests. |
buildSrc/src/main/kotlin/kmp-module.gradle.kts |
Applies the same configuration to multiplatform JVM tests. |
buildSrc/src/main/kotlin/io/spine/gradle/testing/TestKitDir.kt |
Defines the shared-directory helper and documents its behavior. |
buildSrc/src/main/kotlin/io/spine/gradle/testing/TestKitCoverage.kt |
Limits the coverage helper to coverage-related configuration. |
buildSrc/build.gradle.kts |
Configures buildSrc tests to use the shared TestKit directory. |
Review effort: Balanced
Only local runs would gain from sharing it, as no CI workflow runs them. That does not justify a second copy of the path to keep in sync. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 of 9 tasks
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
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
This PR separates two concerns that #831 bundled together: collecting TestKit coverage, and choosing where TestKit builds keep their Gradle user home.
enableTestKitCoverage()deals with coverage only again. It passes the JaCoCo agent JAR and the exec directory to the test JVM, as it did before Share one TestKit directory among modules for TestKit coverage #831. The customio.spine.tools.gradle.testkit.coverage.homeproperty (TESTKIT_HOME_PROPERTY) is gone. It duplicated Gradle's ownorg.gradle.testkit.dirproperty, whichGradleRunner.create()reads whenever a runner gets no explicitwithTestKitDir(...). ThewithTestKitDirJavadoc documents that property for the test process.Project.useSharedTestKitDir()inio/spine/gradle/testing/TestKitDir.ktsetsorg.gradle.testkit.dirto<Gradle user home>/caches/spine-testkiton everyTesttask of the project. It applies whether or not a task runs TestKit builds, and whether or not the module enables coverage.Where it is applied, and why there
Every JVM module of the SDK repositories applies the
module-testingscript plugin.jvm-module,test-module, and each consumer's ownmodule.gradle.ktsapply it, anduber-jar-modulegets it throughmodule. That plugin already owns the test setup of a module (setupTests(),registerTestTasks(),forceTestDependencies()), so itsproject.run { ... }block now also callsuseSharedTestKitDir().kmp-modulecannot applymodule-testing, because the latter bringsjava-library. Instead, it mirrors that plugin's test setup forjvmTest, so it calls the function too.The logic itself lives in
io.spine.gradle.testingbesideTestKitCoverage.ktandSpineCompilerCoverage.kt. Like them, it is aProjectextension that configurestasks.withType<Test>().Not a task input
The property is set from a
doFirstaction, as the two coverage helpers in the package do for their paths. The absolute path differs between machines (D:\a\.gradle\...on a Windows runner,/home/runner/.gradle/...on Linux). As aTesttask input, it would only make the build-cache keys differ, because where TestKit builds keep their caches does not affect the test results. ATesttask that already has the property, set by a module's own build script, keeps its value.What the shared directory holds
A TestKit directory is the Gradle user home of the builds that
GradleRunnerruns. The local shared one holds mostly Gradle's instrumented copies of the JARs on the classpath of the plugin under test (caches/9.8.2/transforms, about 200 MB). It also holds the compiled build scripts of the test projects (caches/9.8.2/kotlin-dsl, about 36 MB). It holds no generated Gradle API JARs. Without the property, TestKit usesbuild/tmp/<task>/work/.gradle-test-kit, so everyTesttask of every module, everyclean, and every fresh CI checkout starts empty. The directory stays undercaches/, whichgradle/actions/setup-gradlesaves on the default branch (gradle-home-cache-includes: cachesby default) and pull-request runs restore.buildSrc's own tests are left alonebuildSrc's IgTests keep TestKit's default directory. No CI workflow ofconfigruns them, so sharing it would only help local runs. That doesn't justify a second copy of the path inbuildSrc/build.gradle.kts, which can't usebuildSrccode.Compatibility with
plugin-testlibplugin-testlibversions (up to the one on tool-basemaster) callwithTestKitDir(<module>/build/testkit-home)explicitly when coverage is on. That call overrides the system property, so they are unaffected. TestKit tests that don't choose a directory now use the shared one.io.spine.tools.gradle.testkit.coverage.home. Only the original draft of Pass the TestKit coverage agent on the command line, move the shared TestKit directory tool-base#207 read it, so removing the property breaks nothing released.-Dorg.gradle.jvmargs=-javaagent:...to the runner's arguments. With this PR, its runners get the shared directory throughorg.gradle.testkit.dir. Without this PR, TestKit falls back to its default per-task directory, so it still works. This PR and Default GitHub token #207 can merge in either order.BuildCacheSpecin tool-base points its test project at a build-cache folder inside that project, so a persistent shared Gradle home is safe for it.Verification
./gradlew :buildSrc:build detektpasses: 325 tests, 0 failures.useSharedTestKitDir()to a plainjavaproject with JUnit andgradleTestKit(), run with Gradle 9.8.2:org.gradle.testkit.dir=<Gradle user home>/caches/spine-testkit, and aGradleRunnerbuild inside the test writes itscaches/there.--configuration-cache, the second run reuses the entry, and thedoFirststill sets the property.cleanTest testunder a different Gradle user home (so a different absolute path) gets:test FROM-CACHE. The path is not part of the cache key, and thedoFirstaction keepsTesttasks cacheable. As a control, the same fixture withsystemProperty(...)set at configuration time re-executes:testunder the second Gradle user home.gradle-review,kotlin-engineer, andreview-docsreviewers. Their findings are addressed.🤖 Generated with Claude Code