From 0e40cb96a4d115920a7e3def4fb1c2ae6428b2a3 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Tue, 12 Mar 2019 20:42:52 -0400 Subject: [PATCH] Auto migrate YAML changes Print key warnings to console Auto fix YAML using flank doctor --fix Error when deprecated keys are no longer supported --- README.md | 8 +- docs/smart_flank.md | 2 +- release_notes.md | 2 + .../src/main/kotlin/ftl/args/AndroidArgs.kt | 5 +- .../src/main/kotlin/ftl/args/IosArgs.kt | 5 +- .../kotlin/ftl/args/yml/YamlDeprecated.kt | 122 ++++++++++++++++++ .../test/android/AndroidDoctorCommand.kt | 9 +- .../cli/firebase/test/ios/IosDoctorCommand.kt | 9 +- .../src/main/kotlin/ftl/doctor/Doctor.kt | 8 -- .../kotlin/ftl/args/yml/YamlDeprecatedTest.kt | 48 +++++++ .../test/android/AndroidDoctorCommandTest.kt | 6 +- .../firebase/test/ios/IosDoctorCommandTest.kt | 9 +- .../src/test/kotlin/ftl/doctor/DoctorTest.kt | 1 - 13 files changed, 210 insertions(+), 24 deletions(-) create mode 100644 test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt create mode 100644 test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt diff --git a/README.md b/README.md index 27ac6550e5..b0f9c25f11 100644 --- a/README.md +++ b/README.md @@ -119,10 +119,10 @@ flank: ## test shards - the maxmimum amount of groups to split the test suite into ## set to -1 to use one shard per test. default: 1 - # testShards: 1 + # maxTestShards: 1 ## shard time - the amount of time tests within a shard should take - ## when set to > 0, the shard count is dynamically set based on time up to the maxmimum limit defined by testShards + ## when set to > 0, the shard count is dynamically set based on time up to the maxmimum limit defined by maxTestShards ## 2 minutes (120) is recommended. ## default: -1 (unlimited) # shardTime: -1 @@ -259,10 +259,10 @@ flank: ## test shards - the amount of groups to split the test suite into ## set to -1 to use one shard per test. default: 1 - # testShards: 1 + # maxTestShards: 1 ## shard time - the amount of time tests within a shard should take - ## when set to > 0, the shard count is dynamically set based on time up to the maxmimum limit defined by testShards + ## when set to > 0, the shard count is dynamically set based on time up to the maxmimum limit defined by maxTestShards ## 2 minutes (120) is recommended. ## default: -1 (unlimited) # shardTime: -1 diff --git a/docs/smart_flank.md b/docs/smart_flank.md index d89d8410e1..a3cc867a16 100644 --- a/docs/smart_flank.md +++ b/docs/smart_flank.md @@ -4,7 +4,7 @@ Smart Flank is a sharding algorithm that groups tests into equally sized buckets ## Current implementation -At the start of a run, Flank checks to see if there's a JUnit XML with timing info from the previous run. If there's no previous test time recorded, then the test is estimated to be 10 seconds. The tests are then grouped into equally timed buckets. The bucket count is set by the user provided `testShards` count. +At the start of a run, Flank checks to see if there's a JUnit XML with timing info from the previous run. If there's no previous test time recorded, then the test is estimated to be 10 seconds. The tests are then grouped into equally timed buckets. The bucket count is set by the user provided `maxTestShards` count. After each test run, the aggregated JUnit XML from the new run is merged with the old run. Any tests not in the new run are discarded. Tests that were skipped, errored, failed, or empty are discarded. The test time value of successful tests is set using the time from the latest run. If a test failed in the new run and passed in the old run, the timing info from the old run is carried over. diff --git a/release_notes.md b/release_notes.md index c7a7d34a40..fb1395b5af 100644 --- a/release_notes.md +++ b/release_notes.md @@ -2,6 +2,8 @@ - [#506](https://github.com/TestArmada/flank/pull/506) Add bugsnag reporting to detect Flank crashes. ([bootstraponline](https://github.com/bootstraponline)) - [#507](https://github.com/TestArmada/flank/pull/507) Improve error message when credentials fail to load, folder doesn't exist, and on bucket creation failure. Properly pass through user credential when checking the storage bucket. ([bootstraponline](https://github.com/bootstraponline)) +- [#514](https://github.com/TestArmada/flank/pull/514) Rename `testShards` to `maxTestShards` ([miguelslemos](https://github.com/miguelslemos)) +- [#518](https://github.com/TestArmada/flank/pull/518) Add deprecation warnings when old key names are used. `flank android doctor --fix` will auto fix the YAML file. ([bootstraponline](https://github.com/bootstraponline)) ## v4.4.0 diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt index 8a7c3e3ae6..bfe2db6a00 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt @@ -21,6 +21,7 @@ import ftl.args.ArgsToString.mapToString import ftl.args.yml.AndroidGcloudYml import ftl.args.yml.FlankYml import ftl.args.yml.GcloudYml +import ftl.args.yml.YamlDeprecated import ftl.cli.firebase.test.android.AndroidRunCommand import ftl.config.Device import ftl.config.FtlConstants @@ -183,7 +184,9 @@ ${listToString(testTargetsAlwaysRun)} fun load(data: Path, cli: AndroidRunCommand? = null): AndroidArgs = load(String(Files.readAllBytes(data)), cli) - fun load(data: String, cli: AndroidRunCommand? = null): AndroidArgs { + fun load(yamlData: String, cli: AndroidRunCommand? = null): AndroidArgs { + val data = YamlDeprecated.modifyAndThrow(yamlData, android = true) + val flankYml = yamlMapper.readValue(data, FlankYml::class.java) val gcloudYml = yamlMapper.readValue(data, GcloudYml::class.java) val androidGcloudYml = yamlMapper.readValue(data, AndroidGcloudYml::class.java) diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt index 5396a1a79c..b0109246b0 100644 --- a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt +++ b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt @@ -13,6 +13,7 @@ import ftl.args.yml.FlankYml import ftl.args.yml.GcloudYml import ftl.args.yml.IosFlankYml import ftl.args.yml.IosGcloudYml +import ftl.args.yml.YamlDeprecated import ftl.cli.firebase.test.ios.IosRunCommand import ftl.config.Device import ftl.config.FtlConstants @@ -141,7 +142,9 @@ ${listToString(testTargets)} fun load(data: Path, cli: IosRunCommand? = null): IosArgs = IosArgs.load(String(Files.readAllBytes(data)), cli) - fun load(data: String, cli: IosRunCommand? = null): IosArgs { + fun load(yamlData: String, cli: IosRunCommand? = null): IosArgs { + val data = YamlDeprecated.modifyAndThrow(yamlData, android = false) + val flankYml = yamlMapper.readValue(data, FlankYml::class.java) val iosFlankYml = yamlMapper.readValue(data, IosFlankYml::class.java) val gcloudYml = yamlMapper.readValue(data, GcloudYml::class.java) diff --git a/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt b/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt new file mode 100644 index 0000000000..5b0e295da4 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt @@ -0,0 +1,122 @@ +package ftl.args.yml + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.node.ObjectNode +import ftl.args.ArgsHelper.yamlMapper +import ftl.util.Utils +import ftl.util.Utils.fatalError +import java.nio.file.Files +import java.nio.file.Path + +object YamlDeprecated { + + @Suppress("EnumEntryName", "EnumNaming") + enum class Parent { gcloud, flank } + + enum class Level { Warning, Error } + + private data class Key( + val parent: Parent, + val name: String + ) + + private data class ModifiedKey( + val old: Key, + val new: Key, + val level: Level + ) + + private val transforms = listOf( + // flank: testShards -> flank: maxTestShards + ModifiedKey( + Key(YamlDeprecated.Parent.flank, "testShards"), + Key(YamlDeprecated.Parent.flank, "maxTestShards"), + Level.Warning + ) + ) + + private data class Transform( + val keyValue: JsonNode, + val key: ModifiedKey + ) + + private fun JsonNode.remove(parent: Parent, child: String) { + (this[parent.toString()] as ObjectNode).remove(child) + } + + private fun JsonNode.replace(parent: Parent, child: String, value: JsonNode) { + (this[parent.toString()] as ObjectNode).replace(child, value) + } + + private fun mutate(parsed: JsonNode, changes: List) { + changes.forEach { transform -> + mutateNode(parsed, transform.keyValue, transform.key.old, transform.key.new) + } + } + + private fun mutateNode(parsed: JsonNode, keyValue: JsonNode, old: Key, new: Key) { + parsed.remove(parent = old.parent, child = old.name) + parsed.replace(parent = new.parent, child = new.name, value = keyValue) + } + + private fun validate(key: Key, keyValue: JsonNode): Transform? { + transforms.forEach { + if (it.old == key) { + println("${it.level}: `${it.old.parent}: ${it.old.name}:` renamed to `${it.new.parent}: ${it.new.name}:`") + return Transform(keyValue, it) + } + } + + return null + } + + private val yamlWriter by lazy { yamlMapper.writerWithDefaultPrettyPrinter() } + + fun modify(yamlPath: Path, fix: Boolean = false): Boolean { + if (yamlPath.toFile().exists().not()) fatalError("Flank yml doesn't exist at path $yamlPath") + val data = String(Files.readAllBytes(yamlPath)) + + val (errorDetected, string) = modify(data) + + if (fix) { + Files.write(yamlPath, string.toByteArray()) + println("\nUpdated flank.yml file") + } + return errorDetected + } + + // Throw exception when Level.Error modified key is found. + fun modifyAndThrow(yamlData: String, android: Boolean): String { + val (error, data) = YamlDeprecated.modify(yamlData) + + if (error) { + val platform = if (android) "android" else "ios" + Utils.fatalError("Invalid keys detected! Auto fix with: flank $platform doctor --fix") + } + + return data + } + + fun modify(yamlData: String): Pair { + val parsed = yamlMapper.readTree(yamlData) + yamlMapper.writerWithDefaultPrettyPrinter() + var errorDetected = false + val changes = mutableListOf() + + listOf("gcloud", "flank").forEach { keyType -> + parsed[keyType]?.fields()?.forEach { (keyName, keyValue) -> + val type = Parent.valueOf(keyType) + val newChange = validate(Key(type, keyName), keyValue) + + if (newChange != null) { + changes.add(newChange) + if (newChange.key.level == Level.Error) errorDetected = true + } + } + } + + mutate(parsed, changes) + + return errorDetected to yamlWriter.writeValueAsString(parsed) + } +} diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommand.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommand.kt index 1b4566c772..b59cfc86c4 100644 --- a/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommand.kt +++ b/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommand.kt @@ -1,6 +1,7 @@ package ftl.cli.firebase.test.android import ftl.args.AndroidArgs +import ftl.args.yml.YamlDeprecated import ftl.doctor.Doctor.validateYaml import java.nio.file.Paths import picocli.CommandLine.Command @@ -20,7 +21,10 @@ import picocli.CommandLine.Option ) class AndroidDoctorCommand : Runnable { override fun run() { - println(validateYaml(AndroidArgs, Paths.get(configPath))) + val ymlPath = Paths.get(configPath) + println(validateYaml(AndroidArgs, ymlPath)) + + YamlDeprecated.modify(ymlPath, fix) } @Option(names = ["-c", "--config"], description = ["YAML config file path"]) @@ -28,4 +32,7 @@ class AndroidDoctorCommand : Runnable { @Option(names = ["-h", "--help"], usageHelp = true, description = ["Prints this help message"]) var usageHelpRequested: Boolean = false + + @Option(names = ["-f", "--fix"], description = ["Auto fix flank YAML file"]) + var fix: Boolean = false } diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommand.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommand.kt index 78dbab9682..b9d0f77d5f 100644 --- a/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommand.kt +++ b/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommand.kt @@ -1,7 +1,7 @@ package ftl.cli.firebase.test.ios import ftl.args.IosArgs -import ftl.doctor.Doctor.checkIosCatalog +import ftl.args.yml.YamlDeprecated import ftl.doctor.Doctor.validateYaml import java.nio.file.Paths import picocli.CommandLine.Command @@ -21,8 +21,10 @@ import picocli.CommandLine.Option ) class IosDoctorCommand : Runnable { override fun run() { - checkIosCatalog() + val ymlPath = Paths.get(configPath) println(validateYaml(IosArgs, Paths.get(configPath))) + + YamlDeprecated.modify(ymlPath, fix) } @Option(names = ["-c", "--config"], description = ["YAML config file path"]) @@ -30,4 +32,7 @@ class IosDoctorCommand : Runnable { @Option(names = ["-h", "--help"], usageHelp = true, description = ["Prints this help message"]) var usageHelpRequested: Boolean = false + + @Option(names = ["-f", "--fix"], description = ["Auto fix flank YAML file"]) + var fix: Boolean = false } diff --git a/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt b/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt index f36f0f8b8d..f2e279eede 100644 --- a/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt +++ b/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt @@ -2,18 +2,10 @@ package ftl.doctor import ftl.args.ArgsHelper import ftl.args.IArgsCompanion -import ftl.ios.IosCatalog import java.nio.file.Files import java.nio.file.Path object Doctor { - fun checkIosCatalog() { - if (!IosCatalog.supportedDevice("iphone8", "11.2")) { - throw RuntimeException("iPhone 8 with iOS 11.2 is unexpectedly unsupported") - } - println("Flank successfully connected to iOS catalog") - } - fun validateYaml(args: IArgsCompanion, data: Path): String { if (!data.toFile().exists()) return "Skipping yaml validation. No file at path $data" return validateYaml(args, String(Files.readAllBytes(data))) diff --git a/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt b/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt new file mode 100644 index 0000000000..9d17fccd3a --- /dev/null +++ b/test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt @@ -0,0 +1,48 @@ +package ftl.args.yml + +import com.google.common.truth.Truth.assertThat +import ftl.test.util.FlankTestRunner +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(FlankTestRunner::class) +class YamlDeprecatedTest { + + @Test + fun `Valid YAML`() { + val input = """ + --- + gcloud: + app: "a" + test: "b" + + """.trimIndent() + + val (error, output) = YamlDeprecated.modify(input) + + assertThat(error).isFalse() + assertThat(output).isEqualTo(input) + } + + @Test + fun `Flank testShards renamed to maxTestShards`() { + val input = """ + --- + flank: + testShards: 1 + + """.trimIndent() + + val expected = """ + --- + flank: + maxTestShards: 1 + + """.trimIndent() + + val (error, output) = YamlDeprecated.modify(input) + + assertThat(error).isFalse() + assertThat(output).isEqualTo(expected) + } +} diff --git a/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommandTest.kt b/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommandTest.kt index af3d8a9ac9..60360ba3a2 100644 --- a/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommandTest.kt +++ b/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommandTest.kt @@ -26,7 +26,7 @@ class AndroidDoctorCommandTest { Truth.assertThat(output).startsWith( "Verifies flank firebase is setup correctly\n" + "\n" + - "doctor [-h] [-c=]\n" + + "doctor [-fh] [-c=]\n" + "\n" + "Description:\n" + "\n" + @@ -60,5 +60,9 @@ class AndroidDoctorCommandTest { assertThat(cmd.usageHelpRequested).isFalse() cmd.usageHelpRequested = true assertThat(cmd.usageHelpRequested).isTrue() + + assertThat(cmd.fix).isFalse() + cmd.fix = true + assertThat(cmd.fix).isTrue() } } diff --git a/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommandTest.kt b/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommandTest.kt index a1b76725eb..feaa38efbb 100644 --- a/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommandTest.kt +++ b/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommandTest.kt @@ -26,7 +26,7 @@ class IosDoctorCommandTest { Truth.assertThat(output).startsWith( "Verifies flank firebase is setup correctly\n" + "\n" + - "doctor [-h] [-c=]\n" + + "doctor [-fh] [-c=]\n" + "\n" + "Description:\n" + "\n" + @@ -45,9 +45,6 @@ class IosDoctorCommandTest { @Test fun iosDoctorCommandRuns() { IosDoctorCommand().run() - // iOS doctor connects to iOS catalog in addition to YAML validation - val output = systemOutRule.log - Truth.assertThat(output).contains("Flank successfully connected to iOS catalog") } @Test @@ -60,5 +57,9 @@ class IosDoctorCommandTest { assertThat(cmd.usageHelpRequested).isFalse() cmd.usageHelpRequested = true assertThat(cmd.usageHelpRequested).isTrue() + + assertThat(cmd.fix).isFalse() + cmd.fix = true + assertThat(cmd.fix).isTrue() } } diff --git a/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt b/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt index 6af2075c6e..ef030a8b21 100644 --- a/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt +++ b/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt @@ -82,7 +82,6 @@ gcloud: @Test fun iosDoctorTest() { - Doctor.checkIosCatalog() val lint = Doctor.validateYaml(IosArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.ios.yml")) assertThat(lint).isEmpty() }