diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt index 04a4bec..f60660a 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt @@ -137,8 +137,8 @@ class GenerateHashesCommand : Callable { var ignoredRuleHashingAttributes: Set = emptySet() @CommandLine.Option( - names = ["-d", "--depsFile"], - description = ["Path to the file where dependency edges are written to. If not specified, the dependency edges will not be written to a file."], + names = ["-d", "--depEdgesFile"], + description = ["Path to the file where dependency edges are written to. If not specified, the dependency edges will not be written to a file. Needed for computing build graph distance metrics. See bazel-diff docs for more details about build graph distance metrics."], scope = CommandLine.ScopeType.INHERIT, defaultValue = CommandLine.Parameters.NULL_VALUE ) diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt index 12ddace..5841f0e 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/GetImpactedTargetsCommand.kt @@ -40,7 +40,7 @@ class GetImpactedTargetsCommand : Callable { lateinit var finalHashesJSONPath: File @CommandLine.Option( - names = ["-d", "--depsFile"], + names = ["-d", "--depEdgesFile"], description = ["Path to the file where dependency edges are. If specified, build graph distance metrics will be computed from the given hash data."], scope = CommandLine.ScopeType.INHERIT, defaultValue = CommandLine.Parameters.NULL_VALUE @@ -58,7 +58,7 @@ class GetImpactedTargetsCommand : Callable { @CommandLine.Option( names = ["-o", "--output"], scope = CommandLine.ScopeType.LOCAL, - description = ["Filepath to write the impacted Bazel targets to, newline separated. If not specified, the targets will be written to STDOUT."], + description = ["Filepath to write the impacted Bazel targets to. If using depEdgesFile: formatted in json, otherwise: newline separated. If not specified, the output will be written to STDOUT."], ) var outputPath: File? = null @@ -78,30 +78,17 @@ class GetImpactedTargetsCommand : Callable { val from = deserialiser.executeTargetHash(startingHashesJSONPath) val to = deserialiser.executeTargetHash(finalHashesJSONPath) - val typeFilter = TargetTypeFilter(targetType, to) - - val impactedTargetsStream = if (depsMappingJSONPath != null) { - val depsMapping = deserialiser.deserializeDeps(depsMappingJSONPath!!) - CalculateImpactedTargetsInteractor().executeWithDistances(from, to, depsMapping) - .filterKeys { typeFilter.accepts(it) } - .map { (label, metrics) -> - "${label}~${metrics.targetDistance}~${metrics.packageDistance}" - }.stream() - } else { - CalculateImpactedTargetsInteractor().execute(from, to) - .filter { typeFilter.accepts(it) } - .stream() - } - - return try { - BufferedWriter(when (val path = outputPath) { + val outputWriter = BufferedWriter(when (val path = outputPath) { null -> FileWriter(FileDescriptor.out) else -> FileWriter(path) - }).use { writer -> - impactedTargetsStream.forEach { line -> - writer.write(line) - writer.write("\n") - } + }) + + return try { + if (depsMappingJSONPath != null) { + val depsMapping = deserialiser.deserializeDeps(depsMappingJSONPath!!) + CalculateImpactedTargetsInteractor().executeWithDistances(from, to, depsMapping, outputWriter, targetType) + } else { + CalculateImpactedTargetsInteractor().execute(from, to, outputWriter, targetType) } CommandLine.ExitCode.OK } catch (e: IOException) { @@ -122,5 +109,11 @@ class GetImpactedTargetsCommand : Callable { "Incorrect final hashes: file doesn't exist or can't be read." ) } + if (depsFile != null && !depsFile.canRead()) { + throw CommandLine.ParameterException( + spec.commandLine(), + "Incorrect dep edges file: file doesn't exist or can't be read." + ) + } } } diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/TargetDigest.kt b/cli/src/main/kotlin/com/bazel_diff/hash/TargetDigest.kt index 24c86ee..36eb2d2 100644 --- a/cli/src/main/kotlin/com/bazel_diff/hash/TargetDigest.kt +++ b/cli/src/main/kotlin/com/bazel_diff/hash/TargetDigest.kt @@ -8,11 +8,11 @@ data class TargetDigest( val deps: List? = null, ) { fun clone(newDeps: List? = null): TargetDigest { - var newDeps = newDeps + var toUse = newDeps if (newDeps == null) { - newDeps = deps + toUse = deps } - return TargetDigest(overallDigest.clone(), directDigest.clone(), newDeps) + return TargetDigest(overallDigest.clone(), directDigest.clone(), toUse) } } diff --git a/cli/src/main/kotlin/com/bazel_diff/hash/TargetHash.kt b/cli/src/main/kotlin/com/bazel_diff/hash/TargetHash.kt index 1da4b29..9aa4ce4 100644 --- a/cli/src/main/kotlin/com/bazel_diff/hash/TargetHash.kt +++ b/cli/src/main/kotlin/com/bazel_diff/hash/TargetHash.kt @@ -7,11 +7,11 @@ data class TargetHash( val deps: List? = null ) { val hashWithType by lazy { - "${type}#${hash}#${directHash}" + "${type}#${hash}~${directHash}" } val totalHash by lazy { - "${hash}#${directHash}" + "${hash}~${directHash}" } fun toJson(includeTargetType: Boolean): String { @@ -30,9 +30,15 @@ data class TargetHash( fun fromJson(json: String): TargetHash { val parts = json.split("#") return when (parts.size) { - 2 -> TargetHash("", parts[0], parts[1]) - 3 -> TargetHash(parts[0], parts[1], parts[2]) - else -> throw IllegalArgumentException("Invalid JSON format") + 1 -> Pair("", parts[0]) + 2 -> Pair(parts[0], parts[1]) + else -> throw IllegalArgumentException("Invalid targetHash format: $json") + }.let { (type, hash) -> + val hashes = hash.split("~") + when (hashes.size) { + 2 -> TargetHash(type, hashes[0], hashes[1]) + else -> throw IllegalArgumentException("Invalid targetHash format: $json") + } } } } diff --git a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt index 1206769..0b6d4ae 100644 --- a/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt +++ b/cli/src/main/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractor.kt @@ -2,8 +2,11 @@ package com.bazel_diff.interactor import com.bazel_diff.hash.TargetHash import com.google.common.collect.Maps +import com.google.gson.Gson import org.koin.core.component.KoinComponent +import org.koin.core.component.inject import java.io.File +import java.io.Writer import java.util.concurrent.ConcurrentMap import java.util.concurrent.ConcurrentHashMap import java.util.stream.Stream @@ -16,6 +19,7 @@ data class TargetDistanceMetrics( ) {} class CalculateImpactedTargetsInteractor : KoinComponent { + private val gson: Gson by inject() @VisibleForTesting class InvalidDependencyEdgesException(message: String) : Exception(message) @@ -25,10 +29,24 @@ class CalculateImpactedTargetsInteractor : KoinComponent { INDIRECT } - fun execute(from: Map, to: Map): Set { + fun execute(from: Map, to: Map, outputWriter: Writer, targetTypes: Set?) { /** * This call might be faster if end hashes is a sorted map */ + val typeFilter = TargetTypeFilter(targetTypes, to) + + computeSimpleImpactedTargets(from, to) + .filter { typeFilter.accepts(it) } + .let { impactedTargets -> + outputWriter.use { writer -> + impactedTargets.forEach { + writer.write("$it\n") + } + } + } + } + + fun computeSimpleImpactedTargets(from: Map, to: Map): Set { val difference = Maps.difference(to, from) val onlyInEnd: Set = difference.entriesOnlyOnLeft().keys val changed: Set = difference.entriesDiffering().keys @@ -39,7 +57,27 @@ class CalculateImpactedTargetsInteractor : KoinComponent { return impactedTargets } - fun executeWithDistances(from: Map, to: Map, depEdges: Map>): Map { + fun executeWithDistances(from: Map, to: Map, depEdges: Map>, outputWriter: Writer, targetTypes: Set?) { + val typeFilter = TargetTypeFilter(targetTypes, to) + + computeAllDistances(from, to, depEdges) + .filterKeys { typeFilter.accepts(it) } + .let { impactedTargets -> + outputWriter.use { writer -> + writer.write(gson.toJson( + impactedTargets.map{ + mapOf( + "label" to it.key, + "targetDistance" to it.value.targetDistance, + "packageDistance" to it.value.packageDistance + ) + } + )) + } + } + } + + fun computeAllDistances(from: Map, to: Map, depEdges: Map>): Map { val difference = Maps.difference(to, from) val newLabels = difference.entriesOnlyOnLeft().keys diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index c043c5d..7b9a150 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -3,6 +3,8 @@ package com.bazel_diff.e2e import assertk.assertThat import assertk.assertions.isEqualTo import com.bazel_diff.cli.BazelDiff +import com.google.gson.Gson +import com.google.gson.reflect.TypeToken import org.junit.Ignore import org.junit.Rule import org.junit.Test @@ -46,11 +48,23 @@ class E2ETest { listOf("get-impacted-targets", "-sh", from.absolutePath, "-fh", to.absolutePath, "-o", impactedTargetsOutput.absolutePath) + extraGetImpactedTargetsArgs + if (computeDistances) listOf("-d", depsFile.absolutePath) else emptyList() ) - val actual: Set = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet() - val expected: Set = - javaClass.getResourceAsStream(expectedResultFile).use { it.bufferedReader().readLines().filter { it.isNotBlank() }.toSet() } + if (!computeDistances) { + val actual: Set = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet() + val expected: Set = + javaClass.getResourceAsStream(expectedResultFile).use { it.bufferedReader().readLines().filter { it.isNotBlank() }.toSet() } + + assertThat(actual).isEqualTo(expected) + } else { + // When computing target distances, the output format is json. Read the files and assert the sorted contents. + val gson = Gson() + val shape = object : TypeToken>>() {}.type + val actual = gson.fromJson>>(impactedTargetsOutput.readText(), shape).sortedBy { it["label"] as String } + val expected = javaClass.getResourceAsStream(expectedResultFile).use { + gson.fromJson>>(it.bufferedReader().readText(), shape).sortedBy { it["label"] as String } + } - assertThat(actual).isEqualTo(expected) + assertThat(actual).isEqualTo(expected) + } } @Test diff --git a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt index 2a952fb..105db9f 100644 --- a/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/interactor/CalculateImpactedTargetsInteractorTest.kt @@ -34,7 +34,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { end["3"] = "d" val endHashes = end.mapValues { TargetHash("", it.value, it.value) } - val impacted = interactor.execute(startHashes, endHashes) + val impacted = interactor.computeSimpleImpactedTargets(startHashes, endHashes) assertThat(impacted).containsExactlyInAnyOrder( "1", "3" ) @@ -53,7 +53,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { makeIndirectlyChanged(endHashes, "//:3") val interactor = CalculateImpactedTargetsInteractor() - val impacted = interactor.executeWithDistances(startHashes, endHashes, depEdges) + val impacted = interactor.computeAllDistances(startHashes, endHashes, depEdges) assertThat(impacted).containsOnly( "//:1" to TargetDistanceMetrics(0, 0), @@ -72,7 +72,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { ) val interactor = CalculateImpactedTargetsInteractor() - val impacted = interactor.executeWithDistances(startHashes, endHashes, mapOf()) + val impacted = interactor.computeAllDistances(startHashes, endHashes, mapOf()) assertThat(impacted).containsOnly( "//:1" to TargetDistanceMetrics(0, 0), @@ -91,7 +91,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { makeIndirectlyChanged(endHashes, "//:2", "//:3") val interactor = CalculateImpactedTargetsInteractor() - val impacted = interactor.executeWithDistances(startHashes, endHashes, depEdges) + val impacted = interactor.computeAllDistances(startHashes, endHashes, depEdges) assertThat(impacted).containsOnly( "//:1" to TargetDistanceMetrics(0,0), @@ -111,7 +111,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { makeIndirectlyChanged(endHashes, "//A:2", "//B:3", "//B:4", "//C:5") val interactor = CalculateImpactedTargetsInteractor() - val impacted = interactor.executeWithDistances(startHashes, endHashes, depEdges) + val impacted = interactor.computeAllDistances(startHashes, endHashes, depEdges) assertThat(impacted).containsOnly( "//A:1" to TargetDistanceMetrics(0,0), @@ -139,7 +139,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { makeIndirectlyChanged(endHashes, "//A:target_1", "//A:target_2", "//A:target_3", "//A:target_4", "//B:target", "//C:target", "//D:target", "//final:final") val interactor = CalculateImpactedTargetsInteractor() - val impacted = interactor.executeWithDistances(startHashes, endHashes, depEdges) + val impacted = interactor.computeAllDistances(startHashes, endHashes, depEdges) assertThat(impacted["//final:final"]).isEqualTo(TargetDistanceMetrics(4,2)) } @@ -161,7 +161,7 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { makeIndirectlyChanged(endHashes, "//:2", "//:3") val interactor = CalculateImpactedTargetsInteractor() - val impacted = interactor.executeWithDistances(startHashes, endHashes, depEdges) + val impacted = interactor.computeAllDistances(startHashes, endHashes, depEdges) assertThat(impacted).containsOnly( "//:1" to TargetDistanceMetrics(0,0), @@ -181,12 +181,12 @@ class CalculateImpactedTargetsInteractorTest : KoinTest { val interactor = CalculateImpactedTargetsInteractor() assertThat { - interactor.executeWithDistances(startHashes, endHashes, depEdges) + interactor.computeAllDistances(startHashes, endHashes, depEdges) }.isFailure().message().isEqualTo("//:2 was indirectly impacted, but has no impacted dependencies.") assertThat { // empty dep edges - interactor.executeWithDistances(startHashes, endHashes, mapOf()) + interactor.computeAllDistances(startHashes, endHashes, mapOf()) }.isFailure().message().isEqualTo("//:2 was indirectly impacted, but has no dependencies.") } diff --git a/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt b/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt index 0e3485e..3367c72 100644 --- a/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/interactor/DeserialiseHashesInteractorTest.kt @@ -31,7 +31,7 @@ class DeserialiseHashesInteractorTest : KoinTest { @Test fun testDeserialisation() { val file = temp.newFile().apply { - writeText("""{"target-name":"hash#direct"}""") + writeText("""{"target-name":"hash~direct"}""") } val actual = interactor.executeTargetHash(file) @@ -44,9 +44,9 @@ class DeserialiseHashesInteractorTest : KoinTest { fun testDeserialisationWithType() { val file = temp.newFile().apply { writeText("""{ - | "target-1":"GeneratedFile#hash1#direct1", - | "target-2":"Rule#hash2#direct2", - | "target-3":"SourceFile#hash3#direct3" + | "target-1":"GeneratedFile#hash1~direct1", + | "target-2":"Rule#hash2~direct2", + | "target-3":"SourceFile#hash3~direct3" |}""".trimMargin()) } diff --git a/cli/src/test/resources/fixture/impacted_targets_distances-1-2-rule-sourcefile.txt b/cli/src/test/resources/fixture/impacted_targets_distances-1-2-rule-sourcefile.txt index c8edfba..87b8ff9 100644 --- a/cli/src/test/resources/fixture/impacted_targets_distances-1-2-rule-sourcefile.txt +++ b/cli/src/test/resources/fixture/impacted_targets_distances-1-2-rule-sourcefile.txt @@ -1,3 +1,5 @@ -//test/java/com/integration:bazel-diff-integration-test-lib~0~0 -//test/java/com/integration:bazel-diff-integration-tests~1~0 -//test/java/com/integration:TestStringGenerator.java~0~0 +[ +{"label": "//test/java/com/integration:bazel-diff-integration-test-lib", "targetDistance": 0, "packageDistance": 0}, +{"label": "//test/java/com/integration:bazel-diff-integration-tests", "targetDistance": 1, "packageDistance": 0}, +{"label": "//test/java/com/integration:TestStringGenerator.java", "targetDistance": 0, "packageDistance": 0} +] diff --git a/cli/src/test/resources/fixture/impacted_targets_distances-1-2.txt b/cli/src/test/resources/fixture/impacted_targets_distances-1-2.txt index c3ae3fb..2ea3920 100644 --- a/cli/src/test/resources/fixture/impacted_targets_distances-1-2.txt +++ b/cli/src/test/resources/fixture/impacted_targets_distances-1-2.txt @@ -1,8 +1,10 @@ -//test/java/com/integration:libbazel-diff-integration-test-lib-src.jar~0~0 -//test/java/com/integration:TestStringGenerator.java~0~0 -//test/java/com/integration:bazel-diff-integration-test-lib~0~0 -//test/java/com/integration:bazel-diff-integration-tests-src.jar~2~0 -//test/java/com/integration:libbazel-diff-integration-test-lib.jar~0~0 -//test/java/com/integration:bazel-diff-integration-tests~1~0 -//test/java/com/integration:bazel-diff-integration-tests_deploy-src.jar~2~0 -//test/java/com/integration:bazel-diff-integration-tests.jar~2~0 +[ +{"label": "//test/java/com/integration:libbazel-diff-integration-test-lib-src.jar", "targetDistance": 0, "packageDistance": 0}, +{"label": "//test/java/com/integration:TestStringGenerator.java", "targetDistance": 0, "packageDistance": 0}, +{"label": "//test/java/com/integration:bazel-diff-integration-test-lib", "targetDistance": 0, "packageDistance": 0}, +{"label": "//test/java/com/integration:bazel-diff-integration-tests-src.jar", "targetDistance": 2, "packageDistance": 0}, +{"label": "//test/java/com/integration:libbazel-diff-integration-test-lib.jar", "targetDistance": 0, "packageDistance": 0}, +{"label": "//test/java/com/integration:bazel-diff-integration-tests", "targetDistance": 1, "packageDistance": 0}, +{"label": "//test/java/com/integration:bazel-diff-integration-tests_deploy-src.jar", "targetDistance": 2, "packageDistance": 0}, +{"label": "//test/java/com/integration:bazel-diff-integration-tests.jar", "targetDistance": 2, "packageDistance": 0} +]