Skip to content

Commit 8076cdd

Browse files
Added preloading configuration and tree parsing + refactor Doctor file (#920)
* #911 Added preload configuration to detekt .yml issues by Doctor * #911 Refactor Doctor and add json tree validation * fixes PR comments
1 parent 052cd87 commit 8076cdd

7 files changed

Lines changed: 152 additions & 39 deletions

File tree

release_notes.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
- [#916](https://github.com/Flank/flank/pull/916) Test artifacts monorepo. ([jan-gogo](https://github.com/jan-gogo))
1515
- [#910](https://github.com/Flank/flank/pull/910) Migrate Bitrise release workflow into GitHub actions. ([piotradamczyk5](https://github.com/piotradamczyk5))
1616
- [#915](https://github.com/Flank/flank/pull/915) Update virtual devices sharding limit. ([adamfilipow92](https://github.com/adamfilipow92))
17+
- [#920](https://github.com/Flank/flank/pull/920) Improve .yml validation on `doctor` command. ([piotradamczyk5](https://github.com/piotradamczyk5))
1718
-
1819
-
1920

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package ftl.cli.firebase.test.android
33
import ftl.args.AndroidArgs
44
import ftl.cli.firebase.test.processValidation
55
import ftl.config.FtlConstants
6-
import ftl.doctor.Doctor.validateYaml
6+
import ftl.doctor.validateYaml
77
import java.nio.file.Paths
88
import picocli.CommandLine.Command
99
import picocli.CommandLine.Option

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package ftl.cli.firebase.test.ios
33
import ftl.args.IosArgs
44
import ftl.cli.firebase.test.processValidation
55
import ftl.config.FtlConstants
6-
import ftl.doctor.Doctor.validateYaml
6+
import ftl.doctor.validateYaml
77
import java.nio.file.Paths
88
import picocli.CommandLine.Command
99
import picocli.CommandLine.Option
Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,63 @@
11
package ftl.doctor
22

3+
import com.fasterxml.jackson.databind.JsonNode
34
import com.google.common.annotations.VisibleForTesting
5+
import ftl.args.AndroidArgsCompanion
46
import ftl.args.ArgsHelper
57
import ftl.args.IArgs
8+
import ftl.config.loadAndroidConfig
9+
import ftl.config.loadIosConfig
10+
import ftl.util.FlankFatalError
611
import ftl.util.loadFile
712
import java.io.Reader
13+
import java.lang.StringBuilder
814
import java.nio.file.Path
915

10-
object Doctor {
11-
fun validateYaml(args: IArgs.ICompanion, data: Path): String {
12-
if (!data.toFile().exists()) return "Skipping yaml validation. No file at path $data"
13-
return validateYaml(args, loadFile(data))
16+
fun validateYaml(args: IArgs.ICompanion, data: Path) =
17+
if (!data.toFile().exists()) "Skipping yaml validation. No file at path $data"
18+
else validateYaml(args, loadFile(data)) + preloadConfiguration(data, args is AndroidArgsCompanion)
19+
20+
@VisibleForTesting
21+
internal fun validateYaml(args: IArgs.ICompanion, data: Reader) =
22+
runCatching { ArgsHelper.yamlMapper.readTree(data) }
23+
.onFailure { return it.message ?: "Unknown error when parsing tree" }
24+
.getOrNull()
25+
?.run { validateYamlKeys(args) }
26+
.orEmpty()
27+
28+
private fun JsonNode.validateYamlKeys(args: IArgs.ICompanion) = StringBuilder().apply {
29+
append(validateTopLevelKeys(args))
30+
args.validArgs.forEach { (topLevelKey, validArgsKeys) ->
31+
append(validateNestedKeys(topLevelKey, validArgsKeys))
1432
}
33+
}.toString()
1534

16-
@VisibleForTesting
17-
internal fun validateYaml(args: IArgs.ICompanion, data: Reader): String {
18-
var result = ""
19-
val parsed = ArgsHelper.yamlMapper.readTree(data)
35+
private fun JsonNode.validateTopLevelKeys(args: IArgs.ICompanion) =
36+
(parseArgs().keys - args.validArgs.keys)
37+
.takeIf { it.isNotEmpty() }
38+
?.let { unknownKeys -> "Unknown top level keys: $unknownKeys\n" }
39+
.orEmpty()
2040

21-
val validArgs = args.validArgs
22-
val parsedArgs = mutableMapOf<String, List<String>>()
23-
24-
for (child in parsed.fields()) {
25-
val key = child.key
26-
val values = mutableListOf<String>()
27-
child.value.fields().forEach { values.add(it.key) }
28-
29-
parsedArgs[key] = values
30-
}
31-
32-
val unknownTopLevelKeys = parsedArgs.keys - validArgs.keys
33-
if (unknownTopLevelKeys.isNotEmpty()) result += "Unknown top level keys: $unknownTopLevelKeys\n"
34-
35-
validArgs.forEach { (topLevelKey, keyList) ->
36-
val parsedKeys = mutableListOf<String>()
37-
parsed[topLevelKey]?.fields()?.forEach { parsedKeys.add(it.key) }
38-
val unknownKeys = parsedKeys - keyList
39-
if (unknownKeys.isNotEmpty()) result += "Unknown keys in $topLevelKey -> $unknownKeys\n"
40-
}
41-
42-
return result
41+
private fun JsonNode.parseArgs() = mutableMapOf<String, List<String>>().apply {
42+
for (child in fields()) {
43+
this[child.key] = child.value.fields().asSequence().map { it.key }.toList()
4344
}
4445
}
46+
47+
private fun JsonNode.validateNestedKeys(topLevelKey: String, validArgsKeys: List<String>) =
48+
nestedKeysFor(topLevelKey)
49+
.minus(validArgsKeys)
50+
.takeIf { it.isNotEmpty() }
51+
?.let { "Unknown keys in $topLevelKey -> $it\n" }
52+
.orEmpty()
53+
54+
private fun JsonNode.nestedKeysFor(topLevelKey: String) =
55+
this[topLevelKey]?.fields()?.asSequence()?.map { it.key }?.toList().orEmpty()
56+
57+
private fun preloadConfiguration(data: Path, isAndroid: Boolean) =
58+
try {
59+
if (isAndroid) loadAndroidConfig(data) else loadIosConfig(data)
60+
""
61+
} catch (e: FlankFatalError) {
62+
e.message
63+
}

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

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import java.nio.file.Paths
1414
class DoctorTest {
1515
@Test
1616
fun androidDoctorTest() {
17-
val lint = Doctor.validateYaml(AndroidArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.local.yml"))
17+
val lint = validateYaml(AndroidArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.local.yml"))
1818
assertThat(lint).isEmpty()
1919
}
2020

2121
@Test
2222
fun androidDoctorTest2() {
23-
val lint = Doctor.validateYaml(
23+
val lint = validateYaml(
2424
AndroidArgs, """
2525
hi: .
2626
foo:
@@ -71,7 +71,7 @@ Unknown keys in flank -> [three]
7171

7272
@Test
7373
fun androidDoctorTest3() {
74-
val lint = Doctor.validateYaml(
74+
val lint = validateYaml(
7575
AndroidArgs, """
7676
gcloud:
7777
app: .
@@ -83,15 +83,60 @@ flank:
8383
assertThat(lint).isEqualTo("")
8484
}
8585

86+
@Test
87+
fun androidDoctorTestWithFailedConfiguration() {
88+
// given
89+
val expectedErrorMessage = """
90+
Error on parse config: flank->additional-app-test-apks
91+
At line: 20, column: 5
92+
Error node: {
93+
"additional-app-test-apks" : {
94+
"app" : "../sample/app/build/output/apk/debug/sample-app.apk",
95+
"test" : "../library/databases/build/output/apk/androidTest/debug/sample-databases-test.apk"
96+
}
97+
}
98+
""".trimIndent()
99+
100+
// when
101+
val actual = validateYaml(
102+
AndroidArgs,
103+
Paths.get("src/test/kotlin/ftl/fixtures/flank_android_failed_configuration.yml")
104+
)
105+
assertThat(actual).isEqualTo(expectedErrorMessage)
106+
}
107+
108+
@Test
109+
fun androidDoctorTestWithFailedTree() {
110+
// given
111+
val expectedErrorMessage = """
112+
Error on parse config: expected <block end>, but found '?'
113+
At line: 19, column: 4
114+
Error node:
115+
test: ../main/app/build/output/a ...
116+
^Error on parse config: expected <block end>, but found '?'
117+
At line: 19, column: 4
118+
Error node:
119+
test: ../main/app/build/output/a ...
120+
^
121+
""".trimIndent()
122+
123+
// when
124+
val actual = validateYaml(
125+
AndroidArgs,
126+
Paths.get("src/test/kotlin/ftl/fixtures/flank_android_failed_tree.yml")
127+
)
128+
assertThat(actual).isEqualTo(expectedErrorMessage)
129+
}
130+
86131
@Test
87132
fun iosDoctorTest() {
88-
val lint = Doctor.validateYaml(IosArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.ios.yml"))
133+
val lint = validateYaml(IosArgs, Paths.get("src/test/kotlin/ftl/fixtures/flank.ios.yml"))
89134
assertThat(lint).isEmpty()
90135
}
91136

92137
@Test
93138
fun iosDoctorTest2() {
94-
val lint = Doctor.validateYaml(
139+
val lint = validateYaml(
95140
IosArgs, """
96141
hi: .
97142
foo:
@@ -135,7 +180,7 @@ Unknown keys in flank -> [three]
135180

136181
@Test
137182
fun iosDoctorTest3() {
138-
val lint = Doctor.validateYaml(
183+
val lint = validateYaml(
139184
IosArgs, """
140185
gcloud:
141186
test: .
@@ -148,4 +193,4 @@ flank:
148193
}
149194
}
150195

151-
private fun Doctor.validateYaml(args: IArgs.ICompanion, data: String): String = validateYaml(args, StringReader(data))
196+
private fun validateYaml(args: IArgs.ICompanion, data: String): String = validateYaml(args, StringReader(data))
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
gcloud:
2+
app: /Users/no/workspace/fladle/sample/build/outputs/apk/debug/sample-debug.apk
3+
test: /Users/no/workspace/fladle/sample/build/outputs/apk/androidTest/debug/sample-debug-androidTest.apk
4+
device:
5+
- model: Nexus5
6+
version: 23
7+
use-orchestrator: true
8+
auto-google-login: false
9+
record-video: true
10+
performance-metrics: true
11+
timeout: 15m
12+
environment-variables:
13+
clearPackageData: true
14+
test-targets:
15+
- class com.osacky.flank.gradle.sample.ExampleInstrumentedTest#seeView
16+
num-flaky-test-attempts: 1
17+
flank:
18+
additional-app-test-apks:
19+
app: ../main/app/build/output/apk/debug/app.apk
20+
test: ../main/app/build/output/apk/androidTest/debug/app-test.apk
21+
app: ../sample/app/build/output/apk/debug/sample-app.apk
22+
test: ../sample/app/build/output/apk/androidTest/debug/sample-app-test.apk
23+
test: ../feature/room/build/output/apk/androidTest/debug/feature-room-test.apk
24+
test: ../library/databases/build/output/apk/androidTest/debug/sample-databases-test.apk
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
gcloud:
2+
app: /Users/no/workspace/fladle/sample/build/outputs/apk/debug/sample-debug.apk
3+
test: /Users/no/workspace/fladle/sample/build/outputs/apk/androidTest/debug/sample-debug-androidTest.apk
4+
device:
5+
- model: Nexus5
6+
version: 23
7+
use-orchestrator: true
8+
auto-google-login: false
9+
record-video: true
10+
performance-metrics: true
11+
timeout: 15m
12+
environment-variables:
13+
clearPackageData: true
14+
test-targets:
15+
- class com.osacky.flank.gradle.sample.ExampleInstrumentedTest#seeView
16+
num-flaky-test-attempts: 1
17+
flank:
18+
additional-app-test-apks:
19+
- app: ../main/app/build/output/apk/debug/app.apk
20+
test: ../main/app/build/output/apk/androidTest/debug/app-test.apk
21+
app: ../sample/app/build/output/apk/debug/sample-app.apk
22+
test: ../sample/app/build/output/apk/androidTest/debug/sample-app-test.apk
23+
test: ../feature/room/build/output/apk/androidTest/debug/feature-room-test.apk
24+
test: ../library/databases/build/output/apk/androidTest/debug/sample-databases-test.apk

0 commit comments

Comments
 (0)