Mark all projects changed if file effecting all modules changes (#105)

* Mark all projects changed if file effecting all modules changes

* Rename test

* Remove unneccessary code path

Co-authored-by: Josh Feinberg <joshfeinberg@dropbox.com>

Author: joshafeinberg

affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetector.kt

@@ -402,11 +402,14 @@ class AffectedModuleDetectorImpl constructor(
      * Also populates the unknownFiles var which is used in findAffectedProjects
      */
     private fun findChangedProjects(): Set<Project> {
-        changedFiles.addAll(
-            git.findChangedFiles(
-                includeUncommitted = true
-            )
-        )
+        git.findChangedFiles(
+            includeUncommitted = true
+        ).forEach { fileName ->
+            if (affectsAllModules(fileName)) {
+                return allProjects
+            }
+            changedFiles.add(fileName)
+        }
 
         val changedProjects = mutableSetOf<Project>()
 
@@ -466,11 +469,6 @@ class AffectedModuleDetectorImpl constructor(
         if (changedProjects.isEmpty() && unknownFiles.isEmpty()) {
             buildAll = true
         }
-        changedFiles.forEach {
-            if (affectsAllModules(it)) {
-                buildAll = true
-            }
-        }
         logger?.info(
             "unknownFiles: $unknownFiles, changedProjects: $changedProjects, buildAll: " +
                 "$buildAll"
@@ -479,18 +477,7 @@ class AffectedModuleDetectorImpl constructor(
         // If we're in a buildAll state, we return allProjects unless it's the changed target,
         // Since the changed target runs all tests and we don't want 3+ hour presubmit runs
         if (buildAll) {
-            logger?.info("Building all projects")
-            if (unknownFiles.isEmpty()) {
-                logger?.info("because no changed files were detected")
-            } else {
-                logger?.info("because one of the unknown files affects everything in the build")
-                logger?.info(
-                    """
-                    The modules detected as affected by changed files are
-                    ${changedProjects + dependentProjects}
-                    """.trimIndent()
-                )
-            }
+            logger?.info("Building all projects because no changed files were detected")
             when (projectSubset) {
                 ProjectSubset.DEPENDENT_PROJECTS -> return allProjects
                 ProjectSubset.ALL_AFFECTED_PROJECTS -> return allProjects

affectedmoduledetector/src/test/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleDetectorImplTest.kt

@@ -1252,6 +1252,57 @@ class AffectedModuleDetectorImplTest {
         Truth.assertThat(detector.shouldInclude(p5)).isTrue()
     }
 
+    @Test
+    fun `GIVEN a file that effects all changes has a change WHEN projectSubset is CHANGED_PROJECTS THEN all modules should be in this`() {
+        val changedFile = convertToFilePath("android", "gradle", "test.java")
+
+        affectedModuleConfiguration = affectedModuleConfiguration.also {
+            it.pathsAffectingAllModules.toMutableSet().add(changedFile)
+        }
+        val detector = AffectedModuleDetectorImpl(
+            rootProject = root,
+            logger = logger,
+            ignoreUnknownProjects = false,
+            projectSubset = ProjectSubset.CHANGED_PROJECTS,
+            modules = null,
+            injectedGitClient = MockGitClient(
+                changedFiles = listOf(changedFile),
+                tmpFolder = tmpFolder.root
+            ),
+            config = affectedModuleConfiguration
+        )
+        MatcherAssert.assertThat(
+            detector.affectedProjects,
+            CoreMatchers.`is`(
+                setOf(p1, p2, p3, p4, p5, p6, p7, p8, p9, p10)
+            )
+        )
+    }
+
+    @Test
+    fun `GIVEN a file that does not affect all projects has a change WHEN projectSubset is CHANGED_PROJECTS THEN affected projects is empty`() {
+        val changedFile = convertToFilePath("android", "notgradle", "test.java")
+
+        val detector = AffectedModuleDetectorImpl(
+            rootProject = root,
+            logger = logger,
+            ignoreUnknownProjects = false,
+            projectSubset = ProjectSubset.CHANGED_PROJECTS,
+            modules = null,
+            injectedGitClient = MockGitClient(
+                changedFiles = listOf(changedFile),
+                tmpFolder = tmpFolder.root
+            ),
+            config = affectedModuleConfiguration
+        )
+        MatcherAssert.assertThat(
+            detector.affectedProjects,
+            CoreMatchers.`is`(
+                setOf()
+            )
+        )
+    }
+
     // For both Linux/Windows
     fun convertToFilePath(vararg list: String): String {
         return list.toList().joinToString(File.separator)