Repository: vackosar/gitflow-incremental-builder (java)

Issue:
Changed non-reactor module is wrongly mapped to the closest reactor module
Given the following project layout:
- `root`
  - `A`
  - `B`

and given that `B` is changed, but is _not_ part of the reactor (e.g. profile activated `modules` in `root`), then `root` is considered changed, which is clearly wrong.

Pull request:
Issue #157: ignore changed non-reactor modules
Fixes #157 

Patch:
diff --git a/src/main/java/com/vackosar/gitflowincrementalbuild/control/ChangedProjects.java b/src/main/java/com/vackosar/gitflowincrementalbuild/control/ChangedProjects.java
index 29f2ec2..92d95ae 100644
--- a/src/main/java/com/vackosar/gitflowincrementalbuild/control/ChangedProjects.java
+++ b/src/main/java/com/vackosar/gitflowincrementalbuild/control/ChangedProjects.java
@@ -10,6 +10,7 @@ import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Map;
 import java.util.Objects;
@@ -27,24 +28,30 @@ public class ChangedProjects {
     @Inject private Modules modules;
 
     public Set<MavenProject> get() throws GitAPIException, IOException {
+        Map<Path, MavenProject> modulesPathMap = modules.createPathMap(mavenSession);
         return differentFiles.get().stream()
-                .map(path -> findProject(path))
+                .map(path -> findProject(path, modulesPathMap))
                 .filter(Objects::nonNull)
                 .collect(Collectors.toSet());
     }
 
-    private MavenProject findProject(Path diffPath) {
-        Map<Path, MavenProject> map = modules.createPathMap(mavenSession);
+    private MavenProject findProject(Path diffPath, Map<Path, MavenProject> modulesPathMap) {
         Path path = diffPath;
-        while (path != null && ! map.containsKey(path)) {
+        // Files.exist() to spot changes in non-reactor module (path will then yield a null changedReactorProject).
+        // Without this check, the changed would be wrongly mapped to the "closest" reactor module (which hasn't changed at all!).
+        while (path != null && !modulesPathMap.containsKey(path) && !Files.exists(path.resolve("pom.xml"))) {
             path = path.getParent();
         }
-        if (path != null) {
-            logger.debug("Changed file: {}", diffPath);
-            return map.get(path);
-        } else {
-            logger.warn("Changed file outside build project: {}", diffPath);
+        if (path == null) {
+            logger.warn("Ignoring changed file outside build project: {}", diffPath);
             return null;
         }
+        MavenProject changedReactorProject = modulesPathMap.get(path);
+        if (changedReactorProject == null) {
+            logger.warn("Ignoring changed file in non-reactor module: {}", diffPath);
+            return null;
+        }
+        logger.debug("Changed file: {}", diffPath);
+        return changedReactorProject;
     }
 }


Test patch:
diff --git a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/MavenIntegrationTest.java b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/MavenIntegrationTest.java
index 31e2285..7703186 100644
--- a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/MavenIntegrationTest.java
+++ b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/MavenIntegrationTest.java
@@ -104,7 +104,7 @@ public class MavenIntegrationTest extends BaseRepoTest {
     @Test
     public void logChanges() throws Exception {
         final String output = executeBuild("-X", "-N");
-        Assert.assertTrue(output.contains("[DEBUG] Changed file: "));
+        Assert.assertTrue(output.contains("[WARNING] Ignoring changed file in non-reactor module: "));
     }
 
     @Test
diff --git a/src/test/java/com/vackosar/gitflowincrementalbuild/control/BaseChangedProjectsTest.java b/src/test/java/com/vackosar/gitflowincrementalbuild/control/BaseChangedProjectsTest.java
index c37a520..7f3c72c 100644
--- a/src/test/java/com/vackosar/gitflowincrementalbuild/control/BaseChangedProjectsTest.java
+++ b/src/test/java/com/vackosar/gitflowincrementalbuild/control/BaseChangedProjectsTest.java
@@ -59,19 +59,39 @@ public abstract class BaseChangedProjectsTest extends BaseRepoTest {
     @Test
     public void list() throws Exception {
         final Set<Path> expected = new HashSet<>(Arrays.asList(
-                Paths.get("child2/subchild2"),
-                Paths.get("child3"),
-                Paths.get("child4"),
-                Paths.get("testJarDependent")
+                Paths.get("parent/child2/subchild2"),
+                Paths.get("parent/child3"),
+                Paths.get("parent/child4"),
+                Paths.get("parent/testJarDependent")
         ));
 
         final Set<Path> actual = underTest.get().stream()
                 .map(MavenProject::getBasedir)
                     .map(File::toPath)
-                    .map(localRepoMock.getBaseCanonicalBaseFolder().toPath().resolve("parent")::relativize)
+                    .map(localRepoMock.getBaseCanonicalBaseFolder().toPath()::relativize)
                 .collect(Collectors.toSet());
 
         Assert.assertEquals(expected, actual);
     }
 
+    @Test
+    public void list_ignoreChangedNonReactorModule() throws Exception {
+        // remove child3 (which contains changes) from the reactor/session
+        mavenSessionMock.getAllProjects().removeIf(proj -> proj.getArtifactId().equals("child3"));
+        mavenSessionMock.getProjects().removeIf(proj -> proj.getArtifactId().equals("child3"));
+
+        final Set<Path> expected = new HashSet<>(Arrays.asList(
+                Paths.get("parent/child2/subchild2"),
+                Paths.get("parent/child4"),
+                Paths.get("parent/testJarDependent")
+        ));
+
+        final Set<Path> actual = underTest.get().stream()
+                .map(MavenProject::getBasedir)
+                    .map(File::toPath)
+                    .map(localRepoMock.getBaseCanonicalBaseFolder().toPath()::relativize)
+                .collect(Collectors.toSet());
+
+        Assert.assertEquals(expected, actual);
+    }
 }