Added preloading configuration and tree parsing + refactor Doctor file

release_notes.md

@@ -14,6 +14,7 @@
 - [#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))
 - [#915](https://github.com/Flank/flank/pull/915) Update virtual devices sharding limit. ([adamfilipow92](https://github.com/adamfilipow92))
+- [#920](https://github.com/Flank/flank/pull/920) Improve .yml validation on `doctor` command. ([piotradamczyk5](https://github.com/piotradamczyk5))
 -
 -
 

test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidDoctorCommand.kt

@@ -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

test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosDoctorCommand.kt

@@ -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

test_runner/src/main/kotlin/ftl/doctor/Doctor.kt

@@ -1,44 +1,63 @@
 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))
+fun validateYaml(args: IArgs.ICompanion, data: Path) =
+    if (!data.toFile().exists()) "Skipping yaml validation. No file at path $data"
+    else validateYaml(args, loadFile(data)) + preloadConfiguration(data, args is AndroidArgsCompanion)
+
+@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()
+
+private fun JsonNode.validateYamlKeys(args: IArgs.ICompanion) = StringBuilder().apply {
+    append(validateTopLevelKeys(args))
+    args.validArgs.forEach { (topLevelKey, validArgsKeys) ->
+        append(validateNestedKeys(topLevelKey, validArgsKeys))
     }
+}.toString()
 
-    @VisibleForTesting
-    internal fun validateYaml(args: IArgs.ICompanion, data: Reader): String {
-        var result = ""
-        val parsed = ArgsHelper.yamlMapper.readTree(data)
+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 validArgs = args.validArgs
-        val parsedArgs = mutableMapOf<String, List<String>>()
-
-        for (child in parsed.fields()) {
-            val key = child.key
-            val values = mutableListOf<String>()
-            child.value.fields().forEach { values.add(it.key) }
-
-            parsedArgs[key] = values
-        }
-
-        val unknownTopLevelKeys = parsedArgs.keys - validArgs.keys
-        if (unknownTopLevelKeys.isNotEmpty()) result += "Unknown top level keys: $unknownTopLevelKeys\n"
-
-        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"
-        }
-
-        return result
+private fun JsonNode.parseArgs() = mutableMapOf<String, List<String>>().apply {
+    for (child in fields()) {
+        this[child.key] = child.value.fields().asSequence().map { it.key }.toList()
     }
 }
+
+private fun JsonNode.validateNestedKeys(topLevelKey: String, validArgsKeys: List<String>) =
+    nestedKeysFor(topLevelKey)
+        .minus(validArgsKeys)
+        .takeIf { it.isNotEmpty() }
+        ?.let { "Unknown keys in $topLevelKey -> $it\n" }
+        .orEmpty()
+
+private fun JsonNode.nestedKeysFor(topLevelKey: String) =
+    this[topLevelKey]?.fields()?.asSequence()?.map { it.key }?.toList().orEmpty()
+
+private fun preloadConfiguration(data: Path, isAndroid: Boolean) =
+    try {
+        if (isAndroid) loadAndroidConfig(data) else loadIosConfig(data)
+        ""
+    } catch (e: FlankFatalError) {
+        e.message
+    }

test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt

@@ -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:
@@ -71,7 +71,7 @@ Unknown keys in flank -> [three]
 
     @Test
     fun androidDoctorTest3() {
-        val lint = Doctor.validateYaml(
+        val lint = validateYaml(
             AndroidArgs, """
 gcloud:
   app: .
@@ -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:
@@ -135,7 +180,7 @@ Unknown keys in flank -> [three]
 
     @Test
     fun iosDoctorTest3() {
-        val lint = Doctor.validateYaml(
+        val lint = validateYaml(
             IosArgs, """
 gcloud:
   test: .
@@ -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))

test_runner/src/test/kotlin/ftl/fixtures/flank_android_failed_configuration.yml

@@ -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

test_runner/src/test/kotlin/ftl/fixtures/flank_android_failed_tree.yml

@@ -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