Skip to content

Commit

Permalink
#911 Refactor Doctor and add json tree validation
Browse files Browse the repository at this point in the history
  • Loading branch information
Piotr Adamczyk committed Jul 31, 2020
1 parent 0b9b937 commit 9973baa
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 41 deletions.
3 changes: 2 additions & 1 deletion release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
- [#907](https://github.com/Flank/flank/pull/907) Added option to print Android available locales to test against. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#913](https://github.com/Flank/flank/pull/913) Add Gradle Enterprise API example. ([pawelpasterz](https://github.com/pawelpasterz))
- [#916](https://github.com/Flank/flank/pull/916) Test artifacts monorepo. ([jan-gogo](https://github.com/jan-gogo))
- [#910](https://github.com/Flank/flank/pull/910) Migrate Bitrise release workflow into GitHub actions. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#910](https://github.com/Flank/flank/pull/910) Migrate Bitrise release workflow into GitHub actions. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#920](https://github.com/Flank/flank/pull/920) Improve .yml validation on `doctor` command. ([piotradamczyk5](https://github.com/piotradamczyk5))
-
-

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ftl.cli.firebase.test.android
import ftl.args.AndroidArgs
import ftl.cli.firebase.test.processValidation
import ftl.config.FtlConstants
import ftl.doctor.Doctor.validateYaml
import ftl.doctor.validateYaml
import java.nio.file.Paths
import picocli.CommandLine.Command
import picocli.CommandLine.Option
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ftl.cli.firebase.test.ios
import ftl.args.IosArgs
import ftl.cli.firebase.test.processValidation
import ftl.config.FtlConstants
import ftl.doctor.Doctor.validateYaml
import ftl.doctor.validateYaml
import java.nio.file.Paths
import picocli.CommandLine.Command
import picocli.CommandLine.Option
Expand Down
72 changes: 41 additions & 31 deletions test_runner/src/main/kotlin/ftl/doctor/Doctor.kt
Original file line number Diff line number Diff line change
@@ -1,54 +1,64 @@
package ftl.doctor

import com.fasterxml.jackson.databind.JsonNode
import com.google.common.annotations.VisibleForTesting
import ftl.args.AndroidArgsCompanion
import ftl.args.ArgsHelper
import ftl.args.IArgs
import ftl.config.loadAndroidConfig
import ftl.config.loadIosConfig
import ftl.util.FlankFatalError
import ftl.util.loadFile
import java.io.Reader
import java.lang.StringBuilder
import java.nio.file.Path

object Doctor {
fun validateYaml(args: IArgs.ICompanion, data: Path): String {
if (!data.toFile().exists()) return "Skipping yaml validation. No file at path $data"
return validateYaml(args, loadFile(data)) + preloadConfiguration(data, args is AndroidArgsCompanion)
}

@VisibleForTesting
internal fun validateYaml(args: IArgs.ICompanion, data: Reader): String {
var result = ""
val parsed = ArgsHelper.yamlMapper.readTree(data)
fun validateYaml(args: IArgs.ICompanion, data: Path): String {
if (!data.toFile().exists()) return "Skipping yaml validation. No file at path $data"
return validateYaml(args, loadFile(data)) + preloadConfiguration(data, args is AndroidArgsCompanion)
}

val validArgs = args.validArgs
val parsedArgs = mutableMapOf<String, List<String>>()
@VisibleForTesting
internal fun validateYaml(args: IArgs.ICompanion, data: Reader) =
runCatching { ArgsHelper.yamlMapper.readTree(data) }
.onFailure { return it.message ?: "Unknown error when parsing tree" }
.getOrNull()
?.run { validateYamlKeys(args) }
.orEmpty()

for (child in parsed.fields()) {
val key = child.key
val values = mutableListOf<String>()
child.value.fields().forEach { values.add(it.key) }
private fun JsonNode.validateYamlKeys(args: IArgs.ICompanion) = StringBuilder().apply {
append(validateTopLevelKeys(args))
args.validArgs.forEach { (topLevelKey, validArgsKeys) ->
append(validateNestedKeys(topLevelKey, validArgsKeys))
}
}.toString()

parsedArgs[key] = values
}
private fun JsonNode.validateTopLevelKeys(args: IArgs.ICompanion) =
(parseArgs().keys - args.validArgs.keys)
.takeIf { it.isNotEmpty() }
?.let { unknownKeys -> "Unknown top level keys: $unknownKeys\n" }
.orEmpty()

val unknownTopLevelKeys = parsedArgs.keys - validArgs.keys
if (unknownTopLevelKeys.isNotEmpty()) result += "Unknown top level keys: $unknownTopLevelKeys\n"
private fun JsonNode.parseArgs() = mutableMapOf<String, List<String>>().apply {
for (child in fields()) {
this[child.key] = child.value.fields().asSequence().map { it.key }.toList()
}
}

validArgs.forEach { (topLevelKey, keyList) ->
val parsedKeys = mutableListOf<String>()
parsed[topLevelKey]?.fields()?.forEach { parsedKeys.add(it.key) }
val unknownKeys = parsedKeys - keyList
if (unknownKeys.isNotEmpty()) result += "Unknown keys in $topLevelKey -> $unknownKeys\n"
}
private fun JsonNode.validateNestedKeys(topLevelKey: String, validArgsKeys: List<String>) =
nestedKeysFor(topLevelKey)
.minus(validArgsKeys)
.takeIf { it.isNotEmpty() }
?.let { "Unknown keys in $topLevelKey -> $it\n" }
.orEmpty()

return result
}
private fun JsonNode.nestedKeysFor(topLevelKey: String) =
this[topLevelKey]?.fields()?.asSequence()?.map { it.key }?.toList().orEmpty()

private fun preloadConfiguration(data: Path, isAndroid: Boolean) = try {
private fun preloadConfiguration(data: Path, isAndroid: Boolean) =
try {
if (isAndroid) loadAndroidConfig(data) else loadIosConfig(data)
""
} catch (e: Exception) {
} catch (e: FlankFatalError) {
e.message
}
}
59 changes: 52 additions & 7 deletions test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import java.nio.file.Paths
class DoctorTest {
@Test
fun androidDoctorTest() {
val lint = Doctor.validateYaml(AndroidArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.local.yml"))
val lint = validateYaml(AndroidArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.local.yml"))
assertThat(lint).isEmpty()
}

@Test
fun androidDoctorTest2() {
val lint = Doctor.validateYaml(
val lint = validateYaml(
AndroidArgs, """
hi: .
foo:
Expand Down Expand Up @@ -71,7 +71,7 @@ Unknown keys in flank -> [three]

@Test
fun androidDoctorTest3() {
val lint = Doctor.validateYaml(
val lint = validateYaml(
AndroidArgs, """
gcloud:
app: .
Expand All @@ -83,15 +83,60 @@ flank:
assertThat(lint).isEqualTo("")
}

@Test
fun androidDoctorTestWithFailedConfiguration() {
// given
val expectedErrorMessage = """
Error on parse config: flank->additional-app-test-apks
At line: 20, column: 5
Error node: {
"additional-app-test-apks" : {
"app" : "../sample/app/build/output/apk/debug/sample-app.apk",
"test" : "../library/databases/build/output/apk/androidTest/debug/sample-databases-test.apk"
}
}
""".trimIndent()

// when
val actual = validateYaml(
AndroidArgs,
Paths.get("src/test/kotlin/ftl/fixtures/flank_android_failed_configuration.yml")
)
assertThat(actual).isEqualTo(expectedErrorMessage)
}

@Test
fun androidDoctorTestWithFailedTree() {
// given
val expectedErrorMessage = """
Error on parse config: expected <block end>, but found '?'
At line: 19, column: 4
Error node:
test: ../main/app/build/output/a ...
^Error on parse config: expected <block end>, but found '?'
At line: 19, column: 4
Error node:
test: ../main/app/build/output/a ...
^
""".trimIndent()

// when
val actual = validateYaml(
AndroidArgs,
Paths.get("src/test/kotlin/ftl/fixtures/flank_android_failed_tree.yml")
)
assertThat(actual).isEqualTo(expectedErrorMessage)
}

@Test
fun iosDoctorTest() {
val lint = Doctor.validateYaml(IosArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.ios.yml"))
val lint = validateYaml(IosArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.ios.yml"))
assertThat(lint).isEmpty()
}

@Test
fun iosDoctorTest2() {
val lint = Doctor.validateYaml(
val lint = validateYaml(
IosArgs, """
hi: .
foo:
Expand Down Expand Up @@ -135,7 +180,7 @@ Unknown keys in flank -> [three]

@Test
fun iosDoctorTest3() {
val lint = Doctor.validateYaml(
val lint = validateYaml(
IosArgs, """
gcloud:
test: .
Expand All @@ -148,4 +193,4 @@ flank:
}
}

private fun Doctor.validateYaml(args: IArgs.ICompanion, data: String): String = validateYaml(args, StringReader(data))
private fun validateYaml(args: IArgs.ICompanion, data: String): String = validateYaml(args, StringReader(data))
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
gcloud:
app: /Users/no/workspace/fladle/sample/build/outputs/apk/debug/sample-debug.apk
test: /Users/no/workspace/fladle/sample/build/outputs/apk/androidTest/debug/sample-debug-androidTest.apk
device:
- model: Nexus5
version: 23
use-orchestrator: true
auto-google-login: false
record-video: true
performance-metrics: true
timeout: 15m
environment-variables:
clearPackageData: true
test-targets:
- class com.osacky.flank.gradle.sample.ExampleInstrumentedTest#seeView
num-flaky-test-attempts: 1
flank:
additional-app-test-apks:
app: ../main/app/build/output/apk/debug/app.apk
test: ../main/app/build/output/apk/androidTest/debug/app-test.apk
app: ../sample/app/build/output/apk/debug/sample-app.apk
test: ../sample/app/build/output/apk/androidTest/debug/sample-app-test.apk
test: ../feature/room/build/output/apk/androidTest/debug/feature-room-test.apk
test: ../library/databases/build/output/apk/androidTest/debug/sample-databases-test.apk
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
gcloud:
app: /Users/no/workspace/fladle/sample/build/outputs/apk/debug/sample-debug.apk
test: /Users/no/workspace/fladle/sample/build/outputs/apk/androidTest/debug/sample-debug-androidTest.apk
device:
- model: Nexus5
version: 23
use-orchestrator: true
auto-google-login: false
record-video: true
performance-metrics: true
timeout: 15m
environment-variables:
clearPackageData: true
test-targets:
- class com.osacky.flank.gradle.sample.ExampleInstrumentedTest#seeView
num-flaky-test-attempts: 1
flank:
additional-app-test-apks:
- app: ../main/app/build/output/apk/debug/app.apk
test: ../main/app/build/output/apk/androidTest/debug/app-test.apk
app: ../sample/app/build/output/apk/debug/sample-app.apk
test: ../sample/app/build/output/apk/androidTest/debug/sample-app-test.apk
test: ../feature/room/build/output/apk/androidTest/debug/feature-room-test.apk
test: ../library/databases/build/output/apk/androidTest/debug/sample-databases-test.apk

0 comments on commit 9973baa

Please sign in to comment.