From a55f4be84ea5b63edd8b10b36edc51e886d95827 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Wed, 14 Aug 2024 15:36:10 +0530 Subject: [PATCH 01/18] Add support for --base-branch in backward-compatibility-check command Abstract out the common logic in backward-compatibility-check command --- .../kotlin/application/SpecmaticCommand.kt | 4 +- .../backwardCompatibility/BCCheckCommand.kt | 137 ++++++++++++ .../BackwardCompatibilityCheckBaseCommand.kt | 198 ++++++++++++++++++ .../BackwardCompatibilityCheckCommand.kt | 0 .../CompatibilityReport.kt | 16 ++ .../CompatibilityResult.kt | 5 + .../BackwardCompatibilityCheckCommandTest.kt | 3 +- .../application/CompatibleCommandKtTest.kt | 21 +- .../src/test/kotlin/application/FakeGit.kt | 12 ++ .../main/kotlin/io/specmatic/core/Feature.kt | 2 +- .../main/kotlin/io/specmatic/core/IFeature.kt | 3 + .../io/specmatic/core/git/GitCommand.kt | 8 +- .../kotlin/io/specmatic/core/git/SystemGit.kt | 37 +++- 13 files changed, 427 insertions(+), 19 deletions(-) create mode 100644 application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt create mode 100644 application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt rename application/src/main/kotlin/application/{ => backwardCompatibility}/BackwardCompatibilityCheckCommand.kt (100%) create mode 100644 application/src/main/kotlin/application/backwardCompatibility/CompatibilityReport.kt create mode 100644 application/src/main/kotlin/application/backwardCompatibility/CompatibilityResult.kt create mode 100644 core/src/main/kotlin/io/specmatic/core/IFeature.kt diff --git a/application/src/main/kotlin/application/SpecmaticCommand.kt b/application/src/main/kotlin/application/SpecmaticCommand.kt index f8a43a1bb..3f301577b 100644 --- a/application/src/main/kotlin/application/SpecmaticCommand.kt +++ b/application/src/main/kotlin/application/SpecmaticCommand.kt @@ -1,5 +1,7 @@ package application +import application.backwardCompatibility.BCCheckCommand +import application.backwardCompatibility.BackwardCompatibilityCheckCommand import org.springframework.stereotype.Component import picocli.AutoComplete.GenerateCompletion import picocli.CommandLine.Command @@ -10,7 +12,7 @@ import java.util.concurrent.Callable name = "specmatic", mixinStandardHelpOptions = true, versionProvider = VersionProvider::class, - subcommands = [BackwardCompatibilityCheckCommand::class, BundleCommand::class, CompareCommand::class, CompatibleCommand::class, DifferenceCommand::class, GenerateCompletion::class, GraphCommand::class, MergeCommand::class, ToOpenAPICommand::class, ImportCommand::class, InstallCommand::class, ProxyCommand::class, PushCommand::class, ReDeclaredAPICommand::class, ExamplesCommand::class, SamplesCommand::class, StubCommand::class, SubscribeCommand::class, TestCommand::class, ValidateViaLogs::class, CentralContractRepoReportCommand::class] + subcommands = [BCCheckCommand::class, BundleCommand::class, CompareCommand::class, CompatibleCommand::class, DifferenceCommand::class, GenerateCompletion::class, GraphCommand::class, MergeCommand::class, ToOpenAPICommand::class, ImportCommand::class, InstallCommand::class, ProxyCommand::class, PushCommand::class, ReDeclaredAPICommand::class, ExamplesCommand::class, SamplesCommand::class, StubCommand::class, SubscribeCommand::class, TestCommand::class, ValidateViaLogs::class, CentralContractRepoReportCommand::class] ) class SpecmaticCommand : Callable { override fun call(): Int { diff --git a/application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt new file mode 100644 index 000000000..e3747c49e --- /dev/null +++ b/application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt @@ -0,0 +1,137 @@ +package application.backwardCompatibility + +import io.specmatic.conversions.OpenApiSpecification +import io.specmatic.core.CONTRACT_EXTENSION +import io.specmatic.core.CONTRACT_EXTENSIONS +import io.specmatic.core.Feature +import io.specmatic.core.IFeature +import io.specmatic.core.Results +import io.specmatic.core.WSDL +import io.specmatic.core.testBackwardCompatibility +import io.specmatic.stub.isOpenAPI +import org.springframework.stereotype.Component +import picocli.CommandLine.Command +import java.io.File +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import java.util.regex.Pattern +import kotlin.io.path.extension +import kotlin.io.path.pathString + +@Component +@Command( + name = "backwardCompatibilityCheck", + aliases = ["backward-compatibility-check"], + mixinStandardHelpOptions = true, + description = ["Checks backward compatibility of a directory across the current HEAD and the base branch"] +) +class BCCheckCommand: BackwardCompatibilityCheckBaseCommand() { + + override fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results { + return testBackwardCompatibility(oldFeature as Feature, newFeature as Feature) + } + + override fun File.isValidSpec(): Boolean { + if (this.extension !in CONTRACT_EXTENSIONS) return false + return OpenApiSpecification.isParsable(this.path) + } + + override fun getFeatureFromSpecPath(path: String): Feature { + return OpenApiSpecification.fromFile(path).toFeature() + } + + override fun getSpecsReferringTo(schemaFiles: Set): Set { + if (schemaFiles.isEmpty()) return emptySet() + + val inputFileNames = schemaFiles.map { File(it).name } + val result = allOpenApiSpecFiles().filter { + it.readText().trim().let { specContent -> + inputFileNames.any { inputFileName -> + val pattern = Pattern.compile("\\b$inputFileName\\b") + val matcher = pattern.matcher(specContent) + matcher.find() + } + } + }.map { it.path }.toSet() + + return result.flatMap { + getSpecsReferringTo(setOf(it)).ifEmpty { setOf(it) } + }.toSet() + } + + override fun getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch: Set): Set { + data class CollectedFiles( + val specifications: MutableSet = mutableSetOf(), + val examplesMissingSpecifications: MutableList = mutableListOf(), + val ignoredFiles: MutableList = mutableListOf() + ) + + val collectedFiles = filesChangedInCurrentBranch.fold(CollectedFiles()) { acc, filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + + if (examplesDir == null) { + acc.ignoredFiles.add(filePath) + } else { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + + if (specFiles.isNotEmpty()) { + acc.specifications.addAll(specFiles.map { it.toString() }) + } else { + acc.examplesMissingSpecifications.add(filePath) + } + } + acc + } + + val result = collectedFiles.specifications.toMutableSet() + + collectedFiles.examplesMissingSpecifications.forEach { filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + if (examplesDir != null) { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + if (specFiles.isNotEmpty()) { + result.addAll(specFiles.map { it.toString() }) + } else { + result.add("${strippedPath}.yaml") + } + } + } + + return result + } + + override fun areExamplesValid(feature: IFeature, which: String): Boolean { + feature as Feature + return try { + feature.validateExamplesOrException() + true + } catch (t: Throwable) { + println() + false + } + } + + override fun getUnusedExamples(feature: IFeature): Set { + feature as Feature + return feature.loadExternalisedExamplesAndListUnloadableExamples().second + } + + private fun allOpenApiSpecFiles(): List { + return File(".").walk().toList().filterNot { + ".git" in it.path + }.filter { it.isFile && it.isValidSpec() } + } + + private fun findSpecFiles(path: Path): List { + val extensions = CONTRACT_EXTENSIONS + return extensions.map { path.resolveSibling(path.fileName.toString() + it) } + .filter { Files.exists(it) && (isOpenAPI(it.pathString) || it.extension in listOf(WSDL, CONTRACT_EXTENSION)) } + } +} \ No newline at end of file diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt new file mode 100644 index 000000000..44493635e --- /dev/null +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -0,0 +1,198 @@ +package application.backwardCompatibility + +import io.specmatic.core.IFeature +import io.specmatic.core.Results +import io.specmatic.core.git.GitCommand +import io.specmatic.core.git.SystemGit +import io.specmatic.core.log.logger +import io.specmatic.core.utilities.exitWithMessage +import picocli.CommandLine.Option +import java.io.File +import java.util.concurrent.Callable +import kotlin.system.exitProcess + +abstract class BackwardCompatibilityCheckBaseCommand : Callable { + private val gitCommand: GitCommand = SystemGit() + private val newLine = System.lineSeparator() + + @Option(names = ["--base-branch"], description = ["Base branch to compare the changes against"], required = false) + var baseBranch: String? = null + + abstract fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results + abstract fun File.isValidSpec(): Boolean + abstract fun getFeatureFromSpecPath(path: String): IFeature + + abstract fun getSpecsReferringTo(schemaFiles: Set): Set + abstract fun getSpecsOfChangedExternalisedExamples( + filesChangedInCurrentBranch: Set + ): Set + + + open fun areExamplesValid(feature: IFeature, which: String): Boolean = true + open fun getUnusedExamples(feature: IFeature): Set = emptySet() + + override fun call() { + val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch() + val filesReferringToChangedSchemaFiles = getSpecsReferringTo(filesChangedInCurrentBranch) + val specificationsOfChangedExternalisedExamples = + getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch) + + logFilesToBeCheckedForBackwardCompatibility( + filesChangedInCurrentBranch, + filesReferringToChangedSchemaFiles, + specificationsOfChangedExternalisedExamples + ) + + val specificationsToCheck: Set = + filesChangedInCurrentBranch + + filesReferringToChangedSchemaFiles + + specificationsOfChangedExternalisedExamples + + val result = try { + runBackwardCompatibilityCheckFor( + files = specificationsToCheck, + baseBranch = baseBranch() + ) + } catch(e: Throwable) { + logger.newLine() + logger.newLine() + logger.log(e) + exitProcess(1) + } + + println() + println(result.report) + exitProcess(result.exitCode) + } + + private fun getChangedSpecsInCurrentBranch(): Set { + return gitCommand.getFilesChangedInCurrentBranch( + baseBranch() + ).filter { + File(it).exists() && File(it).isValidSpec() + }.toSet().also { + if(it.isEmpty()) exitWithMessage("${newLine}No specs were changed, skipping the check.$newLine") + } + } + + private fun logFilesToBeCheckedForBackwardCompatibility( + changedFiles: Set, + filesReferringToChangedFiles: Set, + specificationsOfChangedExternalisedExamples: Set + ) { + + println("Checking backward compatibility of the following files: $newLine") + println("${ONE_INDENT}Files that have changed:") + changedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + + if(filesReferringToChangedFiles.isNotEmpty()) { + println("${ONE_INDENT}Files referring to the changed files - ") + filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + } + + if(specificationsOfChangedExternalisedExamples.isNotEmpty()) { + println("${ONE_INDENT}Specifications whose externalised examples were changed - ") + filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + } + + println("-".repeat(20)) + println() + } + + private fun runBackwardCompatibilityCheckFor(files: Set, baseBranch: String): CompatibilityReport { + val branchWithChanges = gitCommand.currentBranch() + val treeishWithChanges = if (branchWithChanges == HEAD) gitCommand.detachedHEAD() else branchWithChanges + + try { + val results = files.mapIndexed { index, specFilePath -> + try { + println("${index.inc()}. Running the check for $specFilePath:") + + // newer => the file with changes on the branch + val newer = getFeatureFromSpecPath(specFilePath) + val unusedExamples = getUnusedExamples(newer) + + val olderFile = gitCommand.getFileInTheBaseBranch( + specFilePath, + treeishWithChanges, + baseBranch + ) + if (olderFile == null) { + println("$specFilePath is a new file.$newLine") + return@mapIndexed CompatibilityResult.PASSED + } + + val areLocalChangesStashed = gitCommand.stash() + gitCommand.checkout(baseBranch) + // older => the same file on the default (e.g. main) branch + val older = getFeatureFromSpecPath(olderFile.path) + if (areLocalChangesStashed) gitCommand.stashPop() + + val backwardCompatibilityResult = checkBackwardCompatibility(older, newer) + + if (backwardCompatibilityResult.success()) { + println( + "$newLine The file $specFilePath is backward compatible.$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + var errorsFound = false + + if(!areExamplesValid(newer, "newer")) { + println( + "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + errorsFound = true + } + + if(unusedExamples.isNotEmpty()) { + println( + "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + errorsFound = true + } + + if(errorsFound) CompatibilityResult.FAILED + else CompatibilityResult.PASSED + } else { + println("$newLine ${backwardCompatibilityResult.report().prependIndent( + MARGIN_SPACE + )}") + println( + "$newLine *** The file $specFilePath is NOT backward compatible. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + CompatibilityResult.FAILED + } + } finally { + gitCommand.checkout(treeishWithChanges) + } + } + + return CompatibilityReport(results) + } finally { + gitCommand.checkout(treeishWithChanges) + } + } + + private fun baseBranch() = baseBranch ?: gitCommand.currentRemoteBranch() + + companion object { + private const val HEAD = "HEAD" + private const val MARGIN_SPACE = " " + private const val ONE_INDENT = " " + private const val TWO_INDENTS = "${ONE_INDENT}${ONE_INDENT}" + } +} \ No newline at end of file diff --git a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt similarity index 100% rename from application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt rename to application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt diff --git a/application/src/main/kotlin/application/backwardCompatibility/CompatibilityReport.kt b/application/src/main/kotlin/application/backwardCompatibility/CompatibilityReport.kt new file mode 100644 index 000000000..07a2f4a60 --- /dev/null +++ b/application/src/main/kotlin/application/backwardCompatibility/CompatibilityReport.kt @@ -0,0 +1,16 @@ +package application.backwardCompatibility + +class CompatibilityReport(results: List) { + val report: String + val exitCode: Int + + init { + val failed: Boolean = results.any { it == CompatibilityResult.FAILED } + val failedCount = results.count { it == CompatibilityResult.FAILED } + val passedCount = results.count { it == CompatibilityResult.PASSED } + + report = "Files checked: ${results.size} (Passed: ${passedCount}, Failed: $failedCount)" + exitCode = if(failed) 1 else 0 + } + +} diff --git a/application/src/main/kotlin/application/backwardCompatibility/CompatibilityResult.kt b/application/src/main/kotlin/application/backwardCompatibility/CompatibilityResult.kt new file mode 100644 index 000000000..c53e7255f --- /dev/null +++ b/application/src/main/kotlin/application/backwardCompatibility/CompatibilityResult.kt @@ -0,0 +1,5 @@ +package application.backwardCompatibility + +enum class CompatibilityResult { + PASSED, FAILED +} diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt index 4cb9a4de4..a71ea7c9b 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt @@ -1,5 +1,6 @@ package application +import application.backwardCompatibility.BackwardCompatibilityCheckCommand import io.mockk.every import io.mockk.mockk import io.mockk.spyk @@ -13,7 +14,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `filesReferringToChangedSchemaFiles returns empty set when input is empty`() { - val command = BackwardCompatibilityCheckCommand(mockk()) + val command = BackwardCompatibilityCheckCommand() val result = command.filesReferringToChangedSchemaFiles(emptySet()) assertTrue(result.isEmpty()) } diff --git a/application/src/test/kotlin/application/CompatibleCommandKtTest.kt b/application/src/test/kotlin/application/CompatibleCommandKtTest.kt index 120a05317..1b4d9682e 100644 --- a/application/src/test/kotlin/application/CompatibleCommandKtTest.kt +++ b/application/src/test/kotlin/application/CompatibleCommandKtTest.kt @@ -6,6 +6,7 @@ import io.specmatic.core.Result import io.specmatic.core.Results import io.specmatic.core.git.GitCommand import io.mockk.every +import io.specmatic.core.git.SystemGit import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -46,8 +47,12 @@ internal class CompatibleCommandKtTest { override val workingDirectory: String get() = "" - override fun getFilesChangeInCurrentBranch() = emptyList() - override fun getFileInTheDefaultBranch(fileName: String, currentBranch: String) = null + override fun stashPop(): SystemGit { + TODO("Not yet implemented") + } + + override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() + override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") @@ -77,9 +82,9 @@ internal class CompatibleCommandKtTest { override fun fileIsInGitDir(newerContractPath: String): Boolean = true override val workingDirectory: String get() = "" - override fun getFileInTheDefaultBranch(fileName: String, currentBranch: String) = null + override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null - override fun getFilesChangeInCurrentBranch() = emptyList() + override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") @@ -114,10 +119,10 @@ internal class CompatibleCommandKtTest { val fakeGit = object : FakeGit() { override fun fileIsInGitDir(newerContractPath: String): Boolean = true - override fun getFilesChangeInCurrentBranch() = emptyList() + override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() override val workingDirectory: String get() = "" - override fun getFileInTheDefaultBranch(fileName: String, currentBranch: String) = null + override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") @@ -151,10 +156,10 @@ internal class CompatibleCommandKtTest { val fakeGit = object : FakeGit() { override fun fileIsInGitDir(newerContractPath: String): Boolean = true - override fun getFilesChangeInCurrentBranch() = emptyList() + override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() override val workingDirectory: String get() = "" - override fun getFileInTheDefaultBranch(fileName: String, currentBranch: String) = null + override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") diff --git a/application/src/test/kotlin/application/FakeGit.kt b/application/src/test/kotlin/application/FakeGit.kt index 4e9fe8022..4a67739d7 100644 --- a/application/src/test/kotlin/application/FakeGit.kt +++ b/application/src/test/kotlin/application/FakeGit.kt @@ -16,6 +16,18 @@ abstract class FakeGit: GitCommand { TODO("Not yet implemented") } + override fun stash(): Boolean { + TODO("Not yet implemented") + } + + override fun stashPop(): SystemGit { + TODO("Not yet implemented") + } + + override fun currentRemoteBranch(): String { + TODO("Not yet implemented") + } + override fun add(relativePath: String): SystemGit { TODO("Not yet implemented") } diff --git a/core/src/main/kotlin/io/specmatic/core/Feature.kt b/core/src/main/kotlin/io/specmatic/core/Feature.kt index a67c0c5f3..68bc043ee 100644 --- a/core/src/main/kotlin/io/specmatic/core/Feature.kt +++ b/core/src/main/kotlin/io/specmatic/core/Feature.kt @@ -108,7 +108,7 @@ data class Feature( val stubsFromExamples: Map>> = emptyMap(), val specmaticConfig: SpecmaticConfig = SpecmaticConfig(), val flagsBased: FlagsBased = strategiesFromFlags(specmaticConfig) -) { +): IFeature { fun enableGenerativeTesting(onlyPositive: Boolean = false): Feature { val updatedSpecmaticConfig = specmaticConfig.copy( test = specmaticConfig.test?.copy( diff --git a/core/src/main/kotlin/io/specmatic/core/IFeature.kt b/core/src/main/kotlin/io/specmatic/core/IFeature.kt new file mode 100644 index 000000000..9b7f4881b --- /dev/null +++ b/core/src/main/kotlin/io/specmatic/core/IFeature.kt @@ -0,0 +1,3 @@ +package io.specmatic.core + +interface IFeature \ No newline at end of file diff --git a/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt b/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt index 904c0c28a..48d28ca44 100644 --- a/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt +++ b/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt @@ -9,6 +9,8 @@ interface GitCommand { fun commit(): SystemGit fun push(): SystemGit fun pull(): SystemGit + fun stash(): Boolean + fun stashPop(): SystemGit fun resetHard(): SystemGit fun resetMixed(): SystemGit fun mergeAbort(): SystemGit @@ -30,8 +32,9 @@ interface GitCommand { fun revisionsBehindCount(): Int fun getRemoteUrl(name: String = "origin"): String fun checkIgnore(path: String): String - fun getFilesChangeInCurrentBranch(): List - fun getFileInTheDefaultBranch(fileName: String, currentBranch: String): File? + fun getFilesChangedInCurrentBranch(baseBranch: String): List + fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String): File? + fun currentRemoteBranch(): String fun currentBranch(): String { return "" } @@ -43,4 +46,5 @@ interface GitCommand { fun detachedHEAD(): String { return "" } + } diff --git a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt index 48f0c5113..839a24254 100644 --- a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt +++ b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt @@ -58,6 +58,14 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: } } + override fun stash(): Boolean { + val stashListSizeBefore = getStashListSize() + execute(Configuration.gitCommand, "stash", "push", "-m", "tmp") + return getStashListSize() > stashListSizeBefore + } + + override fun stashPop(): SystemGit = this.also { execute(Configuration.gitCommand, "stash", "pop") } + override fun getCurrentBranch(): String { return execute(Configuration.gitCommand, "git", "diff", "--name-only", "master") } @@ -82,17 +90,21 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: } } - override fun getFilesChangeInCurrentBranch(): List { - val defaultBranch = defaultBranch() + override fun getFilesChangedInCurrentBranch(baseBranch: String): List { + val committedLocalChanges = execute(Configuration.gitCommand, "diff", baseBranch, "HEAD", "--name-only") + .split(System.lineSeparator()) + .filter { it.isNotBlank() } - val result = execute(Configuration.gitCommand, "diff", defaultBranch, "HEAD", "--name-only") + val uncommittedChanges = execute(Configuration.gitCommand, "diff", "--name-only") + .split(System.lineSeparator()) + .filter { it.isNotBlank() } - return result.split(System.lineSeparator()).filter { it.isNotBlank() } + return (committedLocalChanges + uncommittedChanges).distinct() } - override fun getFileInTheDefaultBranch(fileName: String, currentBranch: String): File? { + override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String): File? { try { - checkout(defaultBranch()) + if(baseBranch != currentBranch) checkout(baseBranch) if (!File(fileName).exists()) return null return File(fileName) @@ -156,6 +168,15 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: return execute(Configuration.gitCommand, "rev-parse", "--abbrev-ref", "HEAD").trim() } + override fun currentRemoteBranch(): String { + val branchStatus = execute(Configuration.gitCommand, "status", "-b", "--porcelain=2").trim() + val hasUpstream = branchStatus.lines().any { it.startsWith("# branch.upstream") } + if (hasUpstream) { + return execute(Configuration.gitCommand, "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}").trim() + } + return currentBranch() + } + override fun defaultBranch(): String { System.getenv("LOCAL_GIT_BRANCH")?.let { return it @@ -174,6 +195,10 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: return symbolicRef.split("/")[1].trim() } + private fun getStashListSize(): Int { + return execute(Configuration.gitCommand, "stash", "list").trim().lines().size + } + override fun detachedHEAD(): String { val result = execute(Configuration.gitCommand, "show", "-s", "--pretty=%D", "HEAD") return result.trim().split(",")[1].trim() From 85fe8a4fc31175fbfe09726fb7aa8199973b7640 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 15 Aug 2024 17:07:08 +0530 Subject: [PATCH 02/18] Add support for --target-path argument in BackwardCompatibilityCheckCommand which can let us run the check on a specific folder or file in a repository --- .../kotlin/application/SpecmaticCommand.kt | 3 +- .../backwardCompatibility/BCCheckCommand.kt | 137 -------- .../BackwardCompatibilityCheckBaseCommand.kt | 11 +- .../BackwardCompatibilityCheckCommandV1.kt | 310 ++++++++++++++++++ ...ackwardCompatibilityCheckCommandV1Test.kt} | 13 +- 5 files changed, 327 insertions(+), 147 deletions(-) delete mode 100644 application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt create mode 100644 application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt rename application/src/test/kotlin/application/{BackwardCompatibilityCheckCommandTest.kt => BackwardCompatibilityCheckCommandV1Test.kt} (89%) diff --git a/application/src/main/kotlin/application/SpecmaticCommand.kt b/application/src/main/kotlin/application/SpecmaticCommand.kt index 3f301577b..170d188e1 100644 --- a/application/src/main/kotlin/application/SpecmaticCommand.kt +++ b/application/src/main/kotlin/application/SpecmaticCommand.kt @@ -1,6 +1,5 @@ package application -import application.backwardCompatibility.BCCheckCommand import application.backwardCompatibility.BackwardCompatibilityCheckCommand import org.springframework.stereotype.Component import picocli.AutoComplete.GenerateCompletion @@ -12,7 +11,7 @@ import java.util.concurrent.Callable name = "specmatic", mixinStandardHelpOptions = true, versionProvider = VersionProvider::class, - subcommands = [BCCheckCommand::class, BundleCommand::class, CompareCommand::class, CompatibleCommand::class, DifferenceCommand::class, GenerateCompletion::class, GraphCommand::class, MergeCommand::class, ToOpenAPICommand::class, ImportCommand::class, InstallCommand::class, ProxyCommand::class, PushCommand::class, ReDeclaredAPICommand::class, ExamplesCommand::class, SamplesCommand::class, StubCommand::class, SubscribeCommand::class, TestCommand::class, ValidateViaLogs::class, CentralContractRepoReportCommand::class] + subcommands = [BackwardCompatibilityCheckCommand::class, BundleCommand::class, CompareCommand::class, CompatibleCommand::class, DifferenceCommand::class, GenerateCompletion::class, GraphCommand::class, MergeCommand::class, ToOpenAPICommand::class, ImportCommand::class, InstallCommand::class, ProxyCommand::class, PushCommand::class, ReDeclaredAPICommand::class, ExamplesCommand::class, SamplesCommand::class, StubCommand::class, SubscribeCommand::class, TestCommand::class, ValidateViaLogs::class, CentralContractRepoReportCommand::class] ) class SpecmaticCommand : Callable { override fun call(): Int { diff --git a/application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt deleted file mode 100644 index e3747c49e..000000000 --- a/application/src/main/kotlin/application/backwardCompatibility/BCCheckCommand.kt +++ /dev/null @@ -1,137 +0,0 @@ -package application.backwardCompatibility - -import io.specmatic.conversions.OpenApiSpecification -import io.specmatic.core.CONTRACT_EXTENSION -import io.specmatic.core.CONTRACT_EXTENSIONS -import io.specmatic.core.Feature -import io.specmatic.core.IFeature -import io.specmatic.core.Results -import io.specmatic.core.WSDL -import io.specmatic.core.testBackwardCompatibility -import io.specmatic.stub.isOpenAPI -import org.springframework.stereotype.Component -import picocli.CommandLine.Command -import java.io.File -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.Paths -import java.util.regex.Pattern -import kotlin.io.path.extension -import kotlin.io.path.pathString - -@Component -@Command( - name = "backwardCompatibilityCheck", - aliases = ["backward-compatibility-check"], - mixinStandardHelpOptions = true, - description = ["Checks backward compatibility of a directory across the current HEAD and the base branch"] -) -class BCCheckCommand: BackwardCompatibilityCheckBaseCommand() { - - override fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results { - return testBackwardCompatibility(oldFeature as Feature, newFeature as Feature) - } - - override fun File.isValidSpec(): Boolean { - if (this.extension !in CONTRACT_EXTENSIONS) return false - return OpenApiSpecification.isParsable(this.path) - } - - override fun getFeatureFromSpecPath(path: String): Feature { - return OpenApiSpecification.fromFile(path).toFeature() - } - - override fun getSpecsReferringTo(schemaFiles: Set): Set { - if (schemaFiles.isEmpty()) return emptySet() - - val inputFileNames = schemaFiles.map { File(it).name } - val result = allOpenApiSpecFiles().filter { - it.readText().trim().let { specContent -> - inputFileNames.any { inputFileName -> - val pattern = Pattern.compile("\\b$inputFileName\\b") - val matcher = pattern.matcher(specContent) - matcher.find() - } - } - }.map { it.path }.toSet() - - return result.flatMap { - getSpecsReferringTo(setOf(it)).ifEmpty { setOf(it) } - }.toSet() - } - - override fun getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch: Set): Set { - data class CollectedFiles( - val specifications: MutableSet = mutableSetOf(), - val examplesMissingSpecifications: MutableList = mutableListOf(), - val ignoredFiles: MutableList = mutableListOf() - ) - - val collectedFiles = filesChangedInCurrentBranch.fold(CollectedFiles()) { acc, filePath -> - val path = Paths.get(filePath) - val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } - - if (examplesDir == null) { - acc.ignoredFiles.add(filePath) - } else { - val parentPath = examplesDir.parent - val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) - val specFiles = findSpecFiles(strippedPath) - - if (specFiles.isNotEmpty()) { - acc.specifications.addAll(specFiles.map { it.toString() }) - } else { - acc.examplesMissingSpecifications.add(filePath) - } - } - acc - } - - val result = collectedFiles.specifications.toMutableSet() - - collectedFiles.examplesMissingSpecifications.forEach { filePath -> - val path = Paths.get(filePath) - val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } - if (examplesDir != null) { - val parentPath = examplesDir.parent - val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) - val specFiles = findSpecFiles(strippedPath) - if (specFiles.isNotEmpty()) { - result.addAll(specFiles.map { it.toString() }) - } else { - result.add("${strippedPath}.yaml") - } - } - } - - return result - } - - override fun areExamplesValid(feature: IFeature, which: String): Boolean { - feature as Feature - return try { - feature.validateExamplesOrException() - true - } catch (t: Throwable) { - println() - false - } - } - - override fun getUnusedExamples(feature: IFeature): Set { - feature as Feature - return feature.loadExternalisedExamplesAndListUnloadableExamples().second - } - - private fun allOpenApiSpecFiles(): List { - return File(".").walk().toList().filterNot { - ".git" in it.path - }.filter { it.isFile && it.isValidSpec() } - } - - private fun findSpecFiles(path: Path): List { - val extensions = CONTRACT_EXTENSIONS - return extensions.map { path.resolveSibling(path.fileName.toString() + it) } - .filter { Files.exists(it) && (isOpenAPI(it.pathString) || it.extension in listOf(WSDL, CONTRACT_EXTENSION)) } - } -} \ No newline at end of file diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 44493635e..d281efa5c 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -18,6 +18,9 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { @Option(names = ["--base-branch"], description = ["Base branch to compare the changes against"], required = false) var baseBranch: String? = null + @Option(names = ["--target-path"], description = ["Specification file or folder to run the check against"], required = false) + var targetPath: String? = null + abstract fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results abstract fun File.isValidSpec(): Boolean abstract fun getFeatureFromSpecPath(path: String): IFeature @@ -48,9 +51,15 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { filesReferringToChangedSchemaFiles + specificationsOfChangedExternalisedExamples + val filteredSpecs = if(targetPath.isNullOrBlank()) { + specificationsToCheck + } else { + specificationsToCheck.filter { it.contains(targetPath!!) }.toSet() + } + val result = try { runBackwardCompatibilityCheckFor( - files = specificationsToCheck, + files = filteredSpecs, baseBranch = baseBranch() ) } catch(e: Throwable) { diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt new file mode 100644 index 000000000..cfda2698b --- /dev/null +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt @@ -0,0 +1,310 @@ +package application.backwardCompatibility + +import io.specmatic.conversions.OpenApiSpecification +import io.specmatic.core.* +import io.specmatic.core.git.GitCommand +import io.specmatic.core.git.SystemGit +import io.specmatic.core.log.logger +import io.specmatic.core.utilities.exitWithMessage +import io.specmatic.stub.isOpenAPI +import picocli.CommandLine.Option +import java.io.File +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import java.util.concurrent.Callable +import java.util.regex.Pattern +import kotlin.io.path.extension +import kotlin.io.path.pathString +import kotlin.system.exitProcess + +//@Component +//@Command( +// name = "backwardCompatibilityCheck", +// aliases = ["backward-compatibility-check"], +// mixinStandardHelpOptions = true, +// description = ["Checks backward compatibility of a directory across the current HEAD and the base branch"] +//) +// TODO - knock this off. +class BackwardCompatibilityCheckCommandV1() : Callable { + private val gitCommand: GitCommand = SystemGit() + private val newLine = System.lineSeparator() + + @Option(names = ["--base-branch"], description = ["Base branch to compare the changes against"], required = false) + var baseBranch: String = gitCommand.currentRemoteBranch() + + companion object { + private const val HEAD = "HEAD" + private const val MARGIN_SPACE = " " + private const val ONE_INDENT = " " + private const val TWO_INDENTS = "${ONE_INDENT}${ONE_INDENT}" + } + + override fun call() { + val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch() + val filesReferringToChangedSchemaFiles = filesReferringToChangedSchemaFiles(filesChangedInCurrentBranch) + val specificationsOfChangedExternalisedExamples = + getSpecificationsOfChangedExternalisedExamples(filesChangedInCurrentBranch) + + logFilesToBeCheckedForBackwardCompatibility( + filesChangedInCurrentBranch, + filesReferringToChangedSchemaFiles, + specificationsOfChangedExternalisedExamples + ) + + val specificationsToCheck: Set = + filesChangedInCurrentBranch + + filesReferringToChangedSchemaFiles + + specificationsOfChangedExternalisedExamples + + val result = try { + runBackwardCompatibilityCheckFor( + files = specificationsToCheck, + baseBranch = baseBranch + ) + } catch(e: Throwable) { + logger.newLine() + logger.newLine() + logger.log(e) + exitProcess(1) + } + + println() + println(result.report) + exitProcess(result.exitCode) + } + + private fun getSpecificationsOfChangedExternalisedExamples(filesChangedInCurrentBranch: Set): Set { + data class CollectedFiles( + val specifications: MutableSet = mutableSetOf(), + val examplesMissingSpecifications: MutableList = mutableListOf(), + val ignoredFiles: MutableList = mutableListOf() + ) + + val collectedFiles = filesChangedInCurrentBranch.fold(CollectedFiles()) { acc, filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + + if (examplesDir == null) { + acc.ignoredFiles.add(filePath) + } else { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + + if (specFiles.isNotEmpty()) { + acc.specifications.addAll(specFiles.map { it.toString() }) + } else { + acc.examplesMissingSpecifications.add(filePath) + } + } + acc + } + + val result = collectedFiles.specifications.toMutableSet() + + collectedFiles.examplesMissingSpecifications.forEach { filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + if (examplesDir != null) { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + if (specFiles.isNotEmpty()) { + result.addAll(specFiles.map { it.toString() }) + } else { + result.add("${strippedPath}.yaml") + } + } + } + + return result + } + + private fun Path.find(predicate: (Path) -> Boolean): Path? { + var current: Path? = this + while (current != null) { + if (predicate(current)) { + return current + } + current = current.parent + } + return null + } + + private fun findSpecFiles(path: Path): List { + val extensions = CONTRACT_EXTENSIONS + return extensions.map { path.resolveSibling(path.fileName.toString() + it) } + .filter { Files.exists(it) && (isOpenAPI(it.pathString) || it.extension in listOf(WSDL, CONTRACT_EXTENSION)) } + } + + fun runBackwardCompatibilityCheckFor(files: Set, baseBranch: String): CompatibilityReport { + val branchWithChanges = gitCommand.currentBranch() + val treeishWithChanges = if (branchWithChanges == HEAD) gitCommand.detachedHEAD() else branchWithChanges + + try { + val results = files.mapIndexed { index, specFilePath -> + try { + println("${index.inc()}. Running the check for $specFilePath:") + + // newer => the file with changes on the branch + val newer = getFeatureFromSpecPath(specFilePath) + val unusedExamples = getUnusedExamples(newer) + + val olderFile = gitCommand.getFileInTheBaseBranch( + specFilePath, + treeishWithChanges, + baseBranch + ) + if (olderFile == null) { + println("$specFilePath is a new file.$newLine") + return@mapIndexed CompatibilityResult.PASSED + } + + gitCommand.stash() + gitCommand.checkout(baseBranch) + // older => the same file on the default (e.g. main) branch + val older = getFeatureFromSpecPath(olderFile.path) + gitCommand.stashPop() + + val backwardCompatibilityResult = checkBackwardCompatibility(older, newer) + + if (backwardCompatibilityResult.success()) { + println( + "$newLine The file $specFilePath is backward compatible.$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + var errorsFound = false + + if(!areExamplesValid(newer, "newer")) { + println( + "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + errorsFound = true + } + + if(unusedExamples.isNotEmpty()) { + println( + "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + errorsFound = true + } + + if(errorsFound) CompatibilityResult.FAILED + else CompatibilityResult.PASSED + } else { + println("$newLine ${backwardCompatibilityResult.report().prependIndent(MARGIN_SPACE)}") + println( + "$newLine *** The file $specFilePath is NOT backward compatible. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + CompatibilityResult.FAILED + } + } finally { + gitCommand.checkout(treeishWithChanges) + } + } + + return CompatibilityReport(results) + } finally { + gitCommand.checkout(treeishWithChanges) + } + } + + fun logFilesToBeCheckedForBackwardCompatibility( + changedFiles: Set, + filesReferringToChangedFiles: Set, + specificationsOfChangedExternalisedExamples: Set + ) { + + println("Checking backward compatibility of the following files: $newLine") + println("${ONE_INDENT}Files that have changed:") + changedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + + if(filesReferringToChangedFiles.isNotEmpty()) { + println("${ONE_INDENT}Files referring to the changed files - ") + filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + } + + if(specificationsOfChangedExternalisedExamples.isNotEmpty()) { + println("${ONE_INDENT}Specifications whose externalised examples were changed - ") + filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + } + + println("-".repeat(20)) + println() + } + + internal fun filesReferringToChangedSchemaFiles(inputFiles: Set): Set { + if (inputFiles.isEmpty()) return emptySet() + + val inputFileNames = inputFiles.map { File(it).name } + val result = allOpenApiSpecFiles().filter { + it.readText().trim().let { specContent -> + inputFileNames.any { inputFileName -> + val pattern = Pattern.compile("\\b$inputFileName\\b") + val matcher = pattern.matcher(specContent) + matcher.find() + } + } + }.map { it.path }.toSet() + + return result.flatMap { + filesReferringToChangedSchemaFiles(setOf(it)).ifEmpty { setOf(it) } + }.toSet() + } + + internal fun allOpenApiSpecFiles(): List { + return File(".").walk().toList().filterNot { + ".git" in it.path + }.filter { it.isFile && it.isValidSpec() } + } + + private fun getChangedSpecsInCurrentBranch(): Set { + return gitCommand.getFilesChangedInCurrentBranch(baseBranch).filter { + File(it).exists() && File(it).isValidSpec() + }.toSet().also { + if(it.isEmpty()) exitWithMessage("${newLine}No specs were changed, skipping the check.$newLine") + } + } + + private fun File.isValidSpec(): Boolean { + if (this.extension !in CONTRACT_EXTENSIONS) return false + return OpenApiSpecification.isParsable(this.path) + } + + private fun areExamplesValid(feature: Feature, which: String): Boolean { + return try { + feature.validateExamplesOrException() + true + } catch (t: Throwable) { + println() + false + } + } + + private fun getUnusedExamples(feature: Feature): Set { + return feature.loadExternalisedExamplesAndListUnloadableExamples().second + } + + private fun getFeatureFromSpecPath(path: String): Feature { + return OpenApiSpecification.fromFile(path).toFeature() + } + + private fun checkBackwardCompatibility(oldFeature: Feature, newFeature: Feature): Results { + return testBackwardCompatibility(oldFeature, newFeature) + } +} diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV1Test.kt similarity index 89% rename from application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt rename to application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV1Test.kt index a71ea7c9b..e75d6c424 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV1Test.kt @@ -1,8 +1,7 @@ package application -import application.backwardCompatibility.BackwardCompatibilityCheckCommand +import application.backwardCompatibility.BackwardCompatibilityCheckCommandV1 import io.mockk.every -import io.mockk.mockk import io.mockk.spyk import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals @@ -10,18 +9,18 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.io.File -class BackwardCompatibilityCheckCommandTest { +class BackwardCompatibilityCheckCommandV1Test { @Test fun `filesReferringToChangedSchemaFiles returns empty set when input is empty`() { - val command = BackwardCompatibilityCheckCommand() + val command = BackwardCompatibilityCheckCommandV1() val result = command.filesReferringToChangedSchemaFiles(emptySet()) assertTrue(result.isEmpty()) } @Test fun `filesReferringToChangedSchemaFiles returns empty set when no files refer to changed schema files`() { - val command = spyk() + val command = spyk() every { command.allOpenApiSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("content1") }, File("file2.yaml").apply { writeText("content2") } @@ -32,7 +31,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `filesReferringToChangedSchemaFiles returns set of files that refer to changed schema files`() { - val command = spyk() + val command = spyk() every { command.allOpenApiSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("file3.yaml") }, File("file2.yaml").apply { writeText("file4.yaml") } @@ -43,7 +42,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `filesReferringToChangedSchemaFiles returns set of files which are referring to a changed schema that is one level down`() { - val command = spyk() + val command = spyk() every { command.allOpenApiSpecFiles() } returns listOf( File("file1.yaml").apply { referTo("schema_file1.yaml") }, File("schema_file2.yaml").apply { referTo("schema_file1.yaml") }, // schema within a schema From cc98aeaeffdf76c3f402aac2f0e949eb2d6643dc Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 15 Aug 2024 17:50:30 +0530 Subject: [PATCH 03/18] Move the function to find referred spec files within BCC base class since the logic is reusable. --- .../BackwardCompatibilityCheckBaseCommand.kt | 85 +++-- .../BackwardCompatibilityCheckCommandV1.kt | 310 ------------------ ... BackwardCompatibilityCheckCommandTest.kt} | 34 +- 3 files changed, 74 insertions(+), 355 deletions(-) delete mode 100644 application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt rename application/src/test/kotlin/application/{BackwardCompatibilityCheckCommandV1Test.kt => BackwardCompatibilityCheckCommandTest.kt} (57%) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index d281efa5c..967664c8e 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -9,6 +9,7 @@ import io.specmatic.core.utilities.exitWithMessage import picocli.CommandLine.Option import java.io.File import java.util.concurrent.Callable +import java.util.regex.Pattern import kotlin.system.exitProcess abstract class BackwardCompatibilityCheckBaseCommand : Callable { @@ -19,44 +20,22 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { var baseBranch: String? = null @Option(names = ["--target-path"], description = ["Specification file or folder to run the check against"], required = false) - var targetPath: String? = null + var targetPath: String = "" abstract fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results abstract fun File.isValidSpec(): Boolean abstract fun getFeatureFromSpecPath(path: String): IFeature - abstract fun getSpecsReferringTo(schemaFiles: Set): Set + abstract fun regexForMatchingReferred(schemaFileName: String): String abstract fun getSpecsOfChangedExternalisedExamples( filesChangedInCurrentBranch: Set ): Set - open fun areExamplesValid(feature: IFeature, which: String): Boolean = true open fun getUnusedExamples(feature: IFeature): Set = emptySet() - override fun call() { - val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch() - val filesReferringToChangedSchemaFiles = getSpecsReferringTo(filesChangedInCurrentBranch) - val specificationsOfChangedExternalisedExamples = - getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch) - - logFilesToBeCheckedForBackwardCompatibility( - filesChangedInCurrentBranch, - filesReferringToChangedSchemaFiles, - specificationsOfChangedExternalisedExamples - ) - - val specificationsToCheck: Set = - filesChangedInCurrentBranch + - filesReferringToChangedSchemaFiles + - specificationsOfChangedExternalisedExamples - - val filteredSpecs = if(targetPath.isNullOrBlank()) { - specificationsToCheck - } else { - specificationsToCheck.filter { it.contains(targetPath!!) }.toSet() - } - + final override fun call() { + val filteredSpecs = getChangedSpecs(logSpecs = true) val result = try { runBackwardCompatibilityCheckFor( files = filteredSpecs, @@ -74,6 +53,30 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { exitProcess(result.exitCode) } + fun getChangedSpecs(logSpecs: Boolean = false): Set { + val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch() + val filesReferringToChangedSchemaFiles = getSpecsReferringTo(filesChangedInCurrentBranch) + val specificationsOfChangedExternalisedExamples = + getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch) + + if(logSpecs) { + logFilesToBeCheckedForBackwardCompatibility( + filesChangedInCurrentBranch, + filesReferringToChangedSchemaFiles, + specificationsOfChangedExternalisedExamples + ) + } + + val specificationsToCheck: Set = + filesChangedInCurrentBranch + + filesReferringToChangedSchemaFiles + + specificationsOfChangedExternalisedExamples + + val filteredSpecs = specificationsToCheck.filter { it.contains(targetPath) }.toSet() + + return filteredSpecs + } + private fun getChangedSpecsInCurrentBranch(): Set { return gitCommand.getFilesChangedInCurrentBranch( baseBranch() @@ -84,6 +87,31 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } } + internal fun getSpecsReferringTo(schemaFiles: Set): Set { + if (schemaFiles.isEmpty()) return emptySet() + + val inputFileNames = schemaFiles.map { File(it).name } + val result = allSpecFiles().filter { + it.readText().trim().let { specContent -> + inputFileNames.any { inputFileName -> + val pattern = Pattern.compile("\\b${regexForMatchingReferred(inputFileName)}\\b") + val matcher = pattern.matcher(specContent) + matcher.find() + } + } + }.map { it.path }.toSet() + + return result.flatMap { + getSpecsReferringTo(setOf(it)).ifEmpty { setOf(it) } + }.toSet() + } + + internal fun allSpecFiles(): List { + return File(".").walk().toList().filterNot { + ".git" in it.path + }.filter { it.isFile && it.isValidSpec() } + } + private fun logFilesToBeCheckedForBackwardCompatibility( changedFiles: Set, filesReferringToChangedFiles: Set, @@ -117,6 +145,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { try { val results = files.mapIndexed { index, specFilePath -> + var areLocalChangesStashed = false try { println("${index.inc()}. Running the check for $specFilePath:") @@ -134,11 +163,10 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { return@mapIndexed CompatibilityResult.PASSED } - val areLocalChangesStashed = gitCommand.stash() + areLocalChangesStashed = gitCommand.stash() gitCommand.checkout(baseBranch) // older => the same file on the default (e.g. main) branch val older = getFeatureFromSpecPath(olderFile.path) - if (areLocalChangesStashed) gitCommand.stashPop() val backwardCompatibilityResult = checkBackwardCompatibility(older, newer) @@ -187,6 +215,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } } finally { gitCommand.checkout(treeishWithChanges) + if (areLocalChangesStashed) gitCommand.stashPop() } } diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt deleted file mode 100644 index cfda2698b..000000000 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV1.kt +++ /dev/null @@ -1,310 +0,0 @@ -package application.backwardCompatibility - -import io.specmatic.conversions.OpenApiSpecification -import io.specmatic.core.* -import io.specmatic.core.git.GitCommand -import io.specmatic.core.git.SystemGit -import io.specmatic.core.log.logger -import io.specmatic.core.utilities.exitWithMessage -import io.specmatic.stub.isOpenAPI -import picocli.CommandLine.Option -import java.io.File -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.Paths -import java.util.concurrent.Callable -import java.util.regex.Pattern -import kotlin.io.path.extension -import kotlin.io.path.pathString -import kotlin.system.exitProcess - -//@Component -//@Command( -// name = "backwardCompatibilityCheck", -// aliases = ["backward-compatibility-check"], -// mixinStandardHelpOptions = true, -// description = ["Checks backward compatibility of a directory across the current HEAD and the base branch"] -//) -// TODO - knock this off. -class BackwardCompatibilityCheckCommandV1() : Callable { - private val gitCommand: GitCommand = SystemGit() - private val newLine = System.lineSeparator() - - @Option(names = ["--base-branch"], description = ["Base branch to compare the changes against"], required = false) - var baseBranch: String = gitCommand.currentRemoteBranch() - - companion object { - private const val HEAD = "HEAD" - private const val MARGIN_SPACE = " " - private const val ONE_INDENT = " " - private const val TWO_INDENTS = "${ONE_INDENT}${ONE_INDENT}" - } - - override fun call() { - val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch() - val filesReferringToChangedSchemaFiles = filesReferringToChangedSchemaFiles(filesChangedInCurrentBranch) - val specificationsOfChangedExternalisedExamples = - getSpecificationsOfChangedExternalisedExamples(filesChangedInCurrentBranch) - - logFilesToBeCheckedForBackwardCompatibility( - filesChangedInCurrentBranch, - filesReferringToChangedSchemaFiles, - specificationsOfChangedExternalisedExamples - ) - - val specificationsToCheck: Set = - filesChangedInCurrentBranch + - filesReferringToChangedSchemaFiles + - specificationsOfChangedExternalisedExamples - - val result = try { - runBackwardCompatibilityCheckFor( - files = specificationsToCheck, - baseBranch = baseBranch - ) - } catch(e: Throwable) { - logger.newLine() - logger.newLine() - logger.log(e) - exitProcess(1) - } - - println() - println(result.report) - exitProcess(result.exitCode) - } - - private fun getSpecificationsOfChangedExternalisedExamples(filesChangedInCurrentBranch: Set): Set { - data class CollectedFiles( - val specifications: MutableSet = mutableSetOf(), - val examplesMissingSpecifications: MutableList = mutableListOf(), - val ignoredFiles: MutableList = mutableListOf() - ) - - val collectedFiles = filesChangedInCurrentBranch.fold(CollectedFiles()) { acc, filePath -> - val path = Paths.get(filePath) - val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } - - if (examplesDir == null) { - acc.ignoredFiles.add(filePath) - } else { - val parentPath = examplesDir.parent - val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) - val specFiles = findSpecFiles(strippedPath) - - if (specFiles.isNotEmpty()) { - acc.specifications.addAll(specFiles.map { it.toString() }) - } else { - acc.examplesMissingSpecifications.add(filePath) - } - } - acc - } - - val result = collectedFiles.specifications.toMutableSet() - - collectedFiles.examplesMissingSpecifications.forEach { filePath -> - val path = Paths.get(filePath) - val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } - if (examplesDir != null) { - val parentPath = examplesDir.parent - val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) - val specFiles = findSpecFiles(strippedPath) - if (specFiles.isNotEmpty()) { - result.addAll(specFiles.map { it.toString() }) - } else { - result.add("${strippedPath}.yaml") - } - } - } - - return result - } - - private fun Path.find(predicate: (Path) -> Boolean): Path? { - var current: Path? = this - while (current != null) { - if (predicate(current)) { - return current - } - current = current.parent - } - return null - } - - private fun findSpecFiles(path: Path): List { - val extensions = CONTRACT_EXTENSIONS - return extensions.map { path.resolveSibling(path.fileName.toString() + it) } - .filter { Files.exists(it) && (isOpenAPI(it.pathString) || it.extension in listOf(WSDL, CONTRACT_EXTENSION)) } - } - - fun runBackwardCompatibilityCheckFor(files: Set, baseBranch: String): CompatibilityReport { - val branchWithChanges = gitCommand.currentBranch() - val treeishWithChanges = if (branchWithChanges == HEAD) gitCommand.detachedHEAD() else branchWithChanges - - try { - val results = files.mapIndexed { index, specFilePath -> - try { - println("${index.inc()}. Running the check for $specFilePath:") - - // newer => the file with changes on the branch - val newer = getFeatureFromSpecPath(specFilePath) - val unusedExamples = getUnusedExamples(newer) - - val olderFile = gitCommand.getFileInTheBaseBranch( - specFilePath, - treeishWithChanges, - baseBranch - ) - if (olderFile == null) { - println("$specFilePath is a new file.$newLine") - return@mapIndexed CompatibilityResult.PASSED - } - - gitCommand.stash() - gitCommand.checkout(baseBranch) - // older => the same file on the default (e.g. main) branch - val older = getFeatureFromSpecPath(olderFile.path) - gitCommand.stashPop() - - val backwardCompatibilityResult = checkBackwardCompatibility(older, newer) - - if (backwardCompatibilityResult.success()) { - println( - "$newLine The file $specFilePath is backward compatible.$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - var errorsFound = false - - if(!areExamplesValid(newer, "newer")) { - println( - "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - errorsFound = true - } - - if(unusedExamples.isNotEmpty()) { - println( - "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - errorsFound = true - } - - if(errorsFound) CompatibilityResult.FAILED - else CompatibilityResult.PASSED - } else { - println("$newLine ${backwardCompatibilityResult.report().prependIndent(MARGIN_SPACE)}") - println( - "$newLine *** The file $specFilePath is NOT backward compatible. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - CompatibilityResult.FAILED - } - } finally { - gitCommand.checkout(treeishWithChanges) - } - } - - return CompatibilityReport(results) - } finally { - gitCommand.checkout(treeishWithChanges) - } - } - - fun logFilesToBeCheckedForBackwardCompatibility( - changedFiles: Set, - filesReferringToChangedFiles: Set, - specificationsOfChangedExternalisedExamples: Set - ) { - - println("Checking backward compatibility of the following files: $newLine") - println("${ONE_INDENT}Files that have changed:") - changedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } - println() - - if(filesReferringToChangedFiles.isNotEmpty()) { - println("${ONE_INDENT}Files referring to the changed files - ") - filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } - println() - } - - if(specificationsOfChangedExternalisedExamples.isNotEmpty()) { - println("${ONE_INDENT}Specifications whose externalised examples were changed - ") - filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } - println() - } - - println("-".repeat(20)) - println() - } - - internal fun filesReferringToChangedSchemaFiles(inputFiles: Set): Set { - if (inputFiles.isEmpty()) return emptySet() - - val inputFileNames = inputFiles.map { File(it).name } - val result = allOpenApiSpecFiles().filter { - it.readText().trim().let { specContent -> - inputFileNames.any { inputFileName -> - val pattern = Pattern.compile("\\b$inputFileName\\b") - val matcher = pattern.matcher(specContent) - matcher.find() - } - } - }.map { it.path }.toSet() - - return result.flatMap { - filesReferringToChangedSchemaFiles(setOf(it)).ifEmpty { setOf(it) } - }.toSet() - } - - internal fun allOpenApiSpecFiles(): List { - return File(".").walk().toList().filterNot { - ".git" in it.path - }.filter { it.isFile && it.isValidSpec() } - } - - private fun getChangedSpecsInCurrentBranch(): Set { - return gitCommand.getFilesChangedInCurrentBranch(baseBranch).filter { - File(it).exists() && File(it).isValidSpec() - }.toSet().also { - if(it.isEmpty()) exitWithMessage("${newLine}No specs were changed, skipping the check.$newLine") - } - } - - private fun File.isValidSpec(): Boolean { - if (this.extension !in CONTRACT_EXTENSIONS) return false - return OpenApiSpecification.isParsable(this.path) - } - - private fun areExamplesValid(feature: Feature, which: String): Boolean { - return try { - feature.validateExamplesOrException() - true - } catch (t: Throwable) { - println() - false - } - } - - private fun getUnusedExamples(feature: Feature): Set { - return feature.loadExternalisedExamplesAndListUnloadableExamples().second - } - - private fun getFeatureFromSpecPath(path: String): Feature { - return OpenApiSpecification.fromFile(path).toFeature() - } - - private fun checkBackwardCompatibility(oldFeature: Feature, newFeature: Feature): Results { - return testBackwardCompatibility(oldFeature, newFeature) - } -} diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV1Test.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt similarity index 57% rename from application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV1Test.kt rename to application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt index e75d6c424..6873da05c 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV1Test.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt @@ -1,6 +1,6 @@ package application -import application.backwardCompatibility.BackwardCompatibilityCheckCommandV1 +import application.backwardCompatibility.BackwardCompatibilityCheckCommand import io.mockk.every import io.mockk.spyk import org.junit.jupiter.api.AfterEach @@ -9,46 +9,46 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.io.File -class BackwardCompatibilityCheckCommandV1Test { +class BackwardCompatibilityCheckCommandTest { @Test - fun `filesReferringToChangedSchemaFiles returns empty set when input is empty`() { - val command = BackwardCompatibilityCheckCommandV1() - val result = command.filesReferringToChangedSchemaFiles(emptySet()) + fun `getSpecsReferringTo returns empty set when input is empty`() { + val command = BackwardCompatibilityCheckCommand() + val result = command.getSpecsReferringTo(emptySet()) assertTrue(result.isEmpty()) } @Test - fun `filesReferringToChangedSchemaFiles returns empty set when no files refer to changed schema files`() { - val command = spyk() - every { command.allOpenApiSpecFiles() } returns listOf( + fun `getSpecsReferringTo returns empty set when no files refer to changed schema files`() { + val command = spyk() + every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("content1") }, File("file2.yaml").apply { writeText("content2") } ) - val result = command.filesReferringToChangedSchemaFiles(setOf("file3.yaml")) + val result = command.getSpecsReferringTo(setOf("file3.yaml")) assertTrue(result.isEmpty()) } @Test - fun `filesReferringToChangedSchemaFiles returns set of files that refer to changed schema files`() { - val command = spyk() - every { command.allOpenApiSpecFiles() } returns listOf( + fun `getSpecsReferringTo returns set of files that refer to changed schema files`() { + val command = spyk() + every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("file3.yaml") }, File("file2.yaml").apply { writeText("file4.yaml") } ) - val result = command.filesReferringToChangedSchemaFiles(setOf("file3.yaml")) + val result = command.getSpecsReferringTo(setOf("file3.yaml")) assertEquals(setOf("file1.yaml"), result) } @Test - fun `filesReferringToChangedSchemaFiles returns set of files which are referring to a changed schema that is one level down`() { - val command = spyk() - every { command.allOpenApiSpecFiles() } returns listOf( + fun `getSpecsReferringTo returns set of files which are referring to a changed schema that is one level down`() { + val command = spyk() + every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { referTo("schema_file1.yaml") }, File("schema_file2.yaml").apply { referTo("schema_file1.yaml") }, // schema within a schema File("file2.yaml").apply { referTo("schema_file2.yaml") } ) - val result = command.filesReferringToChangedSchemaFiles(setOf("schema_file1.yaml")) + val result = command.getSpecsReferringTo(setOf("schema_file1.yaml")) assertEquals(setOf("file1.yaml", "file2.yaml"), result) } From f94775bd9e472d22a1b43752413f4a0f8f00edf2 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Wed, 21 Aug 2024 17:28:50 +0530 Subject: [PATCH 04/18] Filter only the specs that have changed using targetPath in BackwardCompatibilityCheckCommand --- .../BackwardCompatibilityCheckBaseCommand.kt | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 967664c8e..19a7b24ba 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -54,7 +54,9 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } fun getChangedSpecs(logSpecs: Boolean = false): Set { - val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch() + val filesChangedInCurrentBranch = getChangedSpecsInCurrentBranch().filter { + it.contains(targetPath) + }.toSet() val filesReferringToChangedSchemaFiles = getSpecsReferringTo(filesChangedInCurrentBranch) val specificationsOfChangedExternalisedExamples = getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch) @@ -67,14 +69,9 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { ) } - val specificationsToCheck: Set = - filesChangedInCurrentBranch + - filesReferringToChangedSchemaFiles + - specificationsOfChangedExternalisedExamples - - val filteredSpecs = specificationsToCheck.filter { it.contains(targetPath) }.toSet() - - return filteredSpecs + return filesChangedInCurrentBranch + + filesReferringToChangedSchemaFiles + + specificationsOfChangedExternalisedExamples } private fun getChangedSpecsInCurrentBranch(): Set { From 39c0899951a59d8ae71d224a66868dd4b42d6f8f Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Wed, 21 Aug 2024 17:58:58 +0530 Subject: [PATCH 05/18] Fix the logs in backwardCompatibilityCheckCommand --- .../BackwardCompatibilityCheckBaseCommand.kt | 110 ++++++++++-------- 1 file changed, 62 insertions(+), 48 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 19a7b24ba..8b3c330b3 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -115,19 +115,19 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { specificationsOfChangedExternalisedExamples: Set ) { - println("Checking backward compatibility of the following files: $newLine") - println("${ONE_INDENT}Files that have changed:") + println("Checking backward compatibility of the following specs: $newLine") + println("${ONE_INDENT}Specs that have changed:") changedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } println() if(filesReferringToChangedFiles.isNotEmpty()) { - println("${ONE_INDENT}Files referring to the changed files - ") + println("${ONE_INDENT}Specs referring to the changed specs - ") filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } println() } if(specificationsOfChangedExternalisedExamples.isNotEmpty()) { - println("${ONE_INDENT}Specifications whose externalised examples were changed - ") + println("${ONE_INDENT}Specs whose externalised examples were changed - ") filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } println() } @@ -167,49 +167,12 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { val backwardCompatibilityResult = checkBackwardCompatibility(older, newer) - if (backwardCompatibilityResult.success()) { - println( - "$newLine The file $specFilePath is backward compatible.$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - var errorsFound = false - - if(!areExamplesValid(newer, "newer")) { - println( - "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - errorsFound = true - } - - if(unusedExamples.isNotEmpty()) { - println( - "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - errorsFound = true - } - - if(errorsFound) CompatibilityResult.FAILED - else CompatibilityResult.PASSED - } else { - println("$newLine ${backwardCompatibilityResult.report().prependIndent( - MARGIN_SPACE - )}") - println( - "$newLine *** The file $specFilePath is NOT backward compatible. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - CompatibilityResult.FAILED - } + return@mapIndexed getCompatibilityResult( + backwardCompatibilityResult, + specFilePath, + newer, + unusedExamples + ) } finally { gitCommand.checkout(treeishWithChanges) if (areLocalChangesStashed) gitCommand.stashPop() @@ -224,10 +187,61 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { private fun baseBranch() = baseBranch ?: gitCommand.currentRemoteBranch() + private fun getCompatibilityResult( + backwardCompatibilityResult: Results, + specFilePath: String, + newer: IFeature, + unusedExamples: Set + ): CompatibilityResult { + if (backwardCompatibilityResult.success()) { + println( + "$newLine The spec $specFilePath is backward compatible with the corresponding spec from ${baseBranch()}$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + var errorsFound = false + + if(!areExamplesValid(newer, "newer")) { + println( + "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + errorsFound = true + } + + if(unusedExamples.isNotEmpty()) { + println( + "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + errorsFound = true + } + + return if(errorsFound) CompatibilityResult.FAILED + else CompatibilityResult.PASSED + } else { + println("$newLine ${backwardCompatibilityResult.report().prependIndent( + MARGIN_SPACE + )}") + println( + "$newLine *** The changes to the spec $specFilePath are NOT backward compatible with the corresponding spec from ${baseBranch()}***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + println() + return CompatibilityResult.FAILED + } + } + companion object { private const val HEAD = "HEAD" private const val MARGIN_SPACE = " " private const val ONE_INDENT = " " private const val TWO_INDENTS = "${ONE_INDENT}${ONE_INDENT}" } -} \ No newline at end of file +} From 6583333f12c96b76592b5a57198c6638d4d1fd53 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Mon, 26 Aug 2024 12:25:28 +0530 Subject: [PATCH 06/18] Bring back the existing backwardCompatibilityCheck command Add deprecation notice to all the b/w compatibility related commands which will be eventually removed --- .../BackwardCompatibilityCheckCommand.kt | 22 ++-- .../main/kotlin/application/CompareCommand.kt | 7 +- .../kotlin/application/CompatibleCommand.kt | 7 +- .../kotlin/application/DifferenceCommand.kt | 8 +- .../main/kotlin/application/PushCommand.kt | 13 +- .../kotlin/application/SpecmaticCommand.kt | 27 ++++- .../BackwardCompatibilityCheckBaseCommand.kt | 9 +- .../BackwardCompatibilityCheckCommandV2.kt | 112 ++++++++++++++++++ ...ackwardCompatibilityCheckCommandV2Test.kt} | 12 +- 9 files changed, 196 insertions(+), 21 deletions(-) rename application/src/main/kotlin/application/{backwardCompatibility => }/BackwardCompatibilityCheckCommand.kt (94%) create mode 100644 application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt rename application/src/test/kotlin/application/{BackwardCompatibilityCheckCommandTest.kt => BackwardCompatibilityCheckCommandV2Test.kt} (88%) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt similarity index 94% rename from application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt rename to application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt index 2e1f64e34..fd14d55c8 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt +++ b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt @@ -26,7 +26,12 @@ const val TWO_INDENTS = "${ONE_INDENT}${ONE_INDENT}" @Command( name = "backwardCompatibilityCheck", mixinStandardHelpOptions = true, - description = ["Checks backward compatibility of a directory across the current HEAD and the main branch"] + description = [ +""" +Checks backward compatibility of a directory across the current HEAD and the main branch. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ] ) class BackwardCompatibilityCheckCommand( private val gitCommand: GitCommand = SystemGit(), @@ -42,10 +47,7 @@ class BackwardCompatibilityCheckCommand( override fun call() { val filesChangedInCurrentBranch: Set = getOpenAPISpecFilesChangedInCurrentBranch() - if (filesChangedInCurrentBranch.isEmpty()) { - logger.log("${newLine}No OpenAPI spec files were changed, skipping the check.$newLine") - exitProcess(0) - } + if (filesChangedInCurrentBranch.isEmpty()) exitWithMessage("${newLine}No OpenAPI spec files were changed, skipping the check.$newLine") val filesReferringToChangedSchemaFiles = filesReferringToChangedSchemaFiles(filesChangedInCurrentBranch) @@ -149,7 +151,11 @@ class BackwardCompatibilityCheckCommand( // newer => the file with changes on the branch val (newer, unusedExamples) = OpenApiSpecification.fromFile(specFilePath).toFeature().loadExternalisedExamplesAndListUnloadableExamples() - val olderFile = gitCommand.getFileInTheDefaultBranch(specFilePath, treeishWithChanges) + val olderFile = gitCommand.getFileInTheBaseBranch( + specFilePath, + treeishWithChanges, + gitCommand.defaultBranch() + ) if (olderFile == null) { println("$specFilePath is a new file.$newLine") return@mapIndexed PASSED @@ -289,7 +295,9 @@ class BackwardCompatibilityCheckCommand( } private fun getOpenAPISpecFilesChangedInCurrentBranch(): Set { - return gitCommand.getFilesChangeInCurrentBranch().filter { + return gitCommand.getFilesChangedInCurrentBranch( + gitCommand.defaultBranch() + ).filter { File(it).exists() && File(it).isOpenApiSpec() }.toSet() } diff --git a/application/src/main/kotlin/application/CompareCommand.kt b/application/src/main/kotlin/application/CompareCommand.kt index bdb0b7dbc..61918d226 100644 --- a/application/src/main/kotlin/application/CompareCommand.kt +++ b/application/src/main/kotlin/application/CompareCommand.kt @@ -12,7 +12,12 @@ import kotlin.system.exitProcess @Command(name = "compare", mixinStandardHelpOptions = true, - description = ["Checks if two contracts are equivalent"]) + description = [ +""" +Checks if two contracts are equivalent. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ]) class CompareCommand : Callable { @Parameters(index = "0", description = ["Older contract file path"]) lateinit var olderContractFilePath: String diff --git a/application/src/main/kotlin/application/CompatibleCommand.kt b/application/src/main/kotlin/application/CompatibleCommand.kt index e5c32dd27..234e28546 100644 --- a/application/src/main/kotlin/application/CompatibleCommand.kt +++ b/application/src/main/kotlin/application/CompatibleCommand.kt @@ -180,7 +180,12 @@ class GitCompatibleCommand : Callable { @Command(name = "compatible", mixinStandardHelpOptions = true, - description = ["Checks if the newer contract is backward compatible with the older one"], + description = [ +""" +Checks if the newer contract is backward compatible with the older one +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ], subcommands = [ GitCompatibleCommand::class ]) internal class CompatibleCommand : Callable { override fun call() { diff --git a/application/src/main/kotlin/application/DifferenceCommand.kt b/application/src/main/kotlin/application/DifferenceCommand.kt index 1effdcc73..e6ce1ee61 100644 --- a/application/src/main/kotlin/application/DifferenceCommand.kt +++ b/application/src/main/kotlin/application/DifferenceCommand.kt @@ -11,7 +11,13 @@ import kotlin.system.exitProcess @Command(name = "similar", mixinStandardHelpOptions = true, - description = ["Show the difference between two contracts"]) + description = [ +""" +Show the difference between two contracts. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ] +) class DifferenceCommand : Callable { @Parameters(index = "0", description = ["Older contract file path"]) lateinit var olderContractFilePath: String diff --git a/application/src/main/kotlin/application/PushCommand.kt b/application/src/main/kotlin/application/PushCommand.kt index 6243370ae..46049033d 100644 --- a/application/src/main/kotlin/application/PushCommand.kt +++ b/application/src/main/kotlin/application/PushCommand.kt @@ -18,7 +18,16 @@ import kotlin.system.exitProcess private const val pipelineKeyInSpecmaticConfig = "pipeline" -@CommandLine.Command(name = "push", description = ["Check the new contract for backward compatibility with the specified version, then overwrite the old one with it."], mixinStandardHelpOptions = true) +@CommandLine.Command( + name = "push", + description = [ +""" +Check the new contract for backward compatibility with the specified version, then overwrite the old one with it. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ], + mixinStandardHelpOptions = true +) class PushCommand: Callable { override fun call() { val userHome = File(System.getProperty("user.home")) @@ -160,4 +169,4 @@ fun registerPipelineCredentials(manifestData: JSONObjectValue, contractPath: Str sourceGit.add() } } -} \ No newline at end of file +} diff --git a/application/src/main/kotlin/application/SpecmaticCommand.kt b/application/src/main/kotlin/application/SpecmaticCommand.kt index 170d188e1..d6591fe32 100644 --- a/application/src/main/kotlin/application/SpecmaticCommand.kt +++ b/application/src/main/kotlin/application/SpecmaticCommand.kt @@ -1,6 +1,6 @@ package application -import application.backwardCompatibility.BackwardCompatibilityCheckCommand +import application.backwardCompatibility.BackwardCompatibilityCheckCommandV2 import org.springframework.stereotype.Component import picocli.AutoComplete.GenerateCompletion import picocli.CommandLine.Command @@ -11,7 +11,30 @@ import java.util.concurrent.Callable name = "specmatic", mixinStandardHelpOptions = true, versionProvider = VersionProvider::class, - subcommands = [BackwardCompatibilityCheckCommand::class, BundleCommand::class, CompareCommand::class, CompatibleCommand::class, DifferenceCommand::class, GenerateCompletion::class, GraphCommand::class, MergeCommand::class, ToOpenAPICommand::class, ImportCommand::class, InstallCommand::class, ProxyCommand::class, PushCommand::class, ReDeclaredAPICommand::class, ExamplesCommand::class, SamplesCommand::class, StubCommand::class, SubscribeCommand::class, TestCommand::class, ValidateViaLogs::class, CentralContractRepoReportCommand::class] + subcommands = [ + BackwardCompatibilityCheckCommandV2::class, + BackwardCompatibilityCheckCommand::class, + BundleCommand::class, + CompareCommand::class, + CompatibleCommand::class, + DifferenceCommand::class, + GenerateCompletion::class, + GraphCommand::class, + MergeCommand::class, + ToOpenAPICommand::class, + ImportCommand::class, + InstallCommand::class, + ProxyCommand::class, + PushCommand::class, + ReDeclaredAPICommand::class, + ExamplesCommand::class, + SamplesCommand::class, + StubCommand::class, + SubscribeCommand::class, + TestCommand::class, + ValidateViaLogs::class, + CentralContractRepoReportCommand::class + ] ) class SpecmaticCommand : Callable { override fun call(): Int { diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 8b3c330b3..11241eeb7 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -16,7 +16,14 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { private val gitCommand: GitCommand = SystemGit() private val newLine = System.lineSeparator() - @Option(names = ["--base-branch"], description = ["Base branch to compare the changes against"], required = false) + @Option( + names = ["--base-branch"], + description = [ + "Base branch to compare the changes against", + "Default value is the local origin HEAD of the current branch" + ], + required = false + ) var baseBranch: String? = null @Option(names = ["--target-path"], description = ["Specification file or folder to run the check against"], required = false) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt new file mode 100644 index 000000000..4c59f7244 --- /dev/null +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt @@ -0,0 +1,112 @@ +package application.backwardCompatibility + +import io.specmatic.conversions.OpenApiSpecification +import io.specmatic.core.CONTRACT_EXTENSION +import io.specmatic.core.CONTRACT_EXTENSIONS +import io.specmatic.core.Feature +import io.specmatic.core.IFeature +import io.specmatic.core.Results +import io.specmatic.core.WSDL +import io.specmatic.core.testBackwardCompatibility +import io.specmatic.stub.isOpenAPI +import org.springframework.stereotype.Component +import picocli.CommandLine.Command +import java.io.File +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import kotlin.io.path.extension +import kotlin.io.path.pathString + +@Component +@Command( + name = "backward-compatibility-check", + mixinStandardHelpOptions = true, + description = ["Checks backward compatibility of OpenAPI specifications"] +) +class BackwardCompatibilityCheckCommandV2: BackwardCompatibilityCheckBaseCommand() { + + override fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results { + return testBackwardCompatibility(oldFeature as Feature, newFeature as Feature) + } + + override fun File.isValidSpec(): Boolean { + if (this.extension !in CONTRACT_EXTENSIONS) return false + return OpenApiSpecification.isParsable(this.path) + } + + override fun getFeatureFromSpecPath(path: String): Feature { + return OpenApiSpecification.fromFile(path).toFeature() + } + + override fun regexForMatchingReferred(schemaFileName: String) = schemaFileName + + override fun getSpecsOfChangedExternalisedExamples(filesChangedInCurrentBranch: Set): Set { + data class CollectedFiles( + val specifications: MutableSet = mutableSetOf(), + val examplesMissingSpecifications: MutableList = mutableListOf(), + val ignoredFiles: MutableList = mutableListOf() + ) + + val collectedFiles = filesChangedInCurrentBranch.fold(CollectedFiles()) { acc, filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + + if (examplesDir == null) { + acc.ignoredFiles.add(filePath) + } else { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + + if (specFiles.isNotEmpty()) { + acc.specifications.addAll(specFiles.map { it.toString() }) + } else { + acc.examplesMissingSpecifications.add(filePath) + } + } + acc + } + + val result = collectedFiles.specifications.toMutableSet() + + collectedFiles.examplesMissingSpecifications.forEach { filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + if (examplesDir != null) { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + if (specFiles.isNotEmpty()) { + result.addAll(specFiles.map { it.toString() }) + } else { + result.add("${strippedPath}.yaml") + } + } + } + + return result + } + + override fun areExamplesValid(feature: IFeature, which: String): Boolean { + feature as Feature + return try { + feature.validateExamplesOrException() + true + } catch (t: Throwable) { + println() + false + } + } + + override fun getUnusedExamples(feature: IFeature): Set { + feature as Feature + return feature.loadExternalisedExamplesAndListUnloadableExamples().second + } + + private fun findSpecFiles(path: Path): List { + val extensions = CONTRACT_EXTENSIONS + return extensions.map { path.resolveSibling(path.fileName.toString() + it) } + .filter { Files.exists(it) && (isOpenAPI(it.pathString) || it.extension in listOf(WSDL, CONTRACT_EXTENSION)) } + } +} \ No newline at end of file diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt similarity index 88% rename from application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt rename to application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt index 6873da05c..18fd33bf7 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt @@ -1,6 +1,6 @@ package application -import application.backwardCompatibility.BackwardCompatibilityCheckCommand +import application.backwardCompatibility.BackwardCompatibilityCheckCommandV2 import io.mockk.every import io.mockk.spyk import org.junit.jupiter.api.AfterEach @@ -9,18 +9,18 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.io.File -class BackwardCompatibilityCheckCommandTest { +class BackwardCompatibilityCheckCommandV2Test { @Test fun `getSpecsReferringTo returns empty set when input is empty`() { - val command = BackwardCompatibilityCheckCommand() + val command = BackwardCompatibilityCheckCommandV2() val result = command.getSpecsReferringTo(emptySet()) assertTrue(result.isEmpty()) } @Test fun `getSpecsReferringTo returns empty set when no files refer to changed schema files`() { - val command = spyk() + val command = spyk() every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("content1") }, File("file2.yaml").apply { writeText("content2") } @@ -31,7 +31,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `getSpecsReferringTo returns set of files that refer to changed schema files`() { - val command = spyk() + val command = spyk() every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("file3.yaml") }, File("file2.yaml").apply { writeText("file4.yaml") } @@ -42,7 +42,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `getSpecsReferringTo returns set of files which are referring to a changed schema that is one level down`() { - val command = spyk() + val command = spyk() every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { referTo("schema_file1.yaml") }, File("schema_file2.yaml").apply { referTo("schema_file1.yaml") }, // schema within a schema From 20f87436fb64f400a5f23f9321f50bc0e86c3050 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Mon, 7 Oct 2024 13:05:25 +0530 Subject: [PATCH 07/18] Update the import statement for exitWithMessage in BackwardCompatibilityCheckCommand --- .../application/BackwardCompatibilityCheckCommand.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt index fd14d55c8..2ceaf79d1 100644 --- a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt +++ b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt @@ -1,11 +1,17 @@ package application -import application.BackwardCompatibilityCheckCommand.CompatibilityResult.* +import application.BackwardCompatibilityCheckCommand.CompatibilityResult.FAILED +import application.BackwardCompatibilityCheckCommand.CompatibilityResult.PASSED import io.specmatic.conversions.OpenApiSpecification -import io.specmatic.core.* +import io.specmatic.core.CONTRACT_EXTENSION +import io.specmatic.core.CONTRACT_EXTENSIONS +import io.specmatic.core.Feature +import io.specmatic.core.WSDL import io.specmatic.core.git.GitCommand import io.specmatic.core.git.SystemGit import io.specmatic.core.log.logger +import io.specmatic.core.testBackwardCompatibility +import io.specmatic.core.utilities.exitWithMessage import io.specmatic.stub.isOpenAPI import org.springframework.stereotype.Component import picocli.CommandLine.Command From 1853f4fde385168219c16539452524c43f6fd0fb Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 10 Oct 2024 11:01:41 +0530 Subject: [PATCH 08/18] Refactor the logging logic of changed files --- .../BackwardCompatibilityCheckBaseCommand.kt | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 11241eeb7..c3319b00b 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -121,26 +121,23 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { filesReferringToChangedFiles: Set, specificationsOfChangedExternalisedExamples: Set ) { + logger.log("Checking backward compatibility of the following specs:$newLine") + changedFiles.printSummaryOfChangedSpecs("Specs that have changed") + filesReferringToChangedFiles.printSummaryOfChangedSpecs("Specs referring to the changed specs") + specificationsOfChangedExternalisedExamples + .printSummaryOfChangedSpecs("Specs whose externalised examples were changed") + logger.log("-".repeat(20)) + logger.log(newLine) + } - println("Checking backward compatibility of the following specs: $newLine") - println("${ONE_INDENT}Specs that have changed:") - changedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } - println() - - if(filesReferringToChangedFiles.isNotEmpty()) { - println("${ONE_INDENT}Specs referring to the changed specs - ") - filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } - println() - } - - if(specificationsOfChangedExternalisedExamples.isNotEmpty()) { - println("${ONE_INDENT}Specs whose externalised examples were changed - ") - filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } - println() + private fun Set.printSummaryOfChangedSpecs(message: String) { + if(this.isNotEmpty()) { + logger.log("${ONE_INDENT}- $message: ") + this.forEachIndexed { index, it -> + logger.log(it.prependIndent("$TWO_INDENTS${index.inc()}. ")) + } + logger.log(newLine) } - - println("-".repeat(20)) - println() } private fun runBackwardCompatibilityCheckFor(files: Set, baseBranch: String): CompatibilityReport { From 79cd2697086111f59ccb98cd9421775f52295cf4 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 10 Oct 2024 11:33:48 +0530 Subject: [PATCH 09/18] Add a shutdown hook in backward compatibility check command which will bring the repository back to the original state if the command is aborted in between --- .../BackwardCompatibilityCheckBaseCommand.kt | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index c3319b00b..e9c74d69e 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -15,6 +15,7 @@ import kotlin.system.exitProcess abstract class BackwardCompatibilityCheckBaseCommand : Callable { private val gitCommand: GitCommand = SystemGit() private val newLine = System.lineSeparator() + private var areLocalChangesStashed = false @Option( names = ["--base-branch"], @@ -42,6 +43,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { open fun getUnusedExamples(feature: IFeature): Set = emptySet() final override fun call() { + addShutdownHook() val filteredSpecs = getChangedSpecs(logSpecs = true) val result = try { runBackwardCompatibilityCheckFor( @@ -140,13 +142,16 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } } - private fun runBackwardCompatibilityCheckFor(files: Set, baseBranch: String): CompatibilityReport { + private fun getCurrentBranch(): String { val branchWithChanges = gitCommand.currentBranch() - val treeishWithChanges = if (branchWithChanges == HEAD) gitCommand.detachedHEAD() else branchWithChanges + return if (branchWithChanges == HEAD) gitCommand.detachedHEAD() else branchWithChanges + } + + private fun runBackwardCompatibilityCheckFor(files: Set, baseBranch: String): CompatibilityReport { + val treeishWithChanges = getCurrentBranch() try { val results = files.mapIndexed { index, specFilePath -> - var areLocalChangesStashed = false try { println("${index.inc()}. Running the check for $specFilePath:") @@ -179,7 +184,10 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { ) } finally { gitCommand.checkout(treeishWithChanges) - if (areLocalChangesStashed) gitCommand.stashPop() + if (areLocalChangesStashed) { + gitCommand.stashPop() + areLocalChangesStashed = false + } } } @@ -242,6 +250,15 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } } + private fun addShutdownHook() { + Runtime.getRuntime().addShutdownHook(object: Thread() { + override fun run() { + gitCommand.checkout(getCurrentBranch()) + if(areLocalChangesStashed) gitCommand.stashPop() + } + }) + } + companion object { private const val HEAD = "HEAD" private const val MARGIN_SPACE = " " From 0312a378616dd8cf5a6d1dcfe1e1cf89d3408c0c Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 10 Oct 2024 12:05:59 +0530 Subject: [PATCH 10/18] Improve logging of backward compatibility report --- .../BackwardCompatibilityCheckBaseCommand.kt | 101 ++++++++++-------- 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index e9c74d69e..55520b6ec 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -57,8 +57,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { exitProcess(1) } - println() - println(result.report) + logger.log(result.report) exitProcess(result.exitCode) } @@ -153,7 +152,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { try { val results = files.mapIndexed { index, specFilePath -> try { - println("${index.inc()}. Running the check for $specFilePath:") + logger.log("${index.inc()}. Running the check for $specFilePath:$newLine") // newer => the file with changes on the branch val newer = getFeatureFromSpecPath(specFilePath) @@ -165,7 +164,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { baseBranch ) if (olderFile == null) { - println("$specFilePath is a new file.$newLine") + logger.log("$ONE_INDENT$specFilePath is a new file.$newLine") return@mapIndexed CompatibilityResult.PASSED } @@ -205,56 +204,70 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { newer: IFeature, unusedExamples: Set ): CompatibilityResult { - if (backwardCompatibilityResult.success()) { - println( - "$newLine The spec $specFilePath is backward compatible with the corresponding spec from ${baseBranch()}$newLine".prependIndent( - MARGIN_SPACE - ) + if(backwardCompatibilityResult.success().not()) { + logger.log("_".repeat(40).prependIndent(ONE_INDENT)) + logger.log("The Incompatibility Report:$newLine".prependIndent(ONE_INDENT)) + logger.log(backwardCompatibilityResult.report().prependIndent(TWO_INDENTS)) + logVerdictFor( + specFilePath, + "(INCOMPATIBLE) The changes to the spec are NOT backward compatible with the corresponding spec from ${baseBranch()}".prependIndent(ONE_INDENT) ) - println() - var errorsFound = false + return CompatibilityResult.FAILED + } - if(!areExamplesValid(newer, "newer")) { - println( - "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - errorsFound = true - } + val errorsFound = printExampleValiditySummaryAndReturnResult(newer, unusedExamples, specFilePath) - if(unusedExamples.isNotEmpty()) { - println( - "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - errorsFound = true - } - - return if(errorsFound) CompatibilityResult.FAILED - else CompatibilityResult.PASSED + val message = if(errorsFound) { + "(INCOMPATIBLE) The spec is backward compatible but the examples are NOT backward compatible or are INVALID." } else { - println("$newLine ${backwardCompatibilityResult.report().prependIndent( - MARGIN_SPACE - )}") - println( - "$newLine *** The changes to the spec $specFilePath are NOT backward compatible with the corresponding spec from ${baseBranch()}***$newLine".prependIndent( - MARGIN_SPACE - ) - ) - println() - return CompatibilityResult.FAILED + "(COMPATIBLE) The spec is backward compatible with the corresponding spec from ${baseBranch()}" + } + logVerdictFor(specFilePath, message.prependIndent(ONE_INDENT)) + + return if (errorsFound) CompatibilityResult.FAILED + else CompatibilityResult.PASSED + } + + private fun logVerdictFor(specFilePath: String, message: String) { + logger.log(newLine) + logger.log("-".repeat(20).prependIndent(ONE_INDENT)) + logger.log("Verdict for spec $specFilePath:".prependIndent(ONE_INDENT)) + logger.log("$ONE_INDENT$message") + logger.log("-".repeat(20).prependIndent(ONE_INDENT)) + logger.log(newLine) + } + + private fun printExampleValiditySummaryAndReturnResult( + newer: IFeature, + unusedExamples: Set, + specFilePath: String + ): Boolean { + var errorsFound = false + val areExamplesInvalid = areExamplesValid(newer, "newer").not() + + if(areExamplesInvalid || unusedExamples.isNotEmpty()) { + logger.log("_".repeat(40).prependIndent(ONE_INDENT)) + logger.log("The Examples Validity Summary:$newLine".prependIndent(ONE_INDENT)) + } + if (areExamplesInvalid) { + logger.log("Examples in $specFilePath are not valid.$newLine".prependIndent(TWO_INDENTS)) + errorsFound = true } + + if (unusedExamples.isNotEmpty()) { + logger.log("Some examples for $specFilePath could not be loaded.$newLine".prependIndent(TWO_INDENTS)) + errorsFound = true + } + return errorsFound } private fun addShutdownHook() { Runtime.getRuntime().addShutdownHook(object: Thread() { override fun run() { - gitCommand.checkout(getCurrentBranch()) - if(areLocalChangesStashed) gitCommand.stashPop() + runCatching { + gitCommand.checkout(getCurrentBranch()) + if(areLocalChangesStashed) gitCommand.stashPop() + } } }) } From 5943086e0070faa5893b306a9925b910415b19db Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 10 Oct 2024 15:22:23 +0530 Subject: [PATCH 11/18] Add ability to disable and enable info logging in LogStrategy to avoid unnecessary logs in the backward compatibility check result logs --- .../BackwardCompatibilityCheckBaseCommand.kt | 8 ++++---- .../BackwardCompatibilityCheckCommandV2.kt | 6 +++++- core/src/main/kotlin/io/specmatic/core/Scenario.kt | 5 ++--- .../src/main/kotlin/io/specmatic/core/log/LogStrategy.kt | 8 +++++++- core/src/main/kotlin/io/specmatic/core/log/NonVerbose.kt | 9 ++++++--- core/src/main/kotlin/io/specmatic/core/log/Verbose.kt | 9 ++++++--- .../io/specmatic/conversions/OpenApiSpecificationTest.kt | 6 ++++++ 7 files changed, 36 insertions(+), 15 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 55520b6ec..92e5b355e 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -152,7 +152,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { try { val results = files.mapIndexed { index, specFilePath -> try { - logger.log("${index.inc()}. Running the check for $specFilePath:$newLine") + logger.log("${index.inc()}. Running the check for $specFilePath:") // newer => the file with changes on the branch val newer = getFeatureFromSpecPath(specFilePath) @@ -222,14 +222,14 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } else { "(COMPATIBLE) The spec is backward compatible with the corresponding spec from ${baseBranch()}" } - logVerdictFor(specFilePath, message.prependIndent(ONE_INDENT)) + logVerdictFor(specFilePath, message.prependIndent(ONE_INDENT), startWithNewLine = errorsFound) return if (errorsFound) CompatibilityResult.FAILED else CompatibilityResult.PASSED } - private fun logVerdictFor(specFilePath: String, message: String) { - logger.log(newLine) + private fun logVerdictFor(specFilePath: String, message: String, startWithNewLine: Boolean = true) { + if (startWithNewLine) logger.log(newLine) logger.log("-".repeat(20).prependIndent(ONE_INDENT)) logger.log("Verdict for spec $specFilePath:".prependIndent(ONE_INDENT)) logger.log("$ONE_INDENT$message") diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt index 4c59f7244..c942e898b 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt @@ -7,6 +7,7 @@ import io.specmatic.core.Feature import io.specmatic.core.IFeature import io.specmatic.core.Results import io.specmatic.core.WSDL +import io.specmatic.core.log.logger import io.specmatic.core.testBackwardCompatibility import io.specmatic.stub.isOpenAPI import org.springframework.stereotype.Component @@ -36,7 +37,10 @@ class BackwardCompatibilityCheckCommandV2: BackwardCompatibilityCheckBaseCommand } override fun getFeatureFromSpecPath(path: String): Feature { - return OpenApiSpecification.fromFile(path).toFeature() + logger.disableInfoLogging() + return OpenApiSpecification.fromFile(path).toFeature().also { + logger.enableInfoLogging() + } } override fun regexForMatchingReferred(schemaFileName: String) = schemaFileName diff --git a/core/src/main/kotlin/io/specmatic/core/Scenario.kt b/core/src/main/kotlin/io/specmatic/core/Scenario.kt index 26f069c9a..04aeaed33 100644 --- a/core/src/main/kotlin/io/specmatic/core/Scenario.kt +++ b/core/src/main/kotlin/io/specmatic/core/Scenario.kt @@ -399,9 +399,8 @@ data class Scenario( "Error loading example named ${row.name} for ${this.apiDescription.trim()}" } - listOf(title).plus(errors).joinToString("${System.lineSeparator()}${System.lineSeparator()}") - .also { message -> - logger.log(message) + listOf(title).plus(errors).joinToString("${System.lineSeparator()}${System.lineSeparator()}").also { message -> + logger.logError(Exception(message)) logger.newLine() } diff --git a/core/src/main/kotlin/io/specmatic/core/log/LogStrategy.kt b/core/src/main/kotlin/io/specmatic/core/log/LogStrategy.kt index 3d58daa2b..b6ab01f6e 100644 --- a/core/src/main/kotlin/io/specmatic/core/log/LogStrategy.kt +++ b/core/src/main/kotlin/io/specmatic/core/log/LogStrategy.kt @@ -2,9 +2,9 @@ package io.specmatic.core.log interface LogStrategy { val printer: CompositePrinter + var infoLoggingEnabled: Boolean fun keepReady(msg: LogMessage) - fun exceptionString(e: Throwable, msg: String? = null): String fun ofTheException(e: Throwable, msg: String? = null): LogMessage fun log(e: Throwable, msg: String? = null) @@ -15,4 +15,10 @@ interface LogStrategy { fun debug(msg: String): String fun debug(msg: LogMessage) fun debug(e: Throwable, msg: String? = null) + fun disableInfoLogging() { + infoLoggingEnabled = false + } + fun enableInfoLogging() { + infoLoggingEnabled = true + } } \ No newline at end of file diff --git a/core/src/main/kotlin/io/specmatic/core/log/NonVerbose.kt b/core/src/main/kotlin/io/specmatic/core/log/NonVerbose.kt index 7b2b4effb..94e0eec7b 100644 --- a/core/src/main/kotlin/io/specmatic/core/log/NonVerbose.kt +++ b/core/src/main/kotlin/io/specmatic/core/log/NonVerbose.kt @@ -2,7 +2,10 @@ package io.specmatic.core.log import io.specmatic.core.utilities.exceptionCauseMessage -class NonVerbose(override val printer: CompositePrinter) : LogStrategy { +class NonVerbose( + override val printer: CompositePrinter, + override var infoLoggingEnabled: Boolean = true +) : LogStrategy { private val readyMessage = ReadyMessage() override fun keepReady(msg: LogMessage) { @@ -30,11 +33,11 @@ class NonVerbose(override val printer: CompositePrinter) : LogStrategy { } override fun log(msg: String) { - log(StringLog(msg)) + if (infoLoggingEnabled) log(StringLog(msg)) } override fun log(msg: LogMessage) { - print(msg) + if (infoLoggingEnabled) print(msg) } override fun logError(e: Throwable) { diff --git a/core/src/main/kotlin/io/specmatic/core/log/Verbose.kt b/core/src/main/kotlin/io/specmatic/core/log/Verbose.kt index e2717ea95..a5394c4a4 100644 --- a/core/src/main/kotlin/io/specmatic/core/log/Verbose.kt +++ b/core/src/main/kotlin/io/specmatic/core/log/Verbose.kt @@ -2,7 +2,10 @@ package io.specmatic.core.log import io.specmatic.core.utilities.exceptionCauseMessage -class Verbose(override val printer: CompositePrinter = CompositePrinter()) : LogStrategy { +class Verbose( + override val printer: CompositePrinter = CompositePrinter(), + override var infoLoggingEnabled: Boolean = true +) : LogStrategy { private val readyMessage = ReadyMessage() override fun keepReady(msg: LogMessage) { @@ -32,11 +35,11 @@ class Verbose(override val printer: CompositePrinter = CompositePrinter()) : Log } override fun log(msg: String) { - log(StringLog(msg)) + if (infoLoggingEnabled) log(StringLog(msg)) } override fun log(msg: LogMessage) { - print(msg) + if (infoLoggingEnabled) print(msg) } override fun logError(e: Throwable) { diff --git a/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt b/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt index 6a4ad5614..fe6dfef9b 100644 --- a/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt +++ b/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt @@ -6232,6 +6232,9 @@ paths: val messages = mutableListOf() override val printer: CompositePrinter get() = TODO("Not yet implemented") + override var infoLoggingEnabled: Boolean + get() = true + set(value) {} override fun keepReady(msg: LogMessage) { TODO("Not yet implemented") @@ -6319,6 +6322,9 @@ paths: val messages = mutableListOf() override val printer: CompositePrinter get() = TODO("Not yet implemented") + override var infoLoggingEnabled: Boolean + get() = true + set(value) {} override fun keepReady(msg: LogMessage) { TODO("Not yet implemented") From 8b6a238e169e2854a8623fbb6bd4d871559ed2ec Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Thu, 10 Oct 2024 17:27:06 +0530 Subject: [PATCH 12/18] Change scope getSpecsReferringTo method from internal to open to enable override in sub-classes of BackwardCompatibilityCheckBaseCommand --- .../BackwardCompatibilityCheckBaseCommand.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 92e5b355e..9a6495303 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -34,11 +34,11 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { abstract fun File.isValidSpec(): Boolean abstract fun getFeatureFromSpecPath(path: String): IFeature - abstract fun regexForMatchingReferred(schemaFileName: String): String abstract fun getSpecsOfChangedExternalisedExamples( filesChangedInCurrentBranch: Set ): Set + open fun regexForMatchingReferred(schemaFileName: String): String = "" open fun areExamplesValid(feature: IFeature, which: String): Boolean = true open fun getUnusedExamples(feature: IFeature): Set = emptySet() @@ -92,7 +92,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { } } - internal fun getSpecsReferringTo(schemaFiles: Set): Set { + open fun getSpecsReferringTo(schemaFiles: Set): Set { if (schemaFiles.isEmpty()) return emptySet() val inputFileNames = schemaFiles.map { File(it).name } From 7441f11f0211aaf5b8782c231d640b016d981cae Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Fri, 11 Oct 2024 17:58:14 +0530 Subject: [PATCH 13/18] Make exitWithMessage method append and prepend newLine to the message --- .../kotlin/application/BackwardCompatibilityCheckCommand.kt | 5 ++++- .../BackwardCompatibilityCheckBaseCommand.kt | 2 +- .../src/main/kotlin/io/specmatic/core/utilities/Utilities.kt | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt index 2ceaf79d1..00e464df3 100644 --- a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt +++ b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt @@ -53,7 +53,10 @@ class BackwardCompatibilityCheckCommand( override fun call() { val filesChangedInCurrentBranch: Set = getOpenAPISpecFilesChangedInCurrentBranch() - if (filesChangedInCurrentBranch.isEmpty()) exitWithMessage("${newLine}No OpenAPI spec files were changed, skipping the check.$newLine") + if (filesChangedInCurrentBranch.isEmpty()) { + logger.log("${newLine}No OpenAPI spec files were changed, skipping the check.$newLine") + exitProcess(0) + } val filesReferringToChangedSchemaFiles = filesReferringToChangedSchemaFiles(filesChangedInCurrentBranch) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 9a6495303..7adfc2def 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -88,7 +88,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { ).filter { File(it).exists() && File(it).isValidSpec() }.toSet().also { - if(it.isEmpty()) exitWithMessage("${newLine}No specs were changed, skipping the check.$newLine") + if(it.isEmpty()) exitWithMessage("No specs were changed, skipping the check.") } } diff --git a/core/src/main/kotlin/io/specmatic/core/utilities/Utilities.kt b/core/src/main/kotlin/io/specmatic/core/utilities/Utilities.kt index 91d21bfef..12e8cd752 100644 --- a/core/src/main/kotlin/io/specmatic/core/utilities/Utilities.kt +++ b/core/src/main/kotlin/io/specmatic/core/utilities/Utilities.kt @@ -37,7 +37,8 @@ import javax.xml.transform.stream.StreamResult import kotlin.system.exitProcess fun exitWithMessage(message: String): Nothing { - logger.log(message) + val newLine = System.lineSeparator() + logger.log("$newLine$message$newLine") exitProcess(1) } From a9a419097b136303faf264416f68c2d92f97392a Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Fri, 11 Oct 2024 18:02:23 +0530 Subject: [PATCH 14/18] Rename getFileInTheBaseBranch to getFileInBranch --- .../application/BackwardCompatibilityCheckCommand.kt | 3 +-- .../BackwardCompatibilityCheckBaseCommand.kt | 2 +- .../test/kotlin/application/CompatibleCommandKtTest.kt | 8 ++++---- core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt | 2 +- core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt index 00e464df3..f6476e0ea 100644 --- a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt +++ b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt @@ -11,7 +11,6 @@ import io.specmatic.core.git.GitCommand import io.specmatic.core.git.SystemGit import io.specmatic.core.log.logger import io.specmatic.core.testBackwardCompatibility -import io.specmatic.core.utilities.exitWithMessage import io.specmatic.stub.isOpenAPI import org.springframework.stereotype.Component import picocli.CommandLine.Command @@ -160,7 +159,7 @@ class BackwardCompatibilityCheckCommand( // newer => the file with changes on the branch val (newer, unusedExamples) = OpenApiSpecification.fromFile(specFilePath).toFeature().loadExternalisedExamplesAndListUnloadableExamples() - val olderFile = gitCommand.getFileInTheBaseBranch( + val olderFile = gitCommand.getFileInBranch( specFilePath, treeishWithChanges, gitCommand.defaultBranch() diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 7adfc2def..52dc9b9e2 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -158,7 +158,7 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { val newer = getFeatureFromSpecPath(specFilePath) val unusedExamples = getUnusedExamples(newer) - val olderFile = gitCommand.getFileInTheBaseBranch( + val olderFile = gitCommand.getFileInBranch( specFilePath, treeishWithChanges, baseBranch diff --git a/application/src/test/kotlin/application/CompatibleCommandKtTest.kt b/application/src/test/kotlin/application/CompatibleCommandKtTest.kt index 1b4d9682e..99c0acc54 100644 --- a/application/src/test/kotlin/application/CompatibleCommandKtTest.kt +++ b/application/src/test/kotlin/application/CompatibleCommandKtTest.kt @@ -52,7 +52,7 @@ internal class CompatibleCommandKtTest { } override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() - override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null + override fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") @@ -82,7 +82,7 @@ internal class CompatibleCommandKtTest { override fun fileIsInGitDir(newerContractPath: String): Boolean = true override val workingDirectory: String get() = "" - override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null + override fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() @@ -122,7 +122,7 @@ internal class CompatibleCommandKtTest { override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() override val workingDirectory: String get() = "" - override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null + override fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") @@ -159,7 +159,7 @@ internal class CompatibleCommandKtTest { override fun getFilesChangedInCurrentBranch(baseBranch: String) = emptyList() override val workingDirectory: String get() = "" - override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String) = null + override fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String) = null override fun relativeGitPath(newerContractPath: String): Pair { assertThat(newerContractPath).isEqualTo("/Users/fakeuser/newer.$CONTRACT_EXTENSION") diff --git a/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt b/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt index 48d28ca44..7ca8bc827 100644 --- a/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt +++ b/core/src/main/kotlin/io/specmatic/core/git/GitCommand.kt @@ -33,7 +33,7 @@ interface GitCommand { fun getRemoteUrl(name: String = "origin"): String fun checkIgnore(path: String): String fun getFilesChangedInCurrentBranch(baseBranch: String): List - fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String): File? + fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String): File? fun currentRemoteBranch(): String fun currentBranch(): String { return "" diff --git a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt index 839a24254..916ae2c70 100644 --- a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt +++ b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt @@ -102,7 +102,7 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: return (committedLocalChanges + uncommittedChanges).distinct() } - override fun getFileInTheBaseBranch(fileName: String, currentBranch: String, baseBranch: String): File? { + override fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String): File? { try { if(baseBranch != currentBranch) checkout(baseBranch) From c1e4e2bfbd76f944ec7266037b926385eeea050a Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Fri, 11 Oct 2024 18:12:45 +0530 Subject: [PATCH 15/18] Staged files should be considered during the backward compatibility check --- .../main/kotlin/io/specmatic/core/git/SystemGit.kt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt index 916ae2c70..b8b351b96 100644 --- a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt +++ b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt @@ -91,15 +91,14 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: } override fun getFilesChangedInCurrentBranch(baseBranch: String): List { - val committedLocalChanges = execute(Configuration.gitCommand, "diff", baseBranch, "HEAD", "--name-only") + val committedLocalChanges = execute(Configuration.gitCommand, "diff", baseBranch, "HEAD", "--name-status") .split(System.lineSeparator()) - .filter { it.isNotBlank() } - - val uncommittedChanges = execute(Configuration.gitCommand, "diff", "--name-only") + val uncommittedChanges = execute(Configuration.gitCommand, "diff", "HEAD", "--name-status") .split(System.lineSeparator()) - .filter { it.isNotBlank() } - return (committedLocalChanges + uncommittedChanges).distinct() + return (committedLocalChanges + uncommittedChanges).map { + it.split("\t").last() + }.distinct() } override fun getFileInBranch(fileName: String, currentBranch: String, baseBranch: String): File? { From 3aa0152634bcd87f730c6b3c6c9944796b20493f Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Mon, 14 Oct 2024 10:27:35 +0530 Subject: [PATCH 16/18] Update the description for the argument --target-path in backward-compatibility-check command --- .../BackwardCompatibilityCheckBaseCommand.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 52dc9b9e2..37980d827 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -27,7 +27,11 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { ) var baseBranch: String? = null - @Option(names = ["--target-path"], description = ["Specification file or folder to run the check against"], required = false) + @Option( + names = ["--target-path"], + description = ["Specify the file or directory to limit the backward compatibility check scope. If omitted, all changed files will be checked."], + required = false + ) var targetPath: String = "" abstract fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results From bb89eba547d90edae8b926df4cba63fc2a882ec3 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Mon, 14 Oct 2024 14:34:17 +0530 Subject: [PATCH 17/18] Add tests for getFilesChangedInCurrentBranch function --- ...BackwardCompatibilityCheckCommandV2Test.kt | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt index 18fd33bf7..2f70baa70 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt @@ -3,13 +3,46 @@ package application import application.backwardCompatibility.BackwardCompatibilityCheckCommandV2 import io.mockk.every import io.mockk.spyk +import io.specmatic.core.git.SystemGit import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import java.io.File +import java.nio.file.Files class BackwardCompatibilityCheckCommandV2Test { + private lateinit var tempDir: File + private lateinit var remoteDir: File + + @BeforeEach + fun setup() { + tempDir = Files.createTempDirectory("git-local").toFile() + tempDir.deleteOnExit() + + remoteDir = Files.createTempDirectory("git-remote").toFile() + remoteDir.deleteOnExit() + + ProcessBuilder("git", "init", "--bare") + .directory(remoteDir) + .inheritIO() + .start() + .waitFor() + + ProcessBuilder("git", "init") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + + ProcessBuilder("git", "remote", "add", "origin", remoteDir.absolutePath) + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + } @Test fun `getSpecsReferringTo returns empty set when input is empty`() { @@ -52,11 +85,85 @@ class BackwardCompatibilityCheckCommandV2Test { assertEquals(setOf("file1.yaml", "file2.yaml"), result) } + @Nested + inner class SystemGitTestsSpecificToBackwardCompatibility { + @Test + fun `getFilesChangedInCurrentBranch returns the uncommitted, unstaged changed file`() { + File(tempDir, "file1.txt").writeText("File 1 content") + ProcessBuilder("git", "add", "file1.txt") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + ProcessBuilder("git", "commit", "-m", "Add file1") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + // Push the committed changes to the remote repository + ProcessBuilder("git", "push", "origin", "main") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + + + val uncommittedFile = File(tempDir, "file1.txt") + uncommittedFile.writeText("File 1 changed content") + + val gitCommand = SystemGit(tempDir.absolutePath) + val result = gitCommand.getFilesChangedInCurrentBranch( + gitCommand.currentRemoteBranch() + ) + + assert(result.contains("file1.txt")) + } + + @Test + fun `getFilesChangedInCurrentBranch returns the uncommitted, staged changed file`() { + File(tempDir, "file1.txt").writeText("File 1 content") + ProcessBuilder("git", "add", "file1.txt") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + ProcessBuilder("git", "commit", "-m", "Add file1") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + // Push the committed changes to the remote repository + ProcessBuilder("git", "push", "origin", "main") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + + + val uncommittedFile = File(tempDir, "file1.txt") + uncommittedFile.writeText("File 1 changed content") + ProcessBuilder("git", "add", "file1.txt") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + + val gitCommand = SystemGit(tempDir.absolutePath) + val result = gitCommand.getFilesChangedInCurrentBranch( + gitCommand.currentRemoteBranch() + ) + + assert(result.contains("file1.txt")) + } + } + @AfterEach fun `cleanup files`() { listOf("file1.yaml", "file2.yaml", "file3.yaml", "file4.yaml", "schema_file1.yaml", "schema_file2.yaml").forEach { File(it).delete() } + tempDir.deleteRecursively() + remoteDir.deleteRecursively() } private fun File.referTo(schemaFileName: String) { From 444af8d7ea18c17bdf720645282d3bb4d966918c Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Mon, 14 Oct 2024 17:50:30 +0530 Subject: [PATCH 18/18] Fix the failing test around git command --- .../BackwardCompatibilityCheckCommandV2Test.kt | 12 ++++++++++++ .../main/kotlin/io/specmatic/core/git/SystemGit.kt | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt index 2f70baa70..401b504d4 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt @@ -37,6 +37,18 @@ class BackwardCompatibilityCheckCommandV2Test { .start() .waitFor() + ProcessBuilder("git", "config", "--local", "user.name", "developer") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + + ProcessBuilder("git", "config", "--local", "user.email", "developer@example.com") + .directory(tempDir) + .inheritIO() + .start() + .waitFor() + ProcessBuilder("git", "remote", "add", "origin", remoteDir.absolutePath) .directory(tempDir) .inheritIO() diff --git a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt index b8b351b96..4b46ced05 100644 --- a/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt +++ b/core/src/main/kotlin/io/specmatic/core/git/SystemGit.kt @@ -92,9 +92,9 @@ class SystemGit(override val workingDirectory: String = ".", private val prefix: override fun getFilesChangedInCurrentBranch(baseBranch: String): List { val committedLocalChanges = execute(Configuration.gitCommand, "diff", baseBranch, "HEAD", "--name-status") - .split(System.lineSeparator()) + .split("\n") val uncommittedChanges = execute(Configuration.gitCommand, "diff", "HEAD", "--name-status") - .split(System.lineSeparator()) + .split("\n") return (committedLocalChanges + uncommittedChanges).map { it.split("\t").last()