Adding recipe for DoesNotIncludeDependency for Gradle (Maven had one already), to allow for checking whether a dependency is not included.#5725
Merged
Conversation
steve-aom-elliott
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Jul 7, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725
Laurens-W
reviewed
Jul 8, 2025
Laurens-W
reviewed
Jul 8, 2025
26793b8 to
b4fbdb2
Compare
Contributor
Author
|
Still working on the tests to include ones for specific configurations and transitive dependencies, but getting closer. |
b4fbdb2 to
0c0443a
Compare
timtebeek
reviewed
Jul 11, 2025
Member
There was a problem hiding this comment.
Would something like this help readability perhaps? Took me 20 seconds with Claude, so don't go changing things manually:
@CsvSource(
useHeadersInDisplayName = true, textBlock = """
configuration, annotationProcessor, api, implementation, compileOnly, runtimeOnly, testImplementation, testCompileOnly, testRuntimeOnly
annotationProcessor, true, false, false, false, false, false, false, false
compileClasspath, false, true, true, true, false, false, false, false
runtimeClasspath, false, true, true, false, true, false, false, false
testCompileClasspath, false, true, true, false, false, true, true, false
testRuntimeClasspath, false, true, true, false, true, true, false, true
""")
Contributor
Author
There was a problem hiding this comment.
Honestly I've never used the @CsvSource with textBlock before, but yeah, something like that would also work.
timtebeek
reviewed
Jul 11, 2025
Comment on lines
104
to
106
Member
There was a problem hiding this comment.
I was a bit surprised by the repeated annotation here; is there a logic to the grouping that I'm missing? Might a textBlock make sense?
Contributor
Author
There was a problem hiding this comment.
I was grouping them by the existing scope of the dependency per row. It was more for my own legibility than anything else.
timtebeek
reviewed
Jul 11, 2025
timtebeek
reviewed
Jul 11, 2025
Member
|
Mostly looks good already; couple questions & comments, but nothing blocking. Once addressed we can merge. |
…e already), to allow for checking whether a dependency is not included.
…sNotIncludeDependency.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…pendencyInsight` to showcase passing `configuration`. Adding `DoesNotIncludeDependency` tests (still need to add more).
…sts to match naming convention of the Maven version's tests.
… Maven version. I still want to change it a bit so that the test body can be shared for situations that behave the same (like `onlyDirect` has not effect when there is a direct dependency present, so all those tests are the same for whether it's `null`, `true` or `false` for that)
e45bea1 to
a69aeb7
Compare
Contributor
Author
|
I think the only things I didn't end up changing right now were the suggestions for using |
timtebeek
approved these changes
Jul 12, 2025
timtebeek
left a comment
Member
There was a problem hiding this comment.
Approved already. Thanks for the help here!
steve-aom-elliott
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Jul 14, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725
steve-aom-elliott
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Jul 16, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725
steve-aom-elliott
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Jul 24, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725
steve-aom-elliott
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Jul 28, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725
steve-aom-elliott
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Jul 31, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725
timtebeek
added a commit
to openrewrite/rewrite-testing-frameworks
that referenced
this pull request
Aug 5, 2025
…OM or build.gradle have a dependency on `org.testng:testng`. (#760) * Adding dependency guards to the Junit 4 -> 5 migration for when the POM or build.gradle have a dependency on `org.testng:testng`. Relies on openrewrite/rewrite#5725 * Updating `DoesNotIncludeDependency` recipe call to be the shared one in `rewrite-java-dependencies` * Avoid tests passing for the wrong reason (class present) * Only pass testng dependency into the single test that needs it * Match any testng dependency * Adding TestNgGuard scanning recipe. Needs refinement, but this is a first pass * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Making separate `TestNgGuard` tests and cleaning up other code * Cleanup of import Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Made the `TestNgGuard` more flexible for situations where there isn't an enclosing project for a source file, which was a majority of our existing tests. - If it figures out it's a loose file not in a `JavaProject`, it can't ensure project scope anyway, so falls back to solely whether the file itself has a TestNG dependency or uses TestNG types instead for those * Do not use getters internally * Adopt `ModuleHasDependency` and drop odd test: only look at dependencies Assuming classes can not be present without those dependencies * Revert change to `TestNgToAssertJTest` * Drop tests that were showing improbable use of types * Switching to using the new `invertMarking` option on `ModuleHasDependency` for the TestNG guard for JUnit --------- Co-authored-by: Tim te Beek <tim@moderne.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's changed?
What's your motivation?
I have a need for being able to detect that a dependency is not included to decide whether to proceed with additional migrations or not. Maven already had a version of this, but Gradle was missing one, and I want to be thorough in this precondition check.
I have not written tests for this quite yet. All of the tests for the Maven version appear to be commented out for some reason, though I do want to add a couple of examples in tests still.
One thing to point out is that it does not include the same
directOnlyoption as the Maven version, as the way the implementations ofDependencyInsightis processing dependencies between Maven and Gradle is slightly different, and that class does not take the option in the Gradle version.Checklist