From fffa0638031c6601995fa51901a612c581fc1aa6 Mon Sep 17 00:00:00 2001 From: Stefano Franz Date: Thu, 16 Aug 2018 15:44:40 +0100 Subject: [PATCH] Cli backwards compatibility testing (#3733) * first pass at infrastructure around cli compatibility * add example unit test * inspect enum types * add a basic unit test to verify behaviour of the cli checker * revert root build.gradle --- .idea/compiler.xml | 2 + settings.gradle | 3 +- testing/test-cli/build.gradle | 17 ++ .../testing/CliBackwardsCompatibleTest.kt | 19 ++ .../testing/GenerateCommandLineCompat.kt | 188 ++++++++++++++++++ .../CommandLineCompatibilityCheckerTest.kt | 106 ++++++++++ .../resources/net.corda.testing.Dummy.yml | 77 +++++++ 7 files changed, 411 insertions(+), 1 deletion(-) create mode 100644 testing/test-cli/build.gradle create mode 100644 testing/test-cli/src/main/kotlin/net/corda/testing/CliBackwardsCompatibleTest.kt create mode 100644 testing/test-cli/src/main/kotlin/net/corda/testing/GenerateCommandLineCompat.kt create mode 100644 testing/test-cli/src/test/kotlin/net/corda/testing/CommandLineCompatibilityCheckerTest.kt create mode 100644 testing/test-cli/src/test/resources/net.corda.testing.Dummy.yml diff --git a/.idea/compiler.xml b/.idea/compiler.xml index bc63206a42..1c4c966a8b 100644 --- a/.idea/compiler.xml +++ b/.idea/compiler.xml @@ -185,6 +185,8 @@ + + diff --git a/settings.gradle b/settings.gradle index fd98fba0ab..8433c9477e 100644 --- a/settings.gradle +++ b/settings.gradle @@ -24,11 +24,12 @@ include 'experimental:kryo-hook' include 'experimental:corda-utils' include 'jdk8u-deterministic' include 'test-common' +include 'test-cli' include 'test-utils' include 'smoke-test-utils' include 'node-driver' // Avoid making 'testing' a project, and allow build.gradle files to refer to these by their simple names: -['test-common', 'test-utils', 'smoke-test-utils', 'node-driver'].each { +['test-common', 'test-utils', 'test-cli', 'smoke-test-utils', 'node-driver'].each { project(":$it").projectDir = new File("$settingsDir/testing/$it") } include 'tools:explorer' diff --git a/testing/test-cli/build.gradle b/testing/test-cli/build.gradle new file mode 100644 index 0000000000..462499bf64 --- /dev/null +++ b/testing/test-cli/build.gradle @@ -0,0 +1,17 @@ +apply plugin: 'java' +apply plugin: 'kotlin' + +dependencies { + compile group: 'info.picocli', name: 'picocli', version: '3.0.1' + compile "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version" + compile group: "com.fasterxml.jackson.dataformat", name: "jackson-dataformat-yaml", version: "2.9.0" + compile group: "com.fasterxml.jackson.core", name: "jackson-databind", version: "2.9.0" + compile "com.fasterxml.jackson.module:jackson-module-kotlin:2.9.+" + compile "junit:junit:$junit_version" + +} +compileKotlin { + kotlinOptions { + languageVersion = "1.2" + } +} \ No newline at end of file diff --git a/testing/test-cli/src/main/kotlin/net/corda/testing/CliBackwardsCompatibleTest.kt b/testing/test-cli/src/main/kotlin/net/corda/testing/CliBackwardsCompatibleTest.kt new file mode 100644 index 0000000000..53b4b1b032 --- /dev/null +++ b/testing/test-cli/src/main/kotlin/net/corda/testing/CliBackwardsCompatibleTest.kt @@ -0,0 +1,19 @@ +package net.corda.testing + +import junit.framework.AssertionFailedError + +open class CliBackwardsCompatibleTest { + + + fun checkBackwardsCompatibility(clazz: Class<*>) { + val checker = CommandLineCompatibilityChecker() + val checkResults = checker.checkCommandLineIsBackwardsCompatible(clazz) + + if (checkResults.isNotEmpty()) { + val exceptionMessage= checkResults.map { it.message }.joinToString(separator = "\n") + throw AssertionFailedError("Command line is not backwards compatible:\n$exceptionMessage") + } + } + + +} \ No newline at end of file diff --git a/testing/test-cli/src/main/kotlin/net/corda/testing/GenerateCommandLineCompat.kt b/testing/test-cli/src/main/kotlin/net/corda/testing/GenerateCommandLineCompat.kt new file mode 100644 index 0000000000..e0141767c5 --- /dev/null +++ b/testing/test-cli/src/main/kotlin/net/corda/testing/GenerateCommandLineCompat.kt @@ -0,0 +1,188 @@ +package net.corda.testing + +import com.fasterxml.jackson.core.type.TypeReference +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory +import com.fasterxml.jackson.module.kotlin.registerKotlinModule +import picocli.CommandLine +import java.io.InputStream +import java.util.* +import kotlin.collections.ArrayList + + +class CommandLineCompatibilityChecker { + + fun topoSort(commandLine: CommandLine): List { + val toVisit = Stack() + toVisit.push(commandLine) + val sorted: MutableList = ArrayList(); + while (toVisit.isNotEmpty()) { + val visiting = toVisit.pop() + sorted.add(visiting) + visiting.subcommands.values.sortedBy { it.commandName }.forEach { + toVisit.push(it) + } + } + return buildDescriptors(sorted) + } + + private fun buildDescriptors(result: MutableList): List { + return result.map { ::parseToDescription.invoke(it) } + } + + internal fun parseToDescription(it: CommandLine): CommandDescription { + val commandSpec = it.commandSpec + val options = commandSpec.options().filterNot { it.usageHelp() || it.versionHelp() } + .map { hit -> hit.names().map { it to hit } } + .flatMap { it } + .sortedBy { it.first } + .map { + val type = it.second.type() + ParameterDescription(it.first, type.componentType?.canonicalName + ?: type.canonicalName, it.second.required(), isMultiple(type), determineAcceptableOptions(type)) + } + + val positionals = commandSpec.positionalParameters().sortedBy { it.index() }.map { + val type = it.type() + ParameterDescription(it.index().toString(), type.componentType?.canonicalName + ?: type.canonicalName, it.required(), isMultiple(type)) + } + return CommandDescription(it.commandName, positionals, options) + } + + private fun determineAcceptableOptions(type: Class<*>?): List { + return if (type?.isEnum == true) { + type.enumConstants.map { it.toString() } + } else { + emptyList() + } + } + + fun isMultiple(clazz: Class<*>): Boolean { + return Iterable::class.java.isAssignableFrom(clazz) || Array::class.java.isAssignableFrom(clazz) + } + + fun printCommandDescription(commandLine: CommandLine) { + val objectMapper = ObjectMapper(YAMLFactory()).registerKotlinModule() + val results = topoSort(commandLine) + println(objectMapper.writeValueAsString(results)) + } + + fun readCommandDescription(inputStream: InputStream): List { + val objectMapper = ObjectMapper(YAMLFactory()).registerKotlinModule() + return objectMapper.readValue>(inputStream, object : TypeReference>() {}); + } + + fun checkAllCommandsArePresent(old: List, new: List): List { + val oldSet = old.map { it.commandName }.toSet() + val newSet = new.map { it.commandName }.toSet() + val newIsSuperSetOfOld = newSet.containsAll(oldSet) + return if (!newIsSuperSetOfOld) { + oldSet.filterNot { newSet.contains(it) }.map { + CommandsChangedError("SubCommand: $it has been removed from the CLI") + } + } else { + emptyList() + } + } + + fun checkAllOptionsArePresent(old: CommandDescription, new: CommandDescription): List { + if (old.commandName != new.commandName) { + throw IllegalArgumentException("Commands must match (${old.commandName} != ${new.commandName})") + } + val oldSet = old.params.map { it.parameterName }.toSet() + val newSet = new.params.map { it.parameterName }.toSet() + + val newIsSuperSetOfOld = newSet.containsAll(oldSet) + + return if (!newIsSuperSetOfOld) { + oldSet.filterNot { newSet.contains(it) }.map { + OptionsChangedError("Parameter: $it has been removed from subcommand: ${old.commandName}") + } + } else { + emptyList() + } + } + + fun checkAllPositionalCharactersArePresent(old: CommandDescription, new: CommandDescription): List { + if (old.commandName != new.commandName) { + throw IllegalArgumentException("Commands must match (${old.commandName} != ${new.commandName})") + } + val oldSet = old.positionalParams.sortedBy { it.parameterName }.toSet() + val newSet = new.positionalParams.sortedBy { it.parameterName}.toSet() + val newIsSuperSetOfOld = newSet.containsAll(oldSet) + return if (!newIsSuperSetOfOld) { + oldSet.filterNot { newSet.contains(it) }.map { + PositionalArgumentsChangedError("Positional Parameter [ ${it.parameterName} ] has been removed from subcommand: ${old.commandName}") + } + } else { + emptyList() + } + } + + fun checkAllParamsAreOfTheSameType(old: CommandDescription, new: CommandDescription): List { + + val oldMap = old.params.map { it.parameterName to it.parameterType }.toMap() + val newMap = new.params.map { it.parameterName to it.parameterType }.toMap() + + val changedTypes = oldMap.filter { newMap[it.key] != null && newMap[it.key] != it.value }.map { + TypesChangedError("Parameter [ ${it.key} has changed from type: ${it.value} to ${newMap[it.key]}") + } + val oldAcceptableTypes = old.params.map { it.parameterName to it.acceptableValues }.toMap() + val newAcceptableTypes = new.params.map { it.parameterName to it.acceptableValues }.toMap() + val potentiallyChanged = oldAcceptableTypes.filter { newAcceptableTypes[it.key] != null && newAcceptableTypes[it.key]!!.toSet() != it.value.toSet() } + val missingEnumErrors = potentiallyChanged.map { + val oldEnums = it.value + val newEnums = newAcceptableTypes[it.key]!! + if (!newEnums.containsAll(oldEnums)) { + val toPrint = oldEnums.toMutableSet() + toPrint.removeAll(newAcceptableTypes[it.key]!!) + EnumOptionsChangedError(it.key + " on command ${old.commandName} previously accepted: $oldEnums, and now is missing $toPrint}") + } else { + null + } + }.filterNotNull() + return changedTypes + missingEnumErrors + + } + + fun checkCommandLineIsBackwardsCompatible(commandLineToCheck: Class<*>): List { + val commandLineToCheckName = commandLineToCheck.canonicalName + val instance = commandLineToCheck.newInstance() + val resourceAsStream = this.javaClass.classLoader.getResourceAsStream("$commandLineToCheckName.yml") + ?: throw IllegalStateException("no Descriptor for $commandLineToCheckName found on classpath") + val old = readCommandDescription(resourceAsStream) + val new = topoSort(CommandLine(instance)) + return checkCommandLineIsBackwardsCompatible(old, new) + } + + + fun checkBackwardsCompatibility(old: CommandLine, new: CommandLine): List { + val topoSortOld= topoSort(old) + val topoSortNew= topoSort(new) + return checkCommandLineIsBackwardsCompatible(topoSortOld, topoSortNew) + } + + private fun checkCommandLineIsBackwardsCompatible(old: List, new: List): List { + val results = ArrayList() + results += checkAllCommandsArePresent(old, new) + for (oldCommand in old) { + new.find { it.commandName == oldCommand.commandName }?.let { newCommand -> + results += checkAllOptionsArePresent(oldCommand, newCommand) + results += checkAllParamsAreOfTheSameType(oldCommand, newCommand) + results += checkAllPositionalCharactersArePresent(oldCommand, newCommand) + } + } + + return results + } +} + +open class CliBackwardsCompatibilityValidationCheck(val message: String) +class OptionsChangedError(error: String) : CliBackwardsCompatibilityValidationCheck(error) +class TypesChangedError(error: String) : CliBackwardsCompatibilityValidationCheck(error) +class EnumOptionsChangedError(error: String) : CliBackwardsCompatibilityValidationCheck(error) +class CommandsChangedError(error: String) : CliBackwardsCompatibilityValidationCheck(error) +class PositionalArgumentsChangedError(error: String) : CliBackwardsCompatibilityValidationCheck(error) +data class CommandDescription(val commandName: String, val positionalParams: List, val params: List) +data class ParameterDescription(val parameterName: String, val parameterType: String, val required: Boolean, val multiParam: Boolean, val acceptableValues: List = emptyList()) \ No newline at end of file diff --git a/testing/test-cli/src/test/kotlin/net/corda/testing/CommandLineCompatibilityCheckerTest.kt b/testing/test-cli/src/test/kotlin/net/corda/testing/CommandLineCompatibilityCheckerTest.kt new file mode 100644 index 0000000000..bf3a3cf653 --- /dev/null +++ b/testing/test-cli/src/test/kotlin/net/corda/testing/CommandLineCompatibilityCheckerTest.kt @@ -0,0 +1,106 @@ +package net.corda.testing + +import org.hamcrest.CoreMatchers.* +import org.junit.Assert +import org.junit.Test +import picocli.CommandLine +import java.util.regex.Pattern + +class CommandLineCompatibilityCheckerTest { + + enum class AllOptions { + YES, NO, MAYBZ + } + + enum class BinaryOptions { + YES, NO + } + + + @Test + fun `should detect missing parameter`() { + val value1 = object { + @CommandLine.Option(names = arrayOf("-d", "--directory"), description = arrayOf("the directory to run in")) + var baseDirectory: String? = null + } + val value2 = object { + @CommandLine.Option(names = arrayOf("--directory"), description = arrayOf("the directory to run in")) + var baseDirectory: String? = null + } + val breaks = CommandLineCompatibilityChecker().checkBackwardsCompatibility(CommandLine(value1), CommandLine(value2)) + Assert.assertThat(breaks.size, `is`(1)) + Assert.assertThat(breaks.first(), `is`(instanceOf(OptionsChangedError::class.java))) + } + + + @Test + fun `should detect changes in positional parameters`() { + val value1 = object { + @CommandLine.Parameters(index = "0") + var baseDirectory: String? = null + @CommandLine.Parameters(index = "1") + var depth: Pattern? = null + } + val value2 = object { + @CommandLine.Parameters(index = "1") + var baseDirectory: String? = null + @CommandLine.Parameters(index = "0") + var depth: Int? = null + } + val breaks = CommandLineCompatibilityChecker().checkBackwardsCompatibility(CommandLine(value1), CommandLine(value2)) + Assert.assertThat(breaks.size, `is`(2)) + Assert.assertThat(breaks.first(), `is`(instanceOf(PositionalArgumentsChangedError::class.java))) + } + + @Test + fun `should detect removal of a subcommand`() { + @CommandLine.Command(subcommands = [ListCommand::class, StatusCommand::class]) + class Dummy + + @CommandLine.Command(subcommands = [ListCommand::class]) + class Dummy2 + + val breaks = CommandLineCompatibilityChecker().checkBackwardsCompatibility(CommandLine(Dummy()), CommandLine(Dummy2())) + Assert.assertThat(breaks.size, `is`(1)) + Assert.assertThat(breaks.first(), `is`(instanceOf(CommandsChangedError::class.java))) + } + + @Test + fun `should detect change of parameter type`() { + val value1 = object { + @CommandLine.Option(names = ["--directory"], description = ["the directory to run in"]) + var baseDirectory: String? = null + } + val value2 = object { + @CommandLine.Option(names = ["--directory"], description = ["the directory to run in"]) + var baseDirectory: Pattern? = null + } + + val breaks = CommandLineCompatibilityChecker().checkBackwardsCompatibility(CommandLine(value1), CommandLine(value2)) + Assert.assertThat(breaks.size, `is`(1)) + Assert.assertThat(breaks.first(), `is`(instanceOf(TypesChangedError::class.java))) + } + + @Test + fun `should detect change of enum options`() { + val value1 = object { + @CommandLine.Option(names = ["--directory"], description = ["the directory to run in"]) + var baseDirectory: AllOptions? = null + } + val value2 = object { + @CommandLine.Option(names = ["--directory"], description = ["the directory to run in"]) + var baseDirectory: BinaryOptions? = null + } + + val breaks = CommandLineCompatibilityChecker().checkBackwardsCompatibility(CommandLine(value1), CommandLine(value2)) + Assert.assertThat(breaks.filter { it is EnumOptionsChangedError }.size, `is`(1)) + Assert.assertThat(breaks.first { it is EnumOptionsChangedError }.message, containsString(AllOptions.MAYBZ.name)) + } + + @CommandLine.Command(name = "status") + class StatusCommand + + @CommandLine.Command(name = "ls") + class ListCommand + +} \ No newline at end of file diff --git a/testing/test-cli/src/test/resources/net.corda.testing.Dummy.yml b/testing/test-cli/src/test/resources/net.corda.testing.Dummy.yml new file mode 100644 index 0000000000..fa0f03a014 --- /dev/null +++ b/testing/test-cli/src/test/resources/net.corda.testing.Dummy.yml @@ -0,0 +1,77 @@ +- commandName: "
" + positionalParams: + - parameterName: "0" + parameterType: "java.net.InetAddress" + required: true + multiParam: false + acceptableValues: [] + - parameterName: "1" + parameterType: "int" + required: true + multiParam: false + acceptableValues: [] + params: + - parameterName: "--directory" + parameterType: "java.lang.String" + required: false + multiParam: false + acceptableValues: [] + - parameterName: "-d" + parameterType: "java.lang.String" + required: false + multiParam: false + acceptableValues: [] +- commandName: "status" + positionalParams: [] + params: + - parameterName: "--pattern" + parameterType: "java.lang.String" + required: false + multiParam: true + acceptableValues: [] + - parameterName: "--style" + parameterType: "net.corda.testing.DummyEnum" + required: false + multiParam: false + acceptableValues: + - "FULL" + - "DIR" + - "FILE" + - "DISK" + - parameterName: "-p" + parameterType: "java.lang.String" + required: false + multiParam: true + acceptableValues: [] + - parameterName: "-s" + parameterType: "net.corda.testing.DummyEnum" + required: false + multiParam: false + acceptableValues: + - "FULL" + - "DIR" + - "FILE" + - "DISK" +- commandName: "ls" + positionalParams: + - parameterName: "0" + parameterType: "java.lang.String" + required: true + multiParam: false + acceptableValues: [] + - parameterName: "1" + parameterType: "int" + required: true + multiParam: false + acceptableValues: [] + params: + - parameterName: "--depth" + parameterType: "java.lang.Integer" + required: false + multiParam: false + acceptableValues: [] + - parameterName: "-d" + parameterType: "java.lang.Integer" + required: false + multiParam: false + acceptableValues: [] \ No newline at end of file