Gradle - Add tests for update in dependencyManagement

[ThomasVitale]

What's changed?

• Introduced tests for demonstrating the issue.

What's your motivation?

• [Gradle] Support updating dependencies in the dependencyManagement block #5300

Anything in particular you'd like reviewers to focus on?

• How to implement the new use case?

Anyone you would like to review specifically?

@timtebeek

Have you considered any alternatives or workarounds?

Yes, workaround considered and explained in #5300, but not feasible in certain scenarios.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[ThomasVitale]

Ignored since this is currently failing:

Recipe was expected to make a change but made no changes.
java.lang.AssertionError: Recipe was expected to make a change but made no changes.
	at org.openrewrite.test.LargeSourceSetCheckingExpectedCycles.afterCycle(LargeSourceSetCheckingExpectedCycles.java:119)
	at org.openrewrite.RecipeScheduler.runRecipeCycles(RecipeScheduler.java:98)
	at org.openrewrite.RecipeScheduler.scheduleRun(RecipeScheduler.java:41)
	at org.openrewrite.Recipe.run(Recipe.java:438)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:377)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:130)
	at org.openrewrite.gradle.UpgradeDependencyVersionTest.makeChangesInDependencyManagement(UpgradeDependencyVersionTest.java:436)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

[shanman190]

We should also probably duplicate this test with a Gradle Kotlin DSL variation as well.

[timtebeek]

Thanks for the start here @ThomasVitale ! I'm going to ask if @shanman190 has some guiding hints here for whether we want to make these changes as part of this recipe, and if that should be in a separate visitor perhaps. He's our friendly community authority on all things Gradle. 🧙🏻‍♂️

[ThomasVitale]

Perfect, thank you so much!

[shanman190]

I think it is probably best as Tim suggested to do this as a separate dedicated visitor given that this section is 1) not type attributed and 2) extremely specific to the Spring Dependency Management plugin.

As you'll see with the other visitors, I would suggest utilizing the JavaIsoVisitor, then also performing similar updates to handle both Gradle Groovy DSL and Gradle Kotlin DSL both.

If you choose to also handle variable updates -- which it seems likely given the test case you've written here -- then you may end up having to collect the parameter names within the scanning phase of the existing JavaIsoVisitor. It's also possible to use a separate visitor for this phase as well though, if you feel that it comes out more cleanly.

Some additional notes:

• mavenBom only supports String notation
  • https://github.com/spring-gradle-plugins/dependency-management-plugin/blob/241762552330f31f7d698d2a711fae748b95ac7c/src/main/java/io/spring/gradle/dependencymanagement/dsl/ImportsHandler.java
• dependency supports String and Map notations
  • https://github.com/spring-gradle-plugins/dependency-management-plugin/blob/241762552330f31f7d698d2a711fae748b95ac7c/src/main/java/io/spring/gradle/dependencymanagement/dsl/DependenciesHandler.java

[shanman190]

This test feels a bit extraneous to me since it's effectively never going to do anything from a recipe standpoint.

[shanman190]

We should also probably duplicate this test with a Gradle Kotlin DSL variation as well.

[ThomasVitale]

@shanman190 thanks so much for the inputs. I'll look a bit deeper into the JavaIsoVisitor and consider support for both the Groovy DSL and Kotlin DSL. I hope I'll get a draft PR ready soon.