Skip to content

Commit

Permalink
Add support for providing a raw commit SHA to compare against as a ne…
Browse files Browse the repository at this point in the history
…w comparison strategy (#279)

* Add support for providing the raw String of a commit SHA as a comparison strategy

* Add documentation for new comparison strategy to README

* Fix ‘Not enough information to infer type variable T’

* Add unit tests for the new comparison strategy

* Undo reformatting

* Fix formatting

* Add unit tests for configuration

* Revert "Fix ‘Not enough information to infer type variable T’"

This reverts commit 2586a3d.
  • Loading branch information
ivanalvarado authored Jan 27, 2025
1 parent 78f344e commit b826d47
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 5 deletions.
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,16 @@ affectedModuleDetector {
- `logFilename`: A filename for the output detector to use
- `logFolder`: A folder to output the log file in
- `specifiedBranch`: A branch to specify changes against. Must be used in combination with configuration `compareFrom = "SpecifiedBranchCommit"`
- `specifiedRawCommitSha`: A raw commit SHA to specify changes against. Must be used in combination with configuration `compareFrom = "SpecifiedRawCommitSha"`
- `ignoredFiles`: A set of files that will be filtered out of the list of changed files retrieved by git.
- `buildAllWhenNoProjectsChanged`: If true, the plugin will build all projects when no projects are considered affected.
- `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: 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.

- SpecifiedRawCommitSha: compare against the provided raw commit SHA.

**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, can be the name of a module, or a regex against the module gradle path
- `includeUncommitted`: If uncommitted files should be considered affected
Expand Down Expand Up @@ -162,6 +164,12 @@ Suppose we have changed 6 files in our "feature" branch.

Hence, depending on your CI settings you have to configure AMD appropriately.

## SpecifiedRawCommitSha

If you want AMD plugin to skip performing the git operations under-the-hood (like `git rev-parse` and `git merge base`), you can provide the raw commit SHA you wish to compare against.
One of the main reasons you might want to follow this approach is, maybe your environment leverages [Git mirroring](https://docs.github.com/en/repositories/creating-and-managing-repositories/duplicating-a-repository) for speed optimizations, for example CI/CD environments.
Mirroring _can_ lead to inaccurate common ancestor commits as the duplicated repository _may be_ out of sync with the true remote repository.

## Sample Usage

Running the plugin generated tasks is quite simple. By default, if `affected_module_detector.enable` is not set,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,16 @@ class AffectedModuleConfiguration {

var specifiedBranch: String? = null

var specifiedRawCommitSha: String? = null

var compareFrom: String = "PreviousCommit"
set(value) {
val commitShaProviders = listOf(
"PreviousCommit",
"ForkCommit",
"SpecifiedBranchCommit",
"SpecifiedBranchCommitMergeBase"
"SpecifiedBranchCommitMergeBase",
"SpecifiedRawCommitSha"
)
require(commitShaProviders.contains(value)) {
"The property configuration compareFrom must be one of the following: ${commitShaProviders.joinToString(", ")}"
Expand All @@ -97,6 +100,11 @@ class AffectedModuleConfiguration {
"Specify a branch using the configuration specifiedBranch"
}
}
if (value == "SpecifiedRawCommitSha") {
requireNotNull(specifiedRawCommitSha) {
"Provide a Commit SHA for the specifiedRawCommitSha property when using SpecifiedRawCommitSha comparison strategy."
}
}
field = value
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ class AffectedModuleDetectorImpl constructor(
injectedGitClient ?: GitClientImpl(
rootProject.projectDir,
logger,
commitShaProvider = CommitShaProvider.fromString(config.compareFrom, config.specifiedBranch),
commitShaProvider = CommitShaProvider.fromString(config.compareFrom, config.specifiedBranch, config.specifiedRawCommitSha),
ignoredFiles = config.ignoredFiles
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ interface CommitShaProvider {
fun get(commandRunner: GitClient.CommandRunner): Sha

companion object {
fun fromString(string: String, specifiedBranch: String? = null): CommitShaProvider {
fun fromString(
string: String,
specifiedBranch: String? = null,
specifiedRawCommitSha: String? = null
): CommitShaProvider {
return when (string) {
"PreviousCommit" -> PreviousCommit()
"ForkCommit" -> ForkCommit()
Expand All @@ -24,6 +28,12 @@ interface CommitShaProvider {
}
SpecifiedBranchCommitMergeBase(specifiedBranch)
}
"SpecifiedRawCommitSha" -> {
requireNotNull(specifiedRawCommitSha) {
"Specified raw commit sha must be defined"
}
SpecifiedRawCommitSha(specifiedRawCommitSha)
}
else -> throw IllegalArgumentException("Unsupported compareFrom type")
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.dropbox.affectedmoduledetector.commitshaproviders

import com.dropbox.affectedmoduledetector.GitClient
import com.dropbox.affectedmoduledetector.Sha

class SpecifiedRawCommitSha(private val commitSha: String) : CommitShaProvider {
override fun get(commandRunner: GitClient.CommandRunner): Sha {
return commitSha
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,39 @@ class AffectedModuleConfigurationTest {
fail("Expected to catch an exception")
}

@Test
fun `GIVEN AffectedModuleConfiguration WHEN compareFrom is set to SpecifiedRawCommitSha THEN is SpecifiedRawCommitSha`() {
val specifiedRawCommitSha = "SpecifiedRawCommitSha"
val commitSha = "12345"

config.specifiedRawCommitSha = commitSha
config.compareFrom = specifiedRawCommitSha

val actual = config.compareFrom
assertThat(actual).isEqualTo(specifiedRawCommitSha)
}

@Test
fun `GIVEN AffectedModuleConfiguration WHEN compareFrom is set to SpecifiedRawCommitSha AND specifiedRawCommitSha not defined THEN error thrown`() {
val specifiedRawCommitSha = "SpecifiedRawCommitSha"

try {
config.compareFrom = specifiedRawCommitSha
} catch (e: IllegalArgumentException) {
// THEN
assertThat(e.message).isEqualTo("Provide a Commit SHA for the specifiedRawCommitSha property when using SpecifiedRawCommitSha comparison strategy.")
return
}
}

@Test
fun `GIVEN AffectedModuleConfiguration WHEN compareFrom is set to invalid sha provider THEN exception thrown and value not set`() {
try {
config.compareFrom = "InvalidInput"
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, SpecifiedBranchCommitMergeBase")
assertThat(e.message).isEqualTo("The property configuration compareFrom must be one of the following: PreviousCommit, ForkCommit, SpecifiedBranchCommit, SpecifiedBranchCommitMergeBase, SpecifiedRawCommitSha")
assertThat(config.compareFrom).isEqualTo("PreviousCommit")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ class CommitShaProviderTest {
assertThat(actual::class).isEqualTo(SpecifiedBranchCommitMergeBase::class)
}

@Test
fun givenSpecifiedRawCommitSha_whenFromString_thenReturnSpecifiedRawCommitSha() {
val actual = CommitShaProvider.fromString("SpecifiedRawCommitSha", specifiedRawCommitSha = "sha")

assertThat(actual::class).isEqualTo(SpecifiedRawCommitSha::class)
}

@Test
fun givenSpecifiedRawCommitShaAndSpecifiedRawCommitShaNull_whenFromString_thenThrowException() {
try {
CommitShaProvider.fromString("SpecifiedRawCommitSha")
} catch (e: Exception) {
assertThat(e::class).isEqualTo(IllegalArgumentException::class)
assertThat(e.message).isEqualTo("Specified raw commit sha must be defined")
}
}

@Test
fun givenInvalidCommitString_whenFromString_thenExceptionThrown() {
try {
Expand Down

0 comments on commit b826d47

Please sign in to comment.