Add support for running GradlePluginTests with configuration-cache #245

crogoz · 2025-10-27T16:23:46Z

Before this PR

Groovy Test spec that runs with Configuration Cache enabled.

After this PR

Ability to run GradlePluginTests with "--configuration-cache" enabled when the extension properrty gradleTestUtils.isConfigurationCacheEnabled is set.

Based on the value of the configuration cache, we will instantiate a DefaultGradleInvoker or a ConfigurationCacheInvoker.

The ConfigurationCacheInvoker will do:

run the tasks with --configuration-cache added to the list of arguments. It will throw an UnexpectedConfigurationCacheFailure exception if the configuration cache was not stored/failed.
run the tasks above including --configuration-cache & --dry-run to make sure the configuration cache was reused. It will throw an UnexpectedConfigurationCacheFailure if not.

Added a new exception type UnexpectedInvocationResult with the corresponding UnexpectedInvocationFailure & UnexpectedInvocationSuccess that are wrappers on top of gradle testkit's exceptions UnexpectedBuildFailure & UnexpectedBuildSuccess.

Added a new DisabledConfigurationCache which will disable per test class/method the configuration-cache. This will be useful later when we are migrating the GradlePluginTests to use by default the configuration-cache and we will need to disable the tests that are incompatible with cc.

==COMMIT_MSG==
Ability to run GradlePluginTests with configuration-cache.
==COMMIT_MSG==

Possible downsides?

changelog-app · 2025-10-27T16:23:50Z

Generate changelog in `changelog/@unreleased`

Type (Select exactly one)

Feature (Adding new functionality)
Improvement (Improving existing functionality)
Fix (Fixing an issue with existing functionality)
Break (Creating a new major version by breaking public APIs)
Deprecation (Removing functionality in a non-breaking way)
Migration (Automatically moving data/functionality to a new system)

Description

Add support for running GradlePluginTests with configuration-cache

Check the box to generate changelog(s)

Generate changelog entry

crogoz · 2025-10-27T16:25:14Z

...ugin-testing-junit/src/main/java/com/palantir/gradle/testing/execution/GradleInvocation.java

+                    .withEnvironment(gradleRunner.getEnvironment())
+                    .withArguments(ImmutableList.<String>builder()
+                            .addAll(gradleRunner.getArguments())
+                            .add("--dry-run")


for config-cache to be reused, we need to run the same tasks. In order to avoid running the same tasks twice, we can pass a --dry-run.
Tested that config cache is triggered by a dry-run:

Reusing configuration cache. :compileJava SKIPPED :help SKIPPED BUILD SUCCESSFUL in 62ms Configuration cache entry reused.

crogoz · 2025-10-27T16:25:26Z

...ugin-testing-junit/src/main/java/com/palantir/gradle/testing/execution/GradleInvocation.java

+            assertConfigCacheReused(configurationCacheResult.output());
+        }
+
+        return result;


always returning the result of the first invocation.

CRogers · 2025-10-27T16:38:06Z

...ugin-testing-junit/src/main/java/com/palantir/gradle/testing/execution/GradleInvocation.java

 public final class GradleInvocation {
    private final GradleRunner gradleRunner;
+    private final GradleVersion gradleVersion;
+    private final boolean configurationCacheEnabled;


Rather than taking a boolean and doing if statements, can we instead try and change GradleInvoker and GradleInvocation into interfaces, and have a BasicGradleInvoker and a ConfigurationCacheGradleInvoker (likely the same for GradleInvocation too). The configuration cache versions should just be able to call the basic versions as many times as they need - we might be able to avoid copying the code for creating the GradleRunner again on line 50 too.

CRogers · 2025-10-27T16:40:14Z

...in-testing-junit/src/test/java/com/palantir/gradle/testing/junit/ConfigurationCacheTest.java

+final class ConfigurationCacheTest {
+
+    @Test
+    void testConfigurationCache(GradleInvoker invoker, RootProject rootProject) {


Test methods should not start with test, it's superfluous. Additionally, test names should be a snake_case_english_sentence.

I actually think we should make an errorprone check for this, to ensure that people do this for our Gradle plugins tests.

FinlayRJW · 2025-10-27T16:44:34Z

...ugin-testing-junit/src/main/java/com/palantir/gradle/testing/execution/GradleInvocation.java

+
+            // Run a second time to verify the cache is reused, we don't need to run the tasks --dry-run is enough to
+            // check if the configuration cache was used.
+            GradleRunner configurationCacheRunner = GradleRunner.create()


I've seen cases where the second run doesn't reuse the cache but the subsequent do - normally related to file generation. I'm not sure if that behaviour is ever valid so the solution might just be that we want to fail those cases but could also be nice to have some kind of ratchet - although in those edge cases we could just get users to write there CC tests

I'm pretty sure that was the case here https://pl.ntr/2xX - but its possible I also just didn't fix it properly!

CRogers · 2025-10-27T16:45:41Z

...in-testing-junit/src/test/java/com/palantir/gradle/testing/junit/ConfigurationCacheTest.java

+                record MyClass() {}
+            """);
+
+        InvocationResult result = invoker.withArgs("compileJava", "help").buildsSuccessfully();


I'm not sure what this is currently testing?

In terms of tests, I think we could have at least a few tests:

In the build.gradle, print whether the configuration cache is enabled and check it is disabled when @WithConfigurationCache is not present, and is enabled when present. You can use the start parameter / configuration cache build feature. This actually raises a good questions - we might not want to run with configuration on Gradle 7, as the implementation and error messages are bad.

Fails when there is a basic configuration cache error eg executing a process at configuration time

Fails when there is an issue storing the cache entry

Fails when there is an issue loading the cache entry

Succeeds when there are no configuration cache issues

CRogers · 2025-10-27T16:50:59Z

...ugin-testing-junit/src/main/java/com/palantir/gradle/testing/execution/GradleInvocation.java

+                        "Running @WithConfigurationCache: Expected configuration cache entry to be reused, but it"
+                                + " wasn't. Output: %s",


I think, especially for the second invocation, we need a really good error message that is very explicit in what happened. We need to tell the dev that first invocation worked and stored the cache entry, but then we ran again with --dry-run to verify that loading the cache still worked and this failed, and how they should diagnose it (look at the report? build scan? can we parse these from the output and put it in the error message?)

CRogers · 2025-10-27T16:51:16Z

...ugin-testing-junit/src/main/java/com/palantir/gradle/testing/execution/GradleInvocation.java

+        File configurationCacheDir = new File(gradleRunner.getProjectDir(), ".gradle/configuration-cache");
+        assertThat(configurationCacheDir.exists())
+                .as(String.format(
+                        "Running @WithConfigurationCache: Expected the configuration-cache %s directory to exits.",


s/exits/exist

CRogers · 2025-10-27T17:08:34Z

...in-testing-junit/src/main/java/com/palantir/gradle/testing/junit/WithConfigurationCache.java

+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@ExtendWith(ConfigurationCacheExtension.class)
+public @interface WithConfigurationCache {}


In terms of actually rolling this out, not sure how I feel about opting in (like this with @WithConfigurationCache) vs a global opt in at the Gradle extesion level and opting out with @ConfigurationCacheDisabled. Or even further - default having configuration cache on without any Gradle configuration and relying on a disable annotation.

My worry with the opting in is people will just forget to add it to new tests and we want to ensure all new code/test code is configuration cache compatible.

But in order for upgrades to not get blocked with opt out, we need to work out a way to feed back test failures to the excavator which will enable it to add @ConfigurationCacheDisabled onto the relevant tests/test classes. Opt-in, it's a little easier as you can make a parameterised excavator that tries to opt in each test class, then the excavator will just not merge if it doesn't work.

My thinking was to add this flag to all tests that are extending ConfigurationCacheSpec example here. This way we can do the migration of the nebula tests -> new java framework 1-1.

Then I think it would be pretty easy to add an extension similar to how we set the Gradle versions and make it by default using the CC and add a workflow to add the @ConfigurationCacheDisabled. But I am thinking that would be a separate migration from the nebula test -> new Java framework. wdyt ?

changelog-app · 2025-10-31T11:06:48Z

✅ Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.

📋Changelog Preview

💡 Improvements

Add support for running GradlePluginTests with configuration-cache (#245)

crogoz · 2025-10-31T11:10:44Z

...-testing-junit/src/main/java/com/palantir/gradle/testing/execution/DefaultGradleInvoker.java

+        // `withDebug(true)` will run the Gradle tooling inside the same JVM as the test, meaning
+        // debugging works, whereas `withDebug(false)` will run Gradle in a new daemon. There can be
+        // differences between these two modes!
+        return ManagementFactory.getRuntimeMXBean().getInputArguments().stream()


FLUP: cc is not compatible with testkit's debugging option see issue: gradle/issues/25846

Good catch, we should probably just disable configuration cache when debugging and paste a warning log line that we've done so in each test. This is what we do in suppressible-error-prone, would be great to get it in the framework directly.

crogoz · 2025-10-31T11:18:34Z

...n-testing-junit/src/test/java/com/palantir/gradle/testing/integration/ProjectUsagesTest.java

-        assertThat(gradle.withArgs().buildsSuccessfully().output()).contains("project name: root");
+        InvocationResult invocation = gradle.withArgs().buildsSuccessfully();
+        assertThat(invocation.output()).contains("hello from :");
+        assertThat(invocation.output()).contains("project name: root");


I've enabled cc here. Previously we were running the tasks twice, which means that cc will not run the second time - hence the second run will not contain the expected output lines. Same below.

Oh, interesting! I have a feeling this more than anything may break existing tests that use printlns at configuration time a lot (though I guess they should be doing it within task actions if they were good).

I wonder if we wanted tests to be more or less configuration cache agnostic whether we delete the configuration cache dir after checking it can be reloaded with --dry-run. I'm not sure if it's a good idea, but it would still check configuration cache but run the configuration every time. Feels a little dodgy though, maybe we just keep it as an option in case converting existing tests is hard?

crogoz · 2025-10-31T11:19:23Z

gradle-plugin-testing-junit/build.gradle


+tasks.named('test', Test) {
+    // by default the tests should run with configuration cache enabled.
+    systemProperty 'com.palantir.gradle.testing.configuration_cache_enabled', 'true'


Once we publish this version, I'll add the isConfigurationCacheEnabled = true in the gradleTestutils extension

CRogers · 2025-10-31T12:46:37Z

...ing-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvoker.java

+import com.palantir.gradle.testing.RestrictedCreation;
+import java.nio.file.Path;
+
+public record ConfigurationCacheInvoker(Path projectDir, GradleInvoker gradleInvoker) implements GradleInvoker {


We should make this a regular class so we can hide .gradleInvoker() etc

...ing-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvoker.java

CRogers · 2025-10-31T12:58:08Z

...it/src/main/java/com/palantir/gradle/testing/junit/ConfigurationCacheParameterExtension.java

+    @Override
+    public void beforeAll(ExtensionContext context) {
+        setConfigurationCache(context);
+    }
+
+    @Override
+    public void beforeEach(ExtensionContext context) {
+        setConfigurationCache(context);
+    }


I wonder if it's safer to just lazily compute whether configuration cache is enabled or not in the store rather than setting them here - can't forget to set it in all the relevant locations then.

I guess that's a bit strange, as you need to look at the enclosing elements to find configuration cache, and how would we handle a @ConfigurationCacheDisabled arg on a test when there's a @WithConfigurationCache on the test class...

how would we handle a @ConfigurationCacheDisabled arg on a test when there's a @WithConfigurationCache on the test class...

I removed this class in the latest commits. I only kept the @DisableConfigurationCache

...n-testing-junit/src/test/java/com/palantir/gradle/testing/junit/ConfigurationCacheTests.java

...-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvocation.java

...unit/src/test/java/com/palantir/gradle/testing/ete/GradleVersionsFromJunitParameterTest.java

Copilot

Pull Request Overview

This PR adds configuration cache support to GradlePluginTests, enabling tests to verify that Gradle plugins are compatible with configuration cache functionality. The implementation provides both the ability to enable configuration cache by default and to selectively disable it for incompatible tests.

Introduces configuration cache support with automatic verification of cache storage and reuse
Adds @DisabledConfigurationCache annotation for excluding incompatible tests
Refactors the invoker architecture to support both standard and configuration cache execution modes

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`ConfigurationCacheHelper.java`	Defines system property constant for configuration cache enablement
`PluginTestingPlugin.java`	Adds system property configuration for tests to use configuration cache setting
`PluginTestingExtension.java`	Adds `configurationCacheEnabled` property with default false value
Multiple test files	Comprehensive test coverage for configuration cache functionality and edge cases
`GradleInvoker.java`	Converts from class to interface with factory method for invoker selection
`DefaultGradleInvoker.java`	Implements standard Gradle invocation without configuration cache
`ConfigurationCacheInvoker.java`	Implements configuration cache-aware Gradle invocation with verification
Exception classes	New exception hierarchy for handling configuration cache and invocation failures
JUnit integration files	Extensions and stores for managing configuration cache state in test context

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...it/src/main/java/com/palantir/gradle/testing/junit/ConfigurationCacheParameterExtension.java

...ing-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvoker.java

…testing/junit/ConfigurationCacheParameterExtension.java Co-authored-by: Copilot <[email protected]>

…testing/execution/ConfigurationCacheInvoker.java Co-authored-by: Copilot <[email protected]>

…le-plugin-testing into cr/tests-with-config-cache

CRogers · 2025-11-04T16:25:06Z

...-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvocation.java

+    @Override
+    public InvocationResult buildsWithFailure() {
+        InvocationResult result = initialGradleInvocation.buildsWithFailure();
+        assertConfigCacheStored(result);


What happens if it fails at configuration time, before executing any tasks? I think this is allowed by buildsWithFailure but the configuration cache entry won't be stored. Should have a test for it at least. Probably want to only check this if a task was executed/it was a task that failed.

CRogers · 2025-11-04T16:25:52Z

...-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvocation.java

+    public InvocationResult buildsWithFailure() {
+        InvocationResult result = initialGradleInvocation.buildsWithFailure();
+        assertConfigCacheStored(result);
+        return result;


I wonder if we actually want to check we can load configuration cache here if it was a task that failed. If the configuration was fine but a task failed, it should be fine to run with --dry-run (I think?)?

Worth writing a test for.

CRogers · 2025-11-04T16:30:46Z

...ing-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvoker.java

+    @Override
+    public GradleInvocation withArgs(String... args) {
+        // not reusing configuration-cache among multiple gradle invocations in a test.
+        cleanupConfigurationCache();


Should we do this here or after a gradle invocation? I guess this might be a bit safer.

crogoz added 3 commits October 27, 2025 11:50

test?

a1edf36

.

09a40ef

.

d5880e1

crogoz commented Oct 27, 2025

View reviewed changes

crogoz marked this pull request as ready for review October 27, 2025 16:26

crogoz requested a review from CRogers October 27, 2025 16:26

CRogers reviewed Oct 27, 2025

View reviewed changes

FinlayRJW reviewed Oct 27, 2025

View reviewed changes

CRogers reviewed Oct 27, 2025

View reviewed changes

crogoz added 6 commits October 29, 2025 16:25

wip

b8a7321

extension config cache

e24bb86

fix tests

572d8cc

wip

69fded3

tests

33a279e

fix

60d02f2

crogoz force-pushed the cr/tests-with-config-cache branch from 5233348 to 60d02f2 Compare October 31, 2025 10:37

fixes

f98146b

crogoz changed the title ~~@WithConfigurationCache Annotation for GradlePluginTests~~ Add support for running GradlePluginTests with configuration-cache Oct 31, 2025

crogoz commented Oct 31, 2025

View reviewed changes

crogoz added 2 commits October 31, 2025 13:22

small cleanup

f4eee87

rename

cc19002

fix

2606885

CRogers reviewed Oct 31, 2025

View reviewed changes

...ing-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvoker.java Show resolved Hide resolved

CRogers reviewed Oct 31, 2025

View reviewed changes

...n-testing-junit/src/test/java/com/palantir/gradle/testing/junit/ConfigurationCacheTests.java Outdated Show resolved Hide resolved

CRogers reviewed Oct 31, 2025

View reviewed changes

...-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvocation.java Show resolved Hide resolved

CRogers reviewed Oct 31, 2025

View reviewed changes

...unit/src/test/java/com/palantir/gradle/testing/ete/GradleVersionsFromJunitParameterTest.java Show resolved Hide resolved

address comments

2ea83ba

crogoz requested review from CRogers, FinlayRJW and Copilot October 31, 2025 16:41

Copilot AI reviewed Oct 31, 2025

View reviewed changes

...it/src/main/java/com/palantir/gradle/testing/junit/ConfigurationCacheParameterExtension.java Outdated Show resolved Hide resolved

...ing-junit/src/main/java/com/palantir/gradle/testing/execution/ConfigurationCacheInvoker.java Outdated Show resolved Hide resolved

crogoz and others added 7 commits October 31, 2025 17:50

Update gradle-plugin-testing-junit/src/main/java/com/palantir/gradle/…

3bf2a87

…testing/junit/ConfigurationCacheParameterExtension.java Co-authored-by: Copilot <[email protected]>

Update gradle-plugin-testing-junit/src/main/java/com/palantir/gradle/…

ef1f86f

…testing/execution/ConfigurationCacheInvoker.java Co-authored-by: Copilot <[email protected]>

rm custom exceptions

612e1cd

Merge branch 'cr/tests-with-config-cache' of github.com:palantir/grad…

27ac740

…le-plugin-testing into cr/tests-with-config-cache

cleanup

362d09d

cleanup

cc4d156

make it config-compat

99e4f8f

CRogers reviewed Nov 4, 2025

View reviewed changes

add tests

663610f

		"Running @WithConfigurationCache: Expected configuration cache entry to be reused, but it"
		+ " wasn't. Output: %s",

Add support for running GradlePluginTests with configuration-cache #245

Are you sure you want to change the base?

Add support for running GradlePluginTests with configuration-cache #245

Uh oh!

Conversation

crogoz commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this PR

After this PR

Possible downsides?

Uh oh!

changelog-app bot commented Oct 27, 2025 • edited by crogoz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generate changelog in changelog/@unreleased

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

changelog-app bot commented Oct 31, 2025

✅ Successfully generated changelog entry!

Need to regenerate?

📋Changelog Preview

💡 Improvements

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crogoz Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CRogers Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crogoz commented Oct 27, 2025 •

edited

Loading

changelog-app bot commented Oct 27, 2025 •

edited by crogoz

Loading

Generate changelog in `changelog/@unreleased`

crogoz Oct 31, 2025 •

edited

Loading

CRogers Oct 31, 2025 •

edited

Loading