fix: Invoking flank yml on gcloud cli (#1041)

* Fast fail when cannot create bucket * added tests and fix device version * update tests * Fix NPE when no devices or Version node * Update Debug.kt * fix imports * Review fixes * Set extension funcs to extension properties * Update Doctor.kt * Update test_runner/src/main/kotlin/ftl/args/yml/YamlDeviceFix.kt Co-authored-by: Jan Góral <60390247+jan-gogo@users.noreply.github.com> Co-authored-by: Jan Góral <60390247+jan-gogo@users.noreply.github.com>
Flank · Aug 26, 2020 · d04f827 · d04f827
1 parent 2e46fbb
commit d04f827
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 12 deletions.
diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
@@ -50,8 +50,8 @@ object ArgsHelper {
     fun assertCommonProps(args: IArgs) {
         assertNotEmpty(
             args.project, "The project is not set. Define GOOGLE_CLOUD_PROJECT, set project in flank.yml\n" +
-                    "or save service account credential to ${defaultCredentialPath}\n" +
-                    " See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
+                "or save service account credential to ${defaultCredentialPath}\n" +
+                " See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
         )
 
         if (args.maxTestShards !in AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE && args.maxTestShards != -1)
@@ -157,7 +157,7 @@ object ArgsHelper {
                     .build()
             )
         } catch (e: Exception) {
-            // User may not have create permission
+            throw FlankGeneralError("Failed to make bucket for $projectId\nCause: ${e.message}")
         }
 
         return bucket

diff --git a/test_runner/src/main/kotlin/ftl/args/yml/YamlDeviceFix.kt b/test_runner/src/main/kotlin/ftl/args/yml/YamlDeviceFix.kt
@@ -0,0 +1,39 @@
+package ftl.args.yml
+
+import com.fasterxml.jackson.databind.JsonNode
+import com.fasterxml.jackson.databind.node.ObjectNode
+import ftl.args.ArgsHelper.yamlMapper
+import ftl.run.exception.FlankGeneralError
+import ftl.util.loadFile
+import java.nio.file.Path
+
+fun fixDevices(yamlPath: Path) =
+    if (yamlPath.toFile().exists().not()) throw FlankGeneralError("Flank yml doesn't exist at path $yamlPath")
+    else yamlPath.loadYml().fixDevices().saveFixedYml(yamlPath)
+
+private fun Path.loadYml() = yamlMapper.readTree(loadFile(this))
+
+private fun JsonNode.fixDevices() = also {
+    devicesNode.notValidDevices.forEach { device ->
+        device.fixDeviceVersion()
+    }
+}
+
+internal val JsonNode.devicesNode
+    get() = get(GCLOUD_NODE).get(DEVICES_NODE)
+
+internal val JsonNode.notValidDevices
+    get() = filterNot { it.deviceVersionValid() }
+
+private fun JsonNode.deviceVersionValid() = get(VERSION_NODE)?.isTextual ?: false
+
+private fun JsonNode.fixDeviceVersion() = get(VERSION_NODE)?.let {
+    (this as ObjectNode).put(VERSION_NODE, it.asText())
+}
+
+private fun JsonNode.saveFixedYml(yamlPath: Path) = yamlMapper.writerWithDefaultPrettyPrinter().writeValue(yamlPath.toFile(), this)
+
+const val MODEL_NODE = "model"
+const val GCLOUD_NODE = "gcloud"
+const val DEVICES_NODE = "device"
+const val VERSION_NODE = "version"
diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/CommandUtil.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/CommandUtil.kt
@@ -1,6 +1,7 @@
 package ftl.cli.firebase.test
 
 import ftl.args.yml.YamlDeprecated
+import ftl.args.yml.fixDevices
 import ftl.run.exception.YmlValidationError
 import java.nio.file.Path
 
@@ -9,15 +10,15 @@ fun processValidation(validationResult: String, shouldFix: Boolean, ymlPath: Pat
         validationResult.isBlank() -> println("Valid yml file")
         !shouldFix -> {
             println(validationResult)
-            throw YmlValidationError()
+            throw YmlValidationError("Invalid yml file, use --fix for automatically fix yml")
         }
         else -> {
             println(validationResult)
             println("Trying to fix yml file")
             if (YamlDeprecated.modify(ymlPath)) {
-                println("Unable to fix yml file")
-                throw YmlValidationError()
+                throw YmlValidationError("Invalid yml file, unable to fix yml file")
             }
+            fixDevices(ymlPath)
         }
     }
 }
diff --git a/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt b/test_runner/src/main/kotlin/ftl/doctor/Doctor.kt
@@ -5,6 +5,12 @@ import com.google.common.annotations.VisibleForTesting
 import ftl.args.AndroidArgsCompanion
 import ftl.args.ArgsHelper
 import ftl.args.IArgs
+import ftl.args.yml.DEVICES_NODE
+import ftl.args.yml.GCLOUD_NODE
+import ftl.args.yml.MODEL_NODE
+import ftl.args.yml.VERSION_NODE
+import ftl.args.yml.devicesNode
+import ftl.args.yml.notValidDevices
 import ftl.config.loadAndroidConfig
 import ftl.config.loadIosConfig
 import ftl.run.exception.FlankConfigurationError
@@ -22,7 +28,7 @@ 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) }
+        ?.run { validateYamlKeys(args).plus(validateDevices()) }
         .orEmpty()
 
 private fun JsonNode.validateYamlKeys(args: IArgs.ICompanion) = StringBuilder().apply {
@@ -61,3 +67,12 @@ private fun preloadConfiguration(data: Path, isAndroid: Boolean) =
     } catch (e: FlankConfigurationError) {
         e.message
     }
+
+private fun JsonNode.validateDevices() = StringBuilder().apply {
+    devicesNode?.notValidDevices?.withVersionNode?.forEach { device ->
+        appendLine("Warning: Version should be string $GCLOUD_NODE -> $DEVICES_NODE[${device[MODEL_NODE]}] -> $VERSION_NODE[${device[VERSION_NODE]}]")
+    }
+}.toString()
+
+private val List<JsonNode>.withVersionNode
+    get() = this.filter { it.has(VERSION_NODE) }
diff --git a/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt b/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt
@@ -142,7 +142,7 @@ object GcStorage {
                 progress.start("Uploading $fileName")
                 storage.create(fileBlob, fileBytes)
             } catch (e: Exception) {
-                throw FlankGeneralError(e)
+                throw FlankGeneralError(e.message.orEmpty(), e)
             } finally {
                 progress.stop()
             }

diff --git a/test_runner/src/main/kotlin/ftl/run/exception/FlankException.kt b/test_runner/src/main/kotlin/ftl/run/exception/FlankException.kt
@@ -33,7 +33,7 @@ open class FTLError(val matrix: SavedMatrix) : FlankException()
  *
  * Exit code: 2
  */
-class YmlValidationError : FlankException()
+class YmlValidationError(message: String? = null) : FlankException(message)
 
 /**
  * Thrown when flank run timeouted. This is flank itself timeout, not FTL.

diff --git a/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt b/test_runner/src/test/kotlin/ftl/doctor/DoctorTest.kt
@@ -5,6 +5,7 @@ import ftl.args.AndroidArgs
 import ftl.args.IArgs
 import ftl.args.IosArgs
 import ftl.test.util.FlankTestRunner
+import org.junit.Assert.assertEquals
 import org.junit.Test
 import org.junit.runner.RunWith
 import java.io.StringReader
@@ -15,7 +16,13 @@ class DoctorTest {
     @Test
     fun androidDoctorTest() {
         val lint = validateYaml(AndroidArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.local.yml"))
-        assertThat(lint).isEmpty()
+        val expected = """
+                Warning: Version should be string gcloud -> device["NexusLowRes"] -> version[23]
+                Warning: Version should be string gcloud -> device["NexusLowRes"] -> version[23]
+                Warning: Version should be string gcloud -> device["shamu"] -> version[22]
+                
+        """.trimIndent()
+        assertEquals(expected, lint)
     }
 
     @Test
@@ -87,6 +94,7 @@ flank:
     fun androidDoctorTestWithFailedConfiguration() {
         // given
         val expectedErrorMessage = """
+Warning: Version should be string gcloud -> device["Nexus5"] -> version[23]
 Error on parse config: flank->additional-app-test-apks
 At line: 20, column: 5
 Error node: {
@@ -102,7 +110,7 @@ Error node: {
             AndroidArgs,
             Paths.get("src/test/kotlin/ftl/fixtures/flank_android_failed_configuration.yml")
         )
-        assertThat(actual).isEqualTo(expectedErrorMessage)
+        assertEquals(expectedErrorMessage, actual)
     }
 
     @Test
@@ -131,7 +139,11 @@ Error node: {
     @Test
     fun iosDoctorTest() {
         val lint = validateYaml(IosArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.ios.yml"))
-        assertThat(lint).isEmpty()
+        val expected = """
+            Warning: Version should be string gcloud -> device["iphone8"] -> version[11.2]
+            
+        """.trimIndent()
+        assertEquals(expected, lint)
     }
 
     @Test
@@ -191,6 +203,40 @@ flank:
         )
         assertThat(lint).isEqualTo("")
     }
+
+    @Test
+    fun `validate result should contains warning about device version if is not compatible with gcloud cli`() {
+        val lint = validateYaml(
+            IosArgs, """
+gcloud:
+  test: .
+  xctestrun-file: .
+  device:
+    - model: NexusLowRes
+      version: 23
+flank:
+  project: .
+""".trimIndent()
+        )
+        assertEquals("Warning: Version should be string gcloud -> device[\"NexusLowRes\"] -> version[23]", lint.trim())
+    }
+
+    @Test
+    fun `should return empty validation message if device version is compatible with gcloud cli`() {
+        val lint = validateYaml(
+            IosArgs, """
+gcloud:
+  test: .
+  xctestrun-file: .
+  device:
+    - model: NexusLowRes
+      version: "23"
+flank:
+  project: .
+""".trimIndent()
+        )
+        assertEquals("", lint)
+    }
 }
 
 private fun validateYaml(args: IArgs.ICompanion, data: String): String = validateYaml(args, StringReader(data))