Copilot commented on code in PR #1337:
URL:
https://github.com/apache/maven-assembly-plugin/pull/1337#discussion_r3695471045
##########
src/main/java/org/apache/maven/plugins/assembly/archive/phase/ModuleSetAssemblyPhase.java:
##########
@@ -121,24 +121,22 @@ public static List<DependencySet> getDependencySets(final
ModuleBinaries binarie
public static Set<MavenProject> getModuleProjects(
final ModuleSet moduleSet, final AssemblerConfigurationSource
configSource, final Logger logger)
throws ArchiveCreationException {
- MavenProject project = configSource.getProject();
- Set<MavenProject> moduleProjects = null;
+ final Set<MavenProject> moduleProjects;
if (moduleSet.isUseAllReactorProjects()) {
- if (!moduleSet.isIncludeSubModules()) {
- moduleProjects = new
LinkedHashSet<>(configSource.getReactorProjects());
- }
-
- project = configSource.getReactorProjects().get(0);
- }
-
- if (moduleProjects == null) {
+ moduleProjects = new
LinkedHashSet<>(configSource.getReactorProjects());
+ } else {
try {
moduleProjects = ProjectUtils.getProjectModules(
- project, configSource.getReactorProjects(),
moduleSet.isIncludeSubModules(), logger);
+ configSource.getProject(),
+ configSource.getReactorProjects(),
+ moduleSet.isIncludeSubModules(),
+ logger);
} catch (final IOException e) {
throw new ArchiveCreationException(
- "Error retrieving module-set for project: " +
project.getId() + ": " + e.getMessage(), e);
+ "Error retrieving module-set for project: "
+ + configSource.getProject().getId() + ": " +
e.getMessage(),
+ e);
}
Review Comment:
`getModuleProjects` calls `configSource.getProject()` and
`configSource.getReactorProjects()` multiple times (including again inside the
exception path). Caching these values locally avoids repeated/mock-sensitive
calls and guarantees the error message refers to the same project instance
passed into `ProjectUtils.getProjectModules()`.
##########
src/test/java/org/apache/maven/plugins/assembly/archive/phase/ModuleSetAssemblyPhaseTest.java:
##########
@@ -616,6 +617,38 @@ void
getModuleProjectsShouldReturnNothingWhenReactorContainsTwoSiblingProjects()
verify(configSource, atLeastOnce()).getProject();
}
+ @Test
+ void
getModuleProjectsShouldUseAllReactorProjectsRegardlessOfReactorOrder() throws
Exception {
+ final MavenProject parent = createProject("group", "parent",
"version", null);
+
+ final Model aggregatorModel = new Model();
+ aggregatorModel.setGroupId("group");
+ aggregatorModel.setArtifactId("aggregator");
+ aggregatorModel.setVersion("version");
+
+ final MavenProject aggregator = new MavenProject(aggregatorModel);
+ aggregator.setFile(new File(temporaryFolder, "aggregator/pom.xml"));
+
+ final MavenProject module = createProject("group", "module",
"version", aggregator);
+ final MavenProject distribution = createProject("group",
"distribution", "version", aggregator);
+
+ final List<MavenProject> projects = Arrays.asList(parent, aggregator,
module, distribution);
+
+ final AssemblerConfigurationSource configSource =
mock(AssemblerConfigurationSource.class);
+ when(configSource.getReactorProjects()).thenReturn(projects);
+ when(configSource.getProject()).thenReturn(distribution);
+
+ final ModuleSet moduleSet = new ModuleSet();
+ moduleSet.setUseAllReactorProjects(true);
+ moduleSet.setIncludeSubModules(true);
+
+ final Set<MavenProject> moduleProjects =
+ ModuleSetAssemblyPhase.getModuleProjects(moduleSet,
configSource, logger);
+
+ assertEquals(new LinkedHashSet<>(projects), moduleProjects);
+ verify(configSource).getReactorProjects();
Review Comment:
This assertion uses `Set#equals`, which does not validate
iteration/insertion order. Since the fix and PR description mention using the
ordered reactor (and the implementation uses `LinkedHashSet`), assert on the
iteration order explicitly so the test will fail if ordering is lost (e.g., by
a future refactor to `HashSet`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
