Skip to content

Commit ba849f3

Browse files
authored
Skip additional config files in workspace execution planner (#3150)
* fix: added files ending with config.yaml as config file * fix: added config check in case flow parsing fail and then determine if its config * fix: added config checks * removed isconfig check, since we handle _config case directly in Filters now * updated config detection * fix: tighten config file detection to avoid false positives - Changed topLevelKeys check from `any` to `all` so only files where every top-level key is a WorkspaceConfig property are excluded - Skip files containing `---` document separator since those are flow files, not config files * fix: resolve config path before filtering flow files (#3159) fix:resolve_config_path_before_filtering_flow_files * fix: added files ending with config.yaml as config file * fix: added config check in case flow parsing fail and then determine if its config * fix: added config checks * removed isconfig check, since we handle _config case directly in Filters now * updated config detection * fix: tighten config file detection to avoid false positives - Changed topLevelKeys check from `any` to `all` so only files where every top-level key is a WorkspaceConfig property are excluded - Skip files containing `---` document separator since those are flow files, not config files * fix: warn in case config file has field that is not supported
1 parent b979e6c commit ba849f3

10 files changed

Lines changed: 130 additions & 2 deletions

File tree

maestro-orchestra-models/src/main/java/maestro/orchestra/WorkspaceConfig.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package maestro.orchestra
22

33
import com.fasterxml.jackson.annotation.JsonAnySetter
44
import com.fasterxml.jackson.annotation.JsonCreator
5+
import kotlin.reflect.full.declaredMemberProperties
56

67
data class WorkspaceConfig(
78
val flows: StringList? = null,
@@ -74,4 +75,10 @@ data class WorkspaceConfig(
7475
}
7576
}
7677
}
78+
79+
companion object {
80+
val KNOWN_KEYS: Set<String> by lazy {
81+
WorkspaceConfig::class.declaredMemberProperties.map { it.name }.toSet()
82+
}
83+
}
7784
}
Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,29 @@
11
package maestro.orchestra.workspace
22

3+
import maestro.orchestra.yaml.YamlCommandReader
34
import java.nio.file.Path
45
import kotlin.io.path.absolutePathString
56
import kotlin.io.path.extension
67
import kotlin.io.path.isRegularFile
78
import kotlin.io.path.nameWithoutExtension
9+
import kotlin.io.path.readText
810

911
fun isFlowFile(path: Path, config: Path?): Boolean {
1012
if (!path.isRegularFile()) return false // Not a file
1113
if (path.absolutePathString() == config?.absolutePathString()) return false // Config file
1214
val extension = path.extension
1315
if (extension != "yaml" && extension != "yml") return false // Not YAML
1416
if (path.nameWithoutExtension == "config") return false // Config file
15-
return true
16-
}
17+
18+
return !isWorkspaceConfigFile(path)
19+
}
20+
21+
private fun isWorkspaceConfigFile(path: Path): Boolean {
22+
return try {
23+
val content = path.readText()
24+
if (content.contains("\n---")) return false // Flow files have a document separator
25+
YamlCommandReader.findUnknownWorkspaceConfigKeys(content)?.isEmpty() ?: false
26+
} catch (e: Exception) {
27+
false
28+
}
29+
}

maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlCommandReader.kt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,24 @@ package maestro.orchestra.yaml
2121

2222
import com.fasterxml.jackson.core.JsonLocation
2323
import com.fasterxml.jackson.core.JsonProcessingException
24+
import com.fasterxml.jackson.databind.ObjectMapper
25+
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory
2426
import maestro.orchestra.ApplyConfigurationCommand
2527
import maestro.orchestra.MaestroCommand
2628
import maestro.orchestra.MaestroConfig
2729
import maestro.orchestra.WorkspaceConfig
2830
import maestro.orchestra.error.SyntaxError
2931
import maestro.utils.drawTextBox
32+
import org.slf4j.LoggerFactory
3033
import java.nio.file.Path
3134
import java.nio.file.Paths
3235
import kotlin.io.path.absolutePathString
3336
import kotlin.io.path.readText
3437

3538
object YamlCommandReader {
3639

40+
private val logger = LoggerFactory.getLogger(YamlCommandReader::class.java)
41+
3742
// If it exists, automatically resolves the initFlow file and inlines the commands into the config
3843
fun readCommands(flowPath: Path): List<MaestroCommand> = mapParsingErrors(flowPath) {
3944
val flow = flowPath.readText()
@@ -49,12 +54,40 @@ object YamlCommandReader {
4954
MaestroFlowParser.parseConfigOnly(flowPath, flow)
5055
}
5156

57+
private val YAML_MAPPER by lazy { ObjectMapper(YAMLFactory()) }
58+
5259
fun readWorkspaceConfig(configPath: Path): WorkspaceConfig = mapParsingErrors(configPath) {
5360
val config = configPath.readText()
5461
if (config.isBlank()) return@mapParsingErrors WorkspaceConfig()
62+
validateWorkspaceConfigKeys(configPath, config)
5563
MaestroFlowParser.parseWorkspaceConfig(configPath, config)
5664
}
5765

66+
private fun validateWorkspaceConfigKeys(configPath: Path, config: String) {
67+
val unknownKeys = findUnknownWorkspaceConfigKeys(config) ?: return
68+
if (unknownKeys.isNotEmpty()) {
69+
logger.warn(
70+
"Config file '{}' contains unknown keys: {}. Valid keys are: {}",
71+
configPath.absolutePathString(),
72+
unknownKeys,
73+
WorkspaceConfig.KNOWN_KEYS.sorted(),
74+
)
75+
}
76+
}
77+
78+
/**
79+
* Returns the list of top-level keys in the config that are not recognized WorkspaceConfig properties,
80+
* or null if the content cannot be parsed as a YAML map.
81+
*/
82+
internal fun findUnknownWorkspaceConfigKeys(config: String): List<Any?>? {
83+
val topLevelKeys = try {
84+
YAML_MAPPER.readValue(config, Map::class.java)?.keys ?: return null
85+
} catch (e: Exception) {
86+
return null
87+
}
88+
return topLevelKeys.filter { it !in WorkspaceConfig.KNOWN_KEYS }
89+
}
90+
5891
// Files to watch for changes. Includes any referenced files.
5992
fun getWatchFiles(flowPath: Path): List<Path> = mapParsingErrors(flowPath) {
6093
MaestroFlowParser.parseWatchFiles(flowPath)

maestro-orchestra/src/test/java/maestro/orchestra/workspace/WorkspaceExecutionPlannerTest.kt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,24 @@ internal class WorkspaceExecutionPlannerTest {
359359
)
360360
}
361361

362+
@Test
363+
internal fun `018 - Additional config files in workspace are not treated as flows`() {
364+
// When
365+
val plan = WorkspaceExecutionPlanner.plan(
366+
input = paths("/workspaces/018_additional_config_files"),
367+
includeTags = listOf(),
368+
excludeTags = listOf(),
369+
config = null,
370+
)
371+
372+
// Then - regression_config.yaml and platform_settings.yaml should be excluded, only flow files should be included
373+
assertThat(plan.flowsToRun).containsExactly(
374+
path("/workspaces/018_additional_config_files/flowA.yaml"),
375+
path("/workspaces/018_additional_config_files/flowB.yaml"),
376+
)
377+
}
378+
379+
362380
private fun path(path: String): Path? {
363381
val clazz = WorkspaceExecutionPlannerTest::class.java
364382
val resource = clazz.getResource(path)?.toURI()

maestro-orchestra/src/test/java/maestro/orchestra/yaml/YamlCommandReaderTest.kt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,41 @@ internal class YamlCommandReaderTest {
826826
}
827827

828828

829+
@Test
830+
fun `findUnknownWorkspaceConfigKeys returns empty for valid keys`() {
831+
val config = """
832+
flows:
833+
- "*"
834+
includeTags:
835+
- smoke
836+
""".trimIndent()
837+
assertThat(YamlCommandReader.findUnknownWorkspaceConfigKeys(config)).isEmpty()
838+
}
839+
840+
@Test
841+
fun `findUnknownWorkspaceConfigKeys detects unknown keys`() {
842+
val config = """
843+
flows:
844+
- "*"
845+
tapOn: some button
846+
invalidKey: true
847+
""".trimIndent()
848+
assertThat(YamlCommandReader.findUnknownWorkspaceConfigKeys(config))
849+
.containsExactly("tapOn", "invalidKey")
850+
}
851+
852+
@Test
853+
fun `findUnknownWorkspaceConfigKeys returns null for malformed yaml`() {
854+
val config = "not: valid: yaml: ["
855+
assertThat(YamlCommandReader.findUnknownWorkspaceConfigKeys(config)).isNull()
856+
}
857+
858+
@Test
859+
fun `findUnknownWorkspaceConfigKeys returns null for non-map yaml`() {
860+
val config = "- launchApp"
861+
assertThat(YamlCommandReader.findUnknownWorkspaceConfigKeys(config)).isNull()
862+
}
863+
829864
private fun commands(vararg commands: Command): List<MaestroCommand> =
830865
commands.map(::MaestroCommand).toList()
831866
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
includeTags:
2+
- included
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
appId: com.example.app
2+
tags:
3+
- included
4+
---
5+
- launchApp
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
appId: com.example.app
2+
tags:
3+
- included
4+
---
5+
- launchApp
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
platform:
2+
ios:
3+
disableAnimations: true
4+
android:
5+
disableAnimations: true
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
platform:
2+
ios:
3+
disableAnimations: true
4+
android:
5+
disableAnimations: true

0 commit comments

Comments
 (0)