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

Issue:
"Impacted"/downstream module wrongly receives *ForUpstreamModules properties in certain cases
Given `buildUpstream=true|always|derived` (and `mvn -am` in  case of `derived`) and one of the following two module setups:
- `A <- B* <- C <- D*` with `buildUpstreamMode=changed`
- `A <- B* <- C <- D` with `buildUpstreamMode=impacted`

(`*` marking a changed module)

results in `C` being subject to `skipTestsForUpstreamModules` and `argsForUpstreamModules` (if set), which is clearly wrong since `C` is "impacted" by the change to `B` and must therefore run all tests etc.

Pull request:
Issue #147: fix *ForUpstream wrongly applied to downstream
Fixes #147 

Patch:
diff --git a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
index a01a3af..c23123a 100644
--- a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
+++ b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
@@ -160,7 +160,7 @@ class UnchangedProjectsRemover {
             }
             upstreamProjects = upstreamRequiringProjects.stream()
                     .flatMap(this::streamUpstreamProjects)
-                    .filter(proj -> ! changed.contains(proj))
+                    .filter(proj -> !impacted.contains(proj))
                     .collect(Collectors.toCollection(LinkedHashSet::new));
         }
 


Test patch:
diff --git a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java
index b3910d8..a71f7fd 100644
--- a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java
+++ b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java
@@ -178,6 +178,33 @@ public class UnchangedProjectsRemoverTest extends BaseUnchangedProjectsRemoverTe
         assertProjectPropertiesEqual(dependsOnBothModuleMock, Collections.emptyMap());
     }
 
+    // linear: A <- B <- C <- D
+    @Test
+    public void singleChanged_buildUpstream_modeChanged_argsForUpstreamModules_linear() throws GitAPIException, IOException {
+        MavenProject changedModuleMock = addModuleMock(AID_MODULE_B, true);
+        MavenProject unchangedIntermediateModuleMock = addModuleMock("module-C", false);
+        MavenProject dependsOnIntermediateModuleMock = addModuleMock("module-D", true);
+
+        when(mavenExecutionRequestMock.getMakeBehavior()).thenReturn(MavenExecutionRequest.REACTOR_MAKE_UPSTREAM);
+
+        setUpstreamProjects(dependsOnIntermediateModuleMock, unchangedIntermediateModuleMock, changedModuleMock, moduleA);
+        setDownstreamProjects(changedModuleMock, unchangedIntermediateModuleMock, dependsOnIntermediateModuleMock);
+        setDownstreamProjects(unchangedIntermediateModuleMock, dependsOnIntermediateModuleMock);
+
+        projectProperties.put(Property.buildUpstreamMode.fullName(), "changed");
+        projectProperties.put(Property.argsForUpstreamModules.fullName(), "foo=bar");
+
+        underTest.act();
+
+        verify(mavenSessionMock).setProjects(
+                Arrays.asList(moduleA, changedModuleMock, unchangedIntermediateModuleMock, dependsOnIntermediateModuleMock));
+
+        assertProjectPropertiesEqual(moduleA, ImmutableMap.of("foo", "bar"));
+        assertProjectPropertiesEqual(changedModuleMock, Collections.emptyMap());
+        assertProjectPropertiesEqual(unchangedIntermediateModuleMock, Collections.emptyMap());
+        assertProjectPropertiesEqual(dependsOnIntermediateModuleMock, Collections.emptyMap());
+    }
+
     @Test
     public void singleChanged_buildUpstream_modeImpacted() throws GitAPIException, IOException {
         MavenProject changedModuleMock = addModuleMock(AID_MODULE_B, true);
@@ -188,7 +215,7 @@ public class UnchangedProjectsRemoverTest extends BaseUnchangedProjectsRemoverTe
 
         setUpAndDownstreamsForBuildUpstreamModeTests(changedModuleMock, unchangedModuleMock, dependsOnBothModuleMock);
 
-        projectProperties.put(Property.buildUpstreamMode.fullName(), "impacted");   // this is also the default value!
+        projectProperties.put(Property.buildUpstreamMode.fullName(), "impacted");
 
         underTest.act();
 
@@ -210,7 +237,7 @@ public class UnchangedProjectsRemoverTest extends BaseUnchangedProjectsRemoverTe
 
         setUpAndDownstreamsForBuildUpstreamModeTests(changedModuleMock, unchangedModuleMock, dependsOnBothModuleMock);
 
-        projectProperties.put(Property.buildUpstreamMode.fullName(), "impacted");   // is also the default value!
+        projectProperties.put(Property.buildUpstreamMode.fullName(), "impacted");
         projectProperties.put(Property.argsForUpstreamModules.fullName(), "foo=bar");
 
         underTest.act();
@@ -223,6 +250,33 @@ public class UnchangedProjectsRemoverTest extends BaseUnchangedProjectsRemoverTe
         assertProjectPropertiesEqual(dependsOnBothModuleMock, Collections.emptyMap());
     }
 
+    // linear: A <- B <- C <- D
+    @Test
+    public void singleChanged_buildUpstream_modeImpacted_argsForUpstreamModules_linear() throws GitAPIException, IOException {
+        MavenProject changedModuleMock = addModuleMock(AID_MODULE_B, true);
+        MavenProject unchangedIntermediateModuleMock = addModuleMock("module-C", false);
+        MavenProject dependsOnIntermediateModuleMock = addModuleMock("module-D", false);
+
+        when(mavenExecutionRequestMock.getMakeBehavior()).thenReturn(MavenExecutionRequest.REACTOR_MAKE_UPSTREAM);
+
+        setUpstreamProjects(dependsOnIntermediateModuleMock, unchangedIntermediateModuleMock, changedModuleMock, moduleA);
+        setDownstreamProjects(changedModuleMock, unchangedIntermediateModuleMock, dependsOnIntermediateModuleMock);
+        setDownstreamProjects(unchangedIntermediateModuleMock, dependsOnIntermediateModuleMock);
+
+        projectProperties.put(Property.buildUpstreamMode.fullName(), "impacted");
+        projectProperties.put(Property.argsForUpstreamModules.fullName(), "foo=bar");
+
+        underTest.act();
+
+        verify(mavenSessionMock).setProjects(
+                Arrays.asList(moduleA, changedModuleMock, unchangedIntermediateModuleMock, dependsOnIntermediateModuleMock));
+
+        assertProjectPropertiesEqual(moduleA, ImmutableMap.of("foo", "bar"));
+        assertProjectPropertiesEqual(changedModuleMock, Collections.emptyMap());
+        assertProjectPropertiesEqual(unchangedIntermediateModuleMock, Collections.emptyMap());
+        assertProjectPropertiesEqual(dependsOnIntermediateModuleMock, Collections.emptyMap());
+    }
+
     @Test
     public void singleChanged_buildAll_argsForUpstreamModules() throws GitAPIException, IOException {
         MavenProject changedModuleMock = addModuleMock(AID_MODULE_B, true);