JGiven icon indicating copy to clipboard operation
JGiven copied to clipboard

Gradle plugin not compatible with Gradle configuration cache

Open jjohannes opened this issue 1 year ago • 12 comments

The Configuration Cache feature of Gradle requires a strict separation between configuration time and execution time state.

The JGiven Gradle plugin is violating this right now by postponing the access to some configuration state to execution time by using Java Callables in JGivenPlugin to delay access to certain configuration values. For this kind of "lazy access" Gradle's Properties and Providers would be the better solution.

To reproduce the problem, run gradle jgivenTestReport --configuraion-cache on a Gradle project that applies the com.tngtech.jgiven.gradle-plugin plugin.

Trace from the configuration cache problem report:

org.gradle.api.InvalidUserCodeException: Execution of task ':entsendung.biz.qs:jgivenTestReport' caused invocation of 'Task.extensions' by task ':entsendung.biz.qs:test' at execution time which is unsupported.
	at org.gradle.configurationcache.problems.DefaultProblemFactory$problem$1$build$diagnostics$1.get(DefaultProblemFactory.kt:86)
	at org.gradle.configurationcache.problems.DefaultProblemFactory$problem$1$build$diagnostics$1.get(DefaultProblemFactory.kt:86)
	at org.gradle.internal.problems.DefaultProblemDiagnosticsFactory$DefaultProblemStream.getImplicitThrowable(DefaultProblemDiagnosticsFactory.java:111)
	at org.gradle.internal.problems.DefaultProblemDiagnosticsFactory$DefaultProblemStream.forCurrentCaller(DefaultProblemDiagnosticsFactory.java:100)
	at org.gradle.configurationcache.problems.DefaultProblemFactory$problem$1.build(DefaultProblemFactory.kt:86)
	at org.gradle.configurationcache.initialization.DefaultConfigurationCacheProblemsListener.onTaskExecutionAccessProblem(ConfigurationCacheProblemsListener.kt:134)
	at org.gradle.configurationcache.initialization.DefaultConfigurationCacheProblemsListener.onConventionAccess(ConfigurationCacheProblemsListener.kt:81)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.event.DefaultListenerManager$ListenerDetails.dispatch(DefaultListenerManager.java:472)
	at org.gradle.internal.event.DefaultListenerManager$ListenerDetails.dispatch(DefaultListenerManager.java:454)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:83)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:69)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast$ListenerDispatch.dispatch(DefaultListenerManager.java:443)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast$ListenerDispatch.dispatch(DefaultListenerManager.java:431)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:43)
	at org.gradle.internal.event.AbstractBroadcastDispatch.dispatch(AbstractBroadcastDispatch.java:66)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast$ListenerDispatch.dispatch(DefaultListenerManager.java:443)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast.dispatch(DefaultListenerManager.java:232)
	at org.gradle.internal.event.DefaultListenerManager$EventBroadcast.dispatch(DefaultListenerManager.java:203)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy88.onConventionAccess(Unknown Source)
	at org.gradle.configurationcache.AbstractTaskProjectAccessChecker.notifyConventionAccess(TaskExecutionAccessCheckers.kt:45)
	at org.gradle.api.internal.AbstractTask.notifyConventionAccess(AbstractTask.java:1073)
	at org.gradle.api.internal.AbstractTask.getConventionVia(AbstractTask.java:604)
	at org.gradle.api.internal.AbstractTask.getExtensions(AbstractTask.java:600)
	at org.gradle.api.DefaultTask.getExtensions(DefaultTask.java:324)
	at com.tngtech.jgiven.gradle.JGivenPlugin.lambda$configureDefaultReportTask$5(JGivenPlugin.java:85)
	at org.gradle.util.internal.GUtil.uncheckedCall(GUtil.java:437)

jjohannes avatar Feb 01 '24 10:02 jjohannes

Thanks for the report and the hints at how to investigate the issue. Ill tackle this with priority

l-1squared avatar Feb 01 '24 15:02 l-1squared

Thank you @l-1squared. If you have any questions, let me know. I can also review a PR.

jjohannes avatar Feb 01 '24 15:02 jjohannes

Hi, I finally got around to tackling this a second time (after my failure at the beginning of the week). I think I understood now what went wrong in my previous attempt, but I lack the knowledge to find the solution. And here I could use your help.

So, the gradle documentation suggests that you use properties and providers for "everything", which prompted me to alter the JGivenTaskExtension in the following way:

public interface JGivenTaskExtension {
    Property<File> getResultsDir();

}

Unfortunately, upon testing, this produces an UnknownServiceException no service of type object factory available in default services.

I tried to debugv this and the error seems to happen, because we create the extensions in JGivenPlugin.java:38 on a Task, and not on the Project.

Thing is, that I want to create them on the task, which puts me at an impasse, because I don't know how to supply an object factory to my extension.

l-1squared avatar Feb 09 '24 13:02 l-1squared

That sounds like you are on the right track. The error is unexpected though. Maybe it's just some detail that is not right. It's unfortunately often too easy to get something wrong with these APIs.

If you could push what you have to a branch, I am happy to take a look.

jjohannes avatar Feb 10 '24 12:02 jjohannes

sure: https://github.com/TNG/JGiven/tree/feature/separate-plugin-config-and-execution

l-1squared avatar Feb 12 '24 05:02 l-1squared

Ah, I finally happened upon this: https://github.com/gradle/gradle/issues/16936. I'll try it

l-1squared avatar Feb 12 '24 05:02 l-1squared

hmm, maybe not. But it gave me an idea: I could create the extension via the object factory and then simply add to the task. (Did so in my WIP commit) I might be on to something...

l-1squared avatar Feb 12 '24 06:02 l-1squared

Okay it looks like you are right. Unfortunately task extensions do not seem to be handled that easily as "normal" (project) extensions. But the workaround suggested in https://github.com/gradle/gradle/issues/16936#issuecomment-857899385 looks slightly wrong. That syntax does not work for me.

I adjusted everything I would in this commit: https://github.com/jjohannes/JGiven/commit/cc3f67a7b4b39f9f78599c2cea18c42c95870c73

Feel free to pick parts you need from there.

Unfortunately, now the configuration cache is still failing with issues with the TaskReportContainer which looks somethign Gradle internal. But other reporting task also use that. So it should be usable somehow.

jjohannes avatar Feb 12 '24 11:02 jjohannes

Okay it looks like you are right. Unfortunately task extensions do not seem to be handled that easily as "normal" (project) extensions. But the workaround suggested in gradle/gradle#16936 (comment) looks slightly wrong. That syntax does not work for me.

I adjusted everything I would in this commit: jjohannes@cc3f67a

Feel free to pick parts you need from there.

Unfortunately, now the configuration cache is still failing with issues with the TaskReportContainer which looks somethign Gradle internal. But other reporting task also use that. So it should be usable somehow.

Thanks for the refactoring which helped me out a great deal. Aside from the issue you already mentioned, I also removed a null checked which seemed to be important. Hope I'll get both sorted out quickly

l-1squared avatar Feb 13 '24 06:02 l-1squared

Unfortunately, this will still take a while. The Report and Container both use task based internal classes and especially the public api of the reports container has A TON of methods to implement :(

Also there is another test failure about the test task running, when it shouldn't that I don't understand yet.

l-1squared avatar Feb 15 '24 06:02 l-1squared

I am not sure if extending the (internal) ReportContainer implementation is the issue here. I also don't know about an alternative solution. Implementing all "DomainObjectContainer" methods yourself can't be it. It feels to me like public API is missing and what you are doing is OK.

I have looked at this again and TBH I don't understand the error. I can't finde the reference in the code the error is complaining about. For reference, this is the trace from the CC report:

CC

I'll see if I can get someone from Gradle to have a look.

jjohannes avatar Feb 16 '24 08:02 jjohannes

Good news @l-1squared.

@mlopatkin helped me to track down the problem. It is totally unrelated to the "internal" container implementation. You do not need to change anything there.

This is the fix: https://github.com/TNG/JGiven/commit/37c736e1c2e6ff524c7dd1060489e61bc4a8c94a

jjohannes avatar Feb 16 '24 11:02 jjohannes