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]

Reply via email to