Make AGP and KGP deps compileOnly #1105
Conversation
Dependencies on other gradle plugins should be `compileOnly` dependencies, as it can accidentally impose the version paparazzi depends on rather than just use the one that the consuming project uses. For example – in our project, this transitive dep prevents us from testing newer AGP versions due to multiple versions appearing on the buildscript classpath.
paparazzi-gradle-plugin/build.gradle
Outdated
| testImplementation libs.plugin.kotlin | ||
| testImplementation libs.plugin.android | ||
| testImplementation(libs.tools.sdkCommon) { |
There was a problem hiding this comment.
Were you trying to replace the runtimeOnly part of the implementation with these to help Gradle TestKit?
If so, I hacked around this area of Gradle before, and got it working.
A likely cause for the CI failure is PluginUnderTestMetadata.getPluginClasspath, because the default only uses production runtime classpath.
Here's my solution: definition and usage.
(If you use this, you could probably ignore my version constraint usage, I needed it because I have AGP/KGP in the test matrix.)
There was a problem hiding this comment.
The above should work more or less without any hacks, not sure why CI is failing. WIll look more later
Is the newer plugin version check a new thing? We've definitely been able to pull in a newer AGP version on CashApp, with the risk of binary incompatibilities resulting in Paparazzi crashes, but that's different from your error message. Re: the premise of |
|
accidentally hit "Close with comment", just meant to "Comment". My bad. |
It's not a check, it's just how gradle and classloading work. It's a first-one-wins policy in many cases due to the hierarchical nature of gradle and generally just good plugin hygiene to not impose other plugin versions transitively. Things get tricky when you start assuming things about versions when a dependency appears sooner in that hierarchy (i.e. such as settings.gradle classpath) and later projects assume their dependency will win or be resolved with others. |
| /paparazzi-gradle-plugin/src/test/projects/rerun-resource-change/src/main/res/values/colors.xml | ||
| /paparazzi-gradle-plugin/src/test/projects/rerun-asset-change/src/main/assets/secret.txt |
There was a problem hiding this comment.
These were getting generated as untracked files when I ran tests locally, so opportunistically adding them here
| Configuration addTestPlugin = configurations.create("addTestPlugin") | ||
|
|
||
| configurations { | ||
| testImplementation.extendsFrom(addTestPlugin) | ||
| } | ||
|
|
||
| tasks.pluginUnderTestMetadata { | ||
| // make sure the test can access plugins for coordination. | ||
| pluginClasspath.from(addTestPlugin) | ||
| } |
There was a problem hiding this comment.
This pattern is what we use in Keeper to test against non-bundled KGP/AGP deps
Dependencies on other gradle plugins should be
compileOnlydependencies, as it can accidentally impose the version paparazzi depends on rather than just use the one that the consuming project uses. For example – in our project, this transitive dep prevents us from testing newer AGP versions due to multiple versions appearing on the buildscript classpath.