Skip to content

Commit

Permalink
Auto migrate YAML changes (#518)
Browse files Browse the repository at this point in the history
Print key warnings to console
Auto fix YAML using flank doctor --fix
Error when deprecated keys are no longer supported
  • Loading branch information
bootstraponline authored Mar 13, 2019
1 parent 4b61cb6 commit 569e1dd
Show file tree
Hide file tree
Showing 13 changed files with 210 additions and 24 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/smart_flank.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 2 additions & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/IosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
122 changes: 122 additions & 0 deletions test_runner/src/main/kotlin/ftl/args/yml/YamlDeprecated.kt
Original file line number Diff line number Diff line change
@@ -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<Transform>) {
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<Boolean, String> {
val parsed = yamlMapper.readTree(yamlData)
yamlMapper.writerWithDefaultPrettyPrinter()
var errorDetected = false
val changes = mutableListOf<Transform>()

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)
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -20,12 +21,18 @@ 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"])
var configPath: String = "./flank.yml"

@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
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -21,13 +21,18 @@ 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"])
var configPath: String = "./flank.ios.yml"

@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
}
8 changes: 0 additions & 8 deletions test_runner/src/main/kotlin/ftl/doctor/Doctor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
48 changes: 48 additions & 0 deletions test_runner/src/test/kotlin/ftl/args/yml/YamlDeprecatedTest.kt
Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AndroidDoctorCommandTest {
Truth.assertThat(output).startsWith(
"Verifies flank firebase is setup correctly\n" +
"\n" +
"doctor [-h] [-c=<configPath>]\n" +
"doctor [-fh] [-c=<configPath>]\n" +
"\n" +
"Description:\n" +
"\n" +
Expand Down Expand Up @@ -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()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class IosDoctorCommandTest {
Truth.assertThat(output).startsWith(
"Verifies flank firebase is setup correctly\n" +
"\n" +
"doctor [-h] [-c=<configPath>]\n" +
"doctor [-fh] [-c=<configPath>]\n" +
"\n" +
"Description:\n" +
"\n" +
Expand All @@ -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
Expand All @@ -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()
}
}
1 change: 0 additions & 1 deletion test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down

0 comments on commit 569e1dd

Please sign in to comment.