gnodet-bot commented on code in PR #13669:
URL: https://github.com/apache/maven/pull/13669#discussion_r4343904003
##########
impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java:
##########
@@ -283,6 +283,65 @@ void testFailedParentStillInitializesChild(@TempDir Path
tempDir) throws Excepti
}
}
+ @Test
+ void testLocalParentResolutionWithValidationLevels(@TempDir Path tempDir)
throws Exception {
+ Path parentPom = tempDir.resolve("pom.xml");
+ Files.writeString(parentPom, """
+ <project>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>parent</artifactId>
+ <version>1</version>
+ <packaging>pom</packaging>
+ </project>
+ """);
+
+ Path childPom =
Files.createDirectory(tempDir.resolve("child")).resolve("pom.xml");
+ Files.writeString(childPom, """
+ <project xmlns="http://maven.apache.org/POM/4.1.0">
+ <modelVersion>4.1.0</modelVersion>
+ <parent>
+ <groupId>test</groupId>
+ <artifactId>parent</artifactId>
+ <version>1</version>
+ <relativePath>../pom.xml</relativePath>
+ </parent>
+ <artifactId>child</artifactId>
+ </project>
+ """);
+
+ MavenSession session = createMavenSession(null);
+ session.getRequest().setRootDirectory(tempDir);
+ org.apache.maven.project.ProjectBuilder builder =
+
getContainer().lookup(org.apache.maven.project.ProjectBuilder.class);
+
+ for (int validationLevel :
+ List.of(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL,
ModelBuildingRequest.VALIDATION_LEVEL_STRICT)) {
+ for (boolean processPlugins : List.of(true)) {
Review Comment:
💡 **Dead loop — consider testing both `true` and `false`**
`List.of(true)` is a single-element list, so this loop executes exactly once
with `processPlugins` always `true`. The loop variable is effectively a
constant, and `&& processPlugins` in the `assertEquals` below is always a no-op.
If the intent was to test both values (to verify behaviour changes depending
on `processPlugins`), use:
```suggestion
for (boolean processPlugins : List.of(true, false)) {
```
If only `processPlugins=true` is meaningful here (because
`setProcessPlugins(false)` skips a different code path entirely), just hoist
the variable and drop the loop:
```java
boolean processPlugins = true;
```
Either way, the current form is misleading.
--
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]