Order of parsed Maven modules is sometimes not correct

[fabapp2]

What's changed?

The method MavenMojoProjectParser#collectPoms adds the poms in the wrong order. I think the problem can be fixed by using MavenSession#getProjects() instead.

What's your motivation?

• #600

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

I need some advice on how you think this should be tested.

My preferred approach would have been to test the method collectPoms and verify that the poms added to the paths exactly in the order as returned by MavenSession#getProjects(). I would have liked to pass in a Mock for MavenSession and verify the call. But there are some problems with this approach:

• The method is private, I would have been nasty and used reflection to make it accessible.
• No Mockito is available

And an additional test that verifies the expected behavior of MavenSession#getProjects()- the correct ordering

The next (additional) way to test this I'd see is an integration test for the plugin itself and verifying the parsing events. I did not understand RewriteRunIT and would be interested in your opinion of how to approach this before proceeding.

Anyone you would like to review specifically?

@timtebeek

Have you considered any alternatives or workarounds?

By decoupling the MavenMojoProject parser from File and Maven unittest would become easier.

Any additional context

Checklist

• [ ] I've added unit tests to cover both positive and negative cases
• [ ] I've added the license header to any new files through ./gradlew licenseFormat
• [ ] I've used the IntelliJ auto-formatter on affected files
• [ ] I've updated the documentation (if applicable)

[timtebeek]

I've struggled a bit locally to see if we can test this through the integration test feature, but no luck thus far. My breakpoints aren't triggered, and even log output I add isn't showing; Perhaps you'd have more luck; my branch is here.

I'm exploring a bit the logic in SBM and the modules going into the Maven parser, roughly starting here: https://github.com/spring-projects-experimental/spring-boot-migrator/blob/fcdfe7134e07386f255d288a6fd0dc37e98ad36b/sbm-rewrite-maven-parser/src/main/java/org/springframework/sbm/parsers/RewriteMavenProjectParser.java#L148-L153 There's a TODO comment there regarding the order, but since runPerSubmodule is false I don't think the first mavenProject argument to listSourceFiles is even being used there.

Instead we look to consistently use mavenSession.getProjects() to iterate over the modules in order returned by the session: https://github.com/openrewrite/rewrite-maven-plugin/blob/9ffe2b33e723226c1843dd9df2f44fde2ee24dd6/src/main/java/org/openrewrite/maven/MavenMojoProjectParser.java#L138-L141 Not quite sure how that might end up with a different order as compared to your changes here. 🤔

Will continue later, unless you have any new insights given the above.

[fabapp2]

I understand that's a tricky part. I would think the problem will be solved with 9ffe2b3. If you want we could see if we can use something from here. There we create a Plexus container, call Maven and use a listener to get access to the session which can then be used to feed tests. Not straightforward but it allows testing methods depending on Maven components (that are difficult to initialize).