Merge pull request #164 from Evleaps/feature/new-specified-branch-commit

Feature/new specified branch commit

Author: joshafeinberg

README.md

@@ -112,7 +112,10 @@ affectedModuleDetector {
  - `compareFrom`: A commit to compare the branch changes against. Can be either:
     - PreviousCommit: compare against the previous commit
     - ForkCommit: compare against the commit the branch was forked from
-    - SpecifiedBranchCommit: specify the branch to compare changes against using the `specifiedBranch` configuration before the `compareFrom` configuration
+    - SpecifiedBranchCommit: compare against the last commit of `$specifiedBranch` using `git rev-parse` approach. 
+    - SpecifiedBranchCommitMergeBase: compare against the nearest ancestors with `$specifiedBranch` using `git merge base` approach.
+    
+ **Note:** specify the branch to compare changes against using the `specifiedBranch` configuration before the `compareFrom` configuration
  - `excludedModules`: A list of modules that will be excluded from the build process
  - `includeUncommitted`: If uncommitted files should be considered affected
  - `top`: The top of the git log to use. Must be used in combination with configuration `includeUncommitted = false`
@@ -132,6 +135,26 @@ affectedModuleDetector {
  * `./gradlew runAffectedAndroidTests` - runs connected tests
  * `./gradlew assembleAffectedAndroidTests` - assembles but does not run on device tests, useful when working with device labs
 
+## SpecifiedBranchCommit vs SpecifiedBranchCommitMergeBase
+
+- SpecifiedBranchCommit using `git rev-parse` command for getting sha. 
+- SpecifiedBranchCommitMergeBase using `git merge base` command for getting sha. 
+
+What does it mean?
+When we run any AMD command we compare the current branch with the specified parent branch. Consider an example when, during the development of our feature,
+another developer merged his changes (9 files) into our common remote parent branch - "origin/dev". 
+
+Please, look at picture:
+![specified_branch_difference.png](specified_branch_difference.png)
+
+Suppose we have changed 6 files in our "feature" branch.
+
+1. Behaviour of SpecifiedBranchCommit:
+   AMD will show the result that 15 files were affected. Because our branch is not updated (pull) and AMD will see our 6 files and 9 files that were merged by another developer.
+2. Behaviour of SpecifiedBranchCommitMergeBase:
+   AMD will show the result that 6 files were affected. And this is the correct behavior.
+
+Hence, depends on your CI settings you have to configure AMD right. 
 
 ## Sample Usage
 

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

@@ -81,11 +81,16 @@ class AffectedModuleConfiguration {
 
     var compareFrom: String = "PreviousCommit"
     set(value) {
-        val commitShaProviders = listOf("PreviousCommit", "ForkCommit", "SpecifiedBranchCommit")
+        val commitShaProviders = listOf(
+            "PreviousCommit",
+            "ForkCommit",
+            "SpecifiedBranchCommit",
+            "SpecifiedBranchCommitMergeBase"
+        )
         require(commitShaProviders.contains(value)) {
             "The property configuration compareFrom must be one of the following: ${commitShaProviders.joinToString(", ")}"
         }
-        if (value == "SpecifiedBranchCommit") {
+        if (value == "SpecifiedBranchCommit" || value == "SpecifiedBranchCommitMergeBase") {
             requireNotNull(specifiedBranch) {
                 "Specify a branch using the configuration specifiedBranch"
             }

affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/commitshaproviders/CommitShaProvider.kt

@@ -4,6 +4,7 @@ import com.dropbox.affectedmoduledetector.GitClient
 import com.dropbox.affectedmoduledetector.Sha
 
 interface CommitShaProvider {
+
     fun get(commandRunner: GitClient.CommandRunner): Sha
 
     companion object {
@@ -17,9 +18,14 @@ interface CommitShaProvider {
                     }
                     SpecifiedBranchCommit(specifiedBranch)
                 }
+                "SpecifiedBranchCommitMergeBase" -> {
+                    requireNotNull(specifiedBranch) {
+                        "Specified branch must be defined"
+                    }
+                    SpecifiedBranchCommitMergeBase(specifiedBranch)
+                }
                 else -> throw IllegalArgumentException("Unsupported compareFrom type")
             }
         }
     }
 }
-

affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/commitshaproviders/SpecifiedBranchCommitMergeBase.kt

@@ -0,0 +1,17 @@
+package com.dropbox.affectedmoduledetector.commitshaproviders
+
+import com.dropbox.affectedmoduledetector.GitClient
+import com.dropbox.affectedmoduledetector.Sha
+
+class SpecifiedBranchCommitMergeBase(private val specifiedBranch: String) : CommitShaProvider {
+
+    override fun get(commandRunner: GitClient.CommandRunner): Sha {
+        val currentBranch = commandRunner.executeAndParseFirst(CURRENT_BRANCH_CMD)
+        return commandRunner.executeAndParseFirst("git merge-base $currentBranch $specifiedBranch")
+    }
+
+    companion object {
+
+        const val CURRENT_BRANCH_CMD = "git rev-parse --abbrev-ref HEAD"
+    }
+}

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

@@ -9,7 +9,6 @@ import org.junit.rules.TemporaryFolder
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 import java.io.File
-import java.lang.Exception
 
 @RunWith(JUnit4::class)
 class AffectedModuleConfigurationTest {
@@ -173,6 +172,30 @@ class AffectedModuleConfigurationTest {
         assertThat(actual).isEqualTo("PreviousCommit")
     }
 
+    @Test
+    fun `WHEN compareFrom is set to SpecifiedBranchCommitMergeBase AND specifiedBranch is set THEN return SpecifiedBranchCommitMergeBase`() {
+        val specifiedBranchCommitMergeBase = "SpecifiedBranchCommitMergeBase"
+        val specifiedBranch = "origin/dev"
+
+        config.specifiedBranch = specifiedBranch
+        config.compareFrom = specifiedBranchCommitMergeBase
+
+        val actual = config.compareFrom
+
+        assertThat(actual).isEqualTo(specifiedBranchCommitMergeBase)
+    }
+
+    @Test
+    fun `WHEN compareFrom is set to SpecifiedBranchCommitMergeBase AND specifiedBranch isn't set THEN throw exception`() {
+        val specifiedBranchCommitMergeBase = "SpecifiedBranchCommitMergeBase"
+
+        try {
+            config.compareFrom = specifiedBranchCommitMergeBase
+        } catch (e: IllegalArgumentException) {
+            assertThat(e.message).isEqualTo("Specify a branch using the configuration specifiedBranch")
+        }
+    }
+
     @Test
     fun `GIVEN AffectedModuleConfiguration WHEN compareFrom is set to ForkCommit THEN is ForkCommit`() {
         val forkCommit = "ForkCommit"
@@ -218,7 +241,7 @@ class AffectedModuleConfigurationTest {
             fail()
         } catch (e: Exception) {
             assertThat(e::class).isEqualTo(IllegalArgumentException::class)
-            assertThat(e.message).isEqualTo("The property configuration compareFrom must be one of the following: PreviousCommit, ForkCommit, SpecifiedBranchCommit")
+            assertThat(e.message).isEqualTo("The property configuration compareFrom must be one of the following: PreviousCommit, ForkCommit, SpecifiedBranchCommit, SpecifiedBranchCommitMergeBase")
             assertThat(config.compareFrom).isEqualTo("PreviousCommit")
         }
     }

affectedmoduledetector/src/test/kotlin/com/dropbox/affectedmoduledetector/commitshaproviders/CommitShaProviderTest.kt

@@ -41,6 +41,23 @@ class CommitShaProviderTest {
         }
     }
 
+    @Test
+    fun givenSpecifiedBranchCommitMergeBaseAndSpecifiedBranchNull_whenFromString_thenThrowException() {
+        try {
+            CommitShaProvider.fromString("SpecifiedBranchCommitMergeBase")
+        } catch (e: Exception) {
+            assertThat(e::class).isEqualTo(IllegalArgumentException::class)
+            assertThat(e.message).isEqualTo("Specified branch must be defined")
+        }
+    }
+
+    @Test
+    fun givenSpecifiedBranchCommitMergeBase_whenFromString_thenReturnSpecifiedBranchCommitMergeBase() {
+        val actual = CommitShaProvider.fromString("SpecifiedBranchCommitMergeBase", "branch")
+
+        assertThat(actual::class).isEqualTo(SpecifiedBranchCommitMergeBase::class)
+    }
+
     @Test
     fun givenInvalidCommitString_whenFromString_thenExceptionThrown() {
         try {

affectedmoduledetector/src/test/kotlin/com/dropbox/affectedmoduledetector/commitshaproviders/SpecifiedBranchCommitMergeBaseTest.kt

@@ -0,0 +1,42 @@
+package com.dropbox.affectedmoduledetector.commitshaproviders
+
+import com.dropbox.affectedmoduledetector.AttachLogsTestRule
+import com.dropbox.affectedmoduledetector.mocks.MockCommandRunner
+import com.google.common.truth.Truth
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(JUnit4::class)
+class SpecifiedBranchCommitMergeBaseTest {
+
+    @Rule
+    @JvmField
+    val attachLogsRule = AttachLogsTestRule()
+    private val logger = attachLogsRule.logger
+    private val commandRunner = MockCommandRunner(logger)
+    private val previousCommit = SpecifiedBranchCommitMergeBase(SPECIFIED_BRANCH)
+
+    @Test
+    fun `WHEN CURRENT_BRANCH_CMD THEN command returned`() {
+        Truth.assertThat(SpecifiedBranchCommitMergeBase.CURRENT_BRANCH_CMD).isEqualTo("git rev-parse --abbrev-ref HEAD")
+    }
+
+    @Test
+    fun `WHEN get commit sha THEN return sha`() {
+
+        commandRunner.addReply(SpecifiedBranchCommitMergeBase.CURRENT_BRANCH_CMD, NEW_FEATURE_BRANCH)
+        commandRunner.addReply("git merge-base $NEW_FEATURE_BRANCH $SPECIFIED_BRANCH", "commit-sha")
+
+        val actual = previousCommit.get(commandRunner)
+
+        Truth.assertThat(actual).isEqualTo("commit-sha")
+    }
+
+    private companion object {
+
+        const val SPECIFIED_BRANCH = "origin/dev"
+        const val NEW_FEATURE_BRANCH = "newFeatureBranch"
+    }
+}

sample/build.gradle

@@ -31,8 +31,8 @@ affectedModuleDetector {
     pathsAffectingAllModules = [
             "buildSrc/"
     ]
-    specifiedBranch = "main"
-    compareFrom = "SpecifiedBranchCommit"
+    specifiedBranch = "origin/main"
+    compareFrom = "SpecifiedBranchCommitMergeBase"
     customTasks = [
             new AffectedModuleConfiguration.CustomTask(
                     "runDetektByImpact",