dependency-analysis-gradle-plugin: Don't suggest adding a (test) project dependency on the project itself (since 1.23.1)

Build scan link

https://scans.gradle.com/s/qoumnifidl3h6

Project is open source, you can see the failure on this pr https://github.com/freeletics/khonshu/pull/554

Plugin version

1.23.1

Gradle version

8.3

(Optional) Android Gradle Plugin (AGP) version

8.1.2

reason output for bugs relating to incorrect advice

./gradlew :codegen-compiler:reason --id=:codegen-compiler
----------------------------------------
You asked about the dependency ':codegen-compiler'.
You have been advised to add this dependency to 'testImplementation'.
----------------------------------------

Shortest path from :codegen-compiler to :codegen-compiler for compileClasspath:
:codegen-compiler

Shortest path from :codegen-compiler to :codegen-compiler for runtimeClasspath:
:codegen-compiler

Shortest path from :codegen-compiler to :codegen-compiler for testCompileClasspath:
:codegen-compiler

Shortest path from :codegen-compiler to :codegen-compiler for testRuntimeClasspath:
:codegen-compiler

Shortest path from :codegen-compiler to :codegen-compiler for testFixturesCompileClasspath:
:codegen-compiler

Shortest path from :codegen-compiler to :codegen-compiler for testFixturesRuntimeClasspath:
:codegen-compiler

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

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

Source: testFixtures
--------------------
(no usages)

Describe the bug

Execution failed for task ':buildHealth'.
> Advice for :codegen-compiler
  These transitive dependencies should be declared directly:
    testImplementation(project(":codegen-compiler"))

I believe this happens if the tests of a project are using the testFixtures defined in the same project which then leads the plugin to believe that the shortest path from the test source set to the main source set goes through the test fixture source set even though no dependency is needed.

To Reproduce

Define test fixtures in a project
Use the test fixture in the test source set

Expected behavior

No advice is given

About this issue

Original URL
State: closed
Created 9 months ago
Comments: 15 (10 by maintainers)

Commits related to this issue

Applied workaround for https://github.com/autonomousapps/dependency-analysis-gradle-plugin/issues/972. — committed to sdkotlin/sd-kotlin-talks by ianbrandt 7 months ago

Most upvoted comments

Just FYI, with 1.30.0 I had a couple projects where even if I added the self references for the advised configurations verbatim, the plugin still listed them as undeclared, transitively used dependencies. I had to apply the exclude(project.path) workaround to get past it. I haven’t gotten around to creating a reproducer for the issue, but I just retested with 1.31.0, and I no longer need the exclude. Whatever the cause for that additional issue was, it seems to have been addressed.

ianbrandt on Apr 8, 2024

The special handling of how the dependency scopes of “test” and “main” are glued together has history – orginating in how the scopes compile and test in Maven behave. There are discussions to change this (since quite some time) in Gradle (https://github.com/gradle/gradle/issues/6483) so that the test test suite becomes less special.

But from a user perspective, I agree that the special handling is probably a good idea. Although ideally I would like to see this being configurable (see #900) to satisfy different use cases.

If memory serves, I also don’t believe any advise is given when the “test” suite of one project refers to the “testFixtures” of another.

This should work with the current version (#854).

jjohannes on Dec 7, 2023

It’s still reproducible. You can test by removing this line here https://github.com/freeletics/khonshu/blob/main/codegen-compiler/codegen-compiler.gradle.kts#L36-L37

gabrielittner on Dec 2, 2023

Jendrik, thanks for that detailed explanation. I like to balance strictness with ergonomics, and since Gradle is unlikely to ever require users to add a dependency from the test source to the main source, I think this plugin should respect that as well. It might be enough to add that exclusion globally as you suggested in your comment, or maybe we can do something “deeper”. I’m happy to take this on with the help of the reproducer provided.

autonomousapps on Oct 13, 2023

Technically, this advice is most likely correct. When you apply the test-fixtures plugin, Gradle rewires the setup (I assume AGP does it similarly). You now have something like:

test --depends-on–> test-fixtures --depends-on–> main

If we look at these things as different variants of one project, as soon as you directly refer to classes in “main” from “test” you should declare a dependency. If we strictly follow the rules.

I can however understand that this seems unintuitive. Especially, because there is special handling of “test” already (#900), which is most likely the reason why this advice was not seen before the support for “test-fixtures” was added. So there could be a special handling for this case added to the plugin. (But should there be even more special handlings?)

You can ignore this kind of advices in your project. For example by doing this:

dependencyAnalysis.issues { onAny { exclude(project.path) } }

I wonder if it would be enough to document that.

jjohannes on Oct 10, 2023

It actually still happens with 1.24.0. Renovate bot was weird and only updated the sample app in the project which didn’t have the issue. Here is the pr that updates the main part to 1.24 https://github.com/freeletics/khonshu/pull/560

gabrielittner on Oct 5, 2023