autonomousapps / dependency-analysis-gradle-plugin

Gradle plugin for JVM projects written in Java, Kotlin, Groovy, or Scala; and Android projects written in Java or Kotlin. Provides advice for managing dependencies and other applied plugins
Apache License 2.0
1.68k stars 114 forks source link

Advised to change KGP-added `testSuiteNameApi` dependency on kotlin-stdlib to `testSuiteNameImplementation` with JVM Test Suite Plugin #1059

Closed ianbrandt closed 2 months ago

ianbrandt commented 7 months ago

Build scan link https://scans.gradle.com/s/dbkbjeg4dv4og/

Plugin version 1.27.0

Gradle version 8.5

JDK version 17.0.8.1

(Optional) Kotlin and Kotlin Gradle Plugin (KGP) version 1.9.21

(Optional) Android Gradle Plugin (AGP) version N/A

(Optional) reason output for bugs relating to incorrect advice

PS C:\Dev\Repos\SDKotlin\sd-kotlin-talks> .\gradlew :di-with-koin:reason --id org.jetbrains.kotlin:kotlin-stdlib
Configuration on demand is an incubating feature.
Calculating task graph as no cached configuration is available for tasks: :di-with-koin:reason --id org.jetbrains.kotlin:kotlin-stdlib
Type-safe project accessors is an incubating feature.

> Configure project :di-with-koin
w: ATTENTION: 'kotlin.experimental.tryK2' is an experimental option enabled in the project for trying out the new Kotlin K2 compiler only.
Please refrain from using it in production code and provide feedback to the Kotlin team for any issues encountered via https://kotl.in/issue

> Task :di-with-koin:compileIntegrationTestKotlin
w: Language version 2.0 is experimental, there are no backwards compatibility guarantees for new language and library features

> Task :di-with-koin:compileTestKotlin
w: Language version 2.0 is experimental, there are no backwards compatibility guarantees for new language and library features

> Task :di-with-koin:reason

----------------------------------------
You asked about the dependency 'org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21'.
There is no advice regarding this dependency.
----------------------------------------

Shortest path from :di-with-koin to org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21 for integrationTestCompileClasspath:
:di-with-koin
\--- io.insert-koin:koin-core-jvm:3.5.0
      \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21

Shortest path from :di-with-koin to org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21 for integrationTestRuntimeClasspath:
:di-with-koin
\--- io.insert-koin:koin-core-jvm:3.5.0
      \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21

Shortest path from :di-with-koin to org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21 for compileClasspath:
:di-with-koin
\--- io.insert-koin:koin-core-jvm:3.5.0
      \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21

Shortest path from :di-with-koin to org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21 for runtimeClasspath:
:di-with-koin
\--- io.insert-koin:koin-core-jvm:3.5.0
      \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21

Shortest path from :di-with-koin to org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21 for testCompileClasspath:
:di-with-koin
\--- io.insert-koin:koin-core-jvm:3.5.0
      \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21

Shortest path from :di-with-koin to org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21 for testRuntimeClasspath:
:di-with-koin
\--- io.insert-koin:koin-core-jvm:3.5.0
      \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.21

Source: main
------------
(no usages)

Source: test
------------
(no usages)

Source: integrationTest
-----------------------
(no usages)

##### 'kotlin.experimental.tryK2' results #####
:di-with-koin:compileIntegrationTestKotlin: 2.0 language version
:di-with-koin:compileTestKotlin: 2.0 language version
##### 100% (2/2) tasks have been compiled with Kotlin 2.0 #####

BUILD SUCCESSFUL

Describe the bug

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':buildHealth'.
> Advice for :di-with-koin
  Existing dependencies which should be modified to be as indicated:
    integrationTestImplementation("org.jetbrains.kotlin:kotlin-stdlib:1.9.21") (was integrationTestApi)

To Reproduce Steps to reproduce the behavior:

  1. gradlew buildHealth on https://github.com/sdkotlin/sd-kotlin-talks/tree/a7571fc464831fc23124e52cd8ce0fc445339326.

Expected behavior No advice for org.jetbrains.kotlin:kotlin-stdlib

Additional context

ianbrandt commented 4 months ago

Just an update, I retested with 1.30.0 and KGP 1.9.22, and the issue still presents.

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':buildHealth'.
> Advice for :di-with-koin
  Existing dependencies which should be modified to be as indicated:
    integrationTestImplementation("org.jetbrains.kotlin:kotlin-stdlib:1.9.22") (was integrationTestApi)

Given this and prior issues, I'm stuck on 1.22.0 pending a fix here or upstream (KT-63285).

ianbrandt commented 3 months ago

Some additional findings and developments...

One thing I forgot to mention before is that staying on 1.22.0 has kept us stuck on Kotlin 1.9.0, as we'd seem to hit an incarnation of #884 if we tried to upgrade Kotlin to any newer version. This is with Gradle 8.5, DAGP 1.22.0, and Kotlin 1.9.23:

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':mymodule:mysubmodule:synthesizeDependenciesTest'.
> A failure occurred while executing com.autonomousapps.tasks.SynthesizeDependenciesTask$SynthesizeDependenciesWorkAction
   > C:\Dev\Repos\myproject\mymodule\mysubmodule\build\reports\dependency-analysis\test\dependencies\() -> kotlin.Any?.json (The filename, directory name, or volume label syntax is incorrect)

Now, after some build changes to associate my test suite and test fixtures compilations (so Kotlin internal elements in the test fixtures can be accessed from the various test suites in the same Gradle module), I'm suddenly hitting something similar with version 1.22.0, even with Kotlin 1.9.0 (still Gradle 8.5):

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':myothermodule:synthesizeDependenciesTest'.
> A failure occurred while executing com.autonomousapps.tasks.SynthesizeDependenciesTask$SynthesizeDependenciesWorkAction
   > C:\Dev\Repos\myproject\myothermodule\build\reports\dependency-analysis\test\dependencies\() -> java.io.File?.json (The filename, directory name, or volume label syntax is incorrect)

So it looks like I can't stay on version 1.22.0 after all.

At this point it seems like my best remaining option is to upgrade to 1.30.0, which allows me to upgrade Kotlin, but then switch from onIncorrectConfiguration { severity("fail") } to onIncorrectConfiguration { severity("warn") }.

This is a shame, as we'll surely not stay on top of incorrect configurations as well if we're not failing our CI build on them, particularly with them obscured by what is currently 180 testSuiteApi-related warnings.

It looks like KT-63285 keeps getting bumped to later versions—first 2.0.0-Beta3 → 2.0.0-Beta4, then 2.0.0-Beta4 → 2.0.0-Beta5, now 2.0.0-Beta5 → 2.0.202.0.0-Beta5 → 2.0.202.0.0-Beta5 → 2.0.20—so I'm not sure when we'll be able to count on that for a solution.

ianbrandt commented 3 months ago

To see if it would be possible to keep onIncorrectConfiguration { severity("fail") } set for my build by ignoring test analysis, I tried setting systemProp.dependency.analysis.test.analysis=false in my 'gradle.properties' per the FAQ. I'm still getting used transitive dependencies and incorrect configuration advice for all my custom test suites. Version 1.30.0, Gradle 8.5.

autonomousapps commented 2 months ago

I think you should use the latest version of DAGP, along with one of two options:

  1. Manually exclude the stdlib using exclude(/* coordinates */) (there's no need to downgrade from fail -> warn)
  2. Tell KGP to stop adding the stdlib by default. (code snippet below)
# gradle.properties
kotlin.stdlib.default.dependency=false

In my opinion, DAGP is providing correct advice. The problem is that the advice often conflicts with the default behavior of KGP. I'm not inclined to add special handling for KGP / the Kotlin stdlib.

I'm going to close this issue.