diff --git a/maestro-client/src/main/java/maestro/KeyCode.kt b/maestro-client/src/main/java/maestro/KeyCode.kt index 9873e6ebf8..83c8f90499 100644 --- a/maestro-client/src/main/java/maestro/KeyCode.kt +++ b/maestro-client/src/main/java/maestro/KeyCode.kt @@ -1,5 +1,13 @@ package maestro +/** + * A key `pressKey` can press. [description] is the word written in YAML, matched case-insensitively by + * [getByName]; the schema derived from the parser reads it through `@YamlValues(spelledBy = "description")`. + * + * The spelling deliberately does NOT live in a `@JsonProperty` on each constant. Jackson serializes this + * enum as `PressKeyCommand.code` on the MaestroCommand wire, where the constant name is what is written + * and read back, so renaming it there would break every command already persisted or in flight. + */ enum class KeyCode( val description: String, ) { diff --git a/maestro-orchestra-models/src/main/java/maestro/orchestra/Commands.kt b/maestro-orchestra-models/src/main/java/maestro/orchestra/Commands.kt index dd6680284a..fed2ced565 100644 --- a/maestro-orchestra-models/src/main/java/maestro/orchestra/Commands.kt +++ b/maestro-orchestra-models/src/main/java/maestro/orchestra/Commands.kt @@ -1177,9 +1177,15 @@ data class StopRecordingCommand( } } -enum class AirplaneValue { - Enable, - Disable, +/** + * [yamlValue] is the word written in YAML. It is deliberately NOT a `@JsonProperty` on each constant: + * Jackson serializes this enum as `SetAirplaneModeCommand.value` on the MaestroCommand wire, where the + * constant name is what is written and read back, so renaming it there would break every command already + * persisted or in flight. The schema reads the word through `@YamlValues(spelledBy = "yamlValue")`. + */ +enum class AirplaneValue(val yamlValue: String) { + Enable("enabled"), + Disable("disabled"), } data class SetAirplaneModeCommand( @@ -1210,9 +1216,15 @@ data class ToggleAirplaneModeCommand( } } -enum class DarkModeValue { - Enable, - Disable, +/** + * [yamlValue] is the word written in YAML. It is deliberately NOT a `@JsonProperty` on each constant: + * Jackson serializes this enum as `SetDarkModeCommand.value` on the MaestroCommand wire, where the + * constant name is what is written and read back, so renaming it there would break every command already + * persisted or in flight. The schema reads the word through `@YamlValues(spelledBy = "yamlValue")`. + */ +enum class DarkModeValue(val yamlValue: String) { + Enable("enabled"), + Disable("disabled"), } data class SetDarkModeCommand( diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/MaestroFlowParser.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/MaestroFlowParser.kt index 5a87403bbd..601a24996d 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/MaestroFlowParser.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/MaestroFlowParser.kt @@ -137,7 +137,11 @@ private fun String.indentWidth(): Int { // Each lambda receives a YamlFluentCommand pre-populated with sourceInfo, and // fills in the matching command field via copy(). -private val stringCommands = mapOf YamlFluentCommand>( +// +// Internal rather than private: this map is the only record of which commands may be written as a +// bare string (`- back`), and maestro.orchestra.yaml.schema.FlowCommandSchema reads its keys so the +// published schema cannot drift from what the parser accepts. +internal val stringCommands = mapOf YamlFluentCommand>( "launchApp" to { it.copy(launchApp = YamlLaunchApp( appId = null, clearState = null, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlAddMedia.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlAddMedia.kt index 9d19832588..5599990976 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlAddMedia.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlAddMedia.kt @@ -1,7 +1,9 @@ package maestro.orchestra.yaml import com.fasterxml.jackson.annotation.JsonCreator +import maestro.orchestra.yaml.schema.YamlRequiresOneOf +@YamlRequiresOneOf("files") data class YamlAddMedia( val files: List? = null, val label: String? = null, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlElementSelector.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlElementSelector.kt index 1b090441bb..57fee6798a 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlElementSelector.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlElementSelector.kt @@ -20,6 +20,8 @@ package maestro.orchestra.yaml import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import maestro.orchestra.ElementTrait +import maestro.orchestra.yaml.schema.YamlValues @JsonDeserialize(`as` = YamlElementSelector::class) data class YamlElementSelector( @@ -40,6 +42,8 @@ data class YamlElementSelector( val rightOf: YamlElementSelectorUnion? = null, val containsChild: YamlElementSelectorUnion? = null, val containsDescendants: List? = null, + /** One or more trait names separated by spaces; each is looked up in [ElementTrait]. */ + @YamlValues(ElementTrait::class) val traits: String? = null, val index: String? = null, val enabled: Boolean? = null, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlExtendedWaitUntil.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlExtendedWaitUntil.kt index 07e3376765..0eafe17190 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlExtendedWaitUntil.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlExtendedWaitUntil.kt @@ -1,5 +1,8 @@ package maestro.orchestra.yaml +import maestro.orchestra.yaml.schema.YamlRequiresOneOf + +@YamlRequiresOneOf("visible", "notVisible") data class YamlExtendedWaitUntil( val visible: YamlElementSelectorUnion? = null, val notVisible: YamlElementSelectorUnion? = null, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlFluentCommand.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlFluentCommand.kt index 2c77e3f25d..57c479e2bc 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlFluentCommand.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlFluentCommand.kt @@ -77,6 +77,7 @@ import maestro.orchestra.ToggleDarkModeCommand import maestro.orchestra.TravelCommand import maestro.orchestra.WaitForAnimationToEndCommand import maestro.orchestra.error.InvalidFlowFile +import maestro.orchestra.yaml.schema.YamlValues import maestro.orchestra.error.MediaFileNotFound import maestro.orchestra.error.SyntaxError import maestro.orchestra.util.Env.withEnv @@ -119,9 +120,9 @@ data class YamlFluentCommand( val setPermissions: YamlSetPermissions? = null, val swipe: YamlSwipe? = null, val openLink: YamlOpenLink? = null, - val openBrowser: String? = null, val pressKey: YamlPressKey? = null, val eraseText: YamlEraseText? = null, + @YamlValues(YamlNavigationAction::class, spelledBy = "yamlValue") val action: String? = null, val takeScreenshot: YamlTakeScreenshot? = null, val extendedWaitUntil: YamlExtendedWaitUntil? = null, @@ -284,13 +285,13 @@ data class YamlFluentCommand( eraseText != null -> listOf(eraseCommand(eraseText)) action != null -> listOf( - when (action) { - "back" -> MaestroCommand(BackPressCommand()) - "hideKeyboard" -> MaestroCommand(HideKeyboardCommand()) - "scroll" -> MaestroCommand(ScrollCommand()) - "clearKeychain" -> MaestroCommand(ClearKeychainCommand()) - "pasteText" -> MaestroCommand(PasteTextCommand()) - else -> error("Unknown navigation target: $action") + when (YamlNavigationAction.entries.firstOrNull { it.yamlValue == action }) { + YamlNavigationAction.Back -> MaestroCommand(BackPressCommand()) + YamlNavigationAction.HideKeyboard -> MaestroCommand(HideKeyboardCommand()) + YamlNavigationAction.Scroll -> MaestroCommand(ScrollCommand()) + YamlNavigationAction.ClearKeychain -> MaestroCommand(ClearKeychainCommand()) + YamlNavigationAction.PasteText -> MaestroCommand(PasteTextCommand()) + null -> error("Unknown navigation target: $action") } ) @@ -921,12 +922,6 @@ data class YamlFluentCommand( } is YamlSwipeElement -> return swipeElementCommand(swipe) - else -> { - throw IllegalStateException( - "Provide swipe direction UP, DOWN, RIGHT OR LEFT or by giving explicit " + - "start and end coordinates." - ) - } } } diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlNavigationAction.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlNavigationAction.kt new file mode 100644 index 0000000000..fa14b9ce07 --- /dev/null +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlNavigationAction.kt @@ -0,0 +1,15 @@ +package maestro.orchestra.yaml + +/** + * The words `action` accepts. `action` is a legacy spelling of commands that all exist under their own + * names — `action: back` is `- back` — and it is a plain `String` so that it keeps parsing as one, but + * the set of words it takes is closed. Holding them here rather than as literals in a `when` is what + * lets the schema advertise them; [yamlValue] is the word, the constant name is not on any wire. + */ +enum class YamlNavigationAction(val yamlValue: String) { + Back("back"), + HideKeyboard("hideKeyboard"), + Scroll("scroll"), + ClearKeychain("clearKeychain"), + PasteText("pasteText"), +} diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlPressKey.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlPressKey.kt index 94b5ece716..69cc7d5981 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlPressKey.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlPressKey.kt @@ -1,8 +1,11 @@ package maestro.orchestra.yaml import com.fasterxml.jackson.annotation.JsonCreator +import maestro.KeyCode +import maestro.orchestra.yaml.schema.YamlValues data class YamlPressKey ( + @YamlValues(KeyCode::class, spelledBy = "description") val key: String, val label: String? = null, val optional: Boolean = false, @@ -10,7 +13,7 @@ data class YamlPressKey ( companion object { @JvmStatic @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - fun parse(key: String) = YamlPressKey( + fun parse(@YamlValues(KeyCode::class, spelledBy = "description") key: String) = YamlPressKey( key = key, ) } diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRetry.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRetry.kt index 8243d3cf8b..52b3e85bf9 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRetry.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRetry.kt @@ -1,5 +1,8 @@ package maestro.orchestra.yaml +import maestro.orchestra.yaml.schema.YamlRequiresOneOf + +@YamlRequiresOneOf("file", "commands", exclusive = true) data class YamlRetryCommand( val maxRetries: String? = null, val file: String? = null, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRunFlow.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRunFlow.kt index 6ee63bf5c9..265c589066 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRunFlow.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlRunFlow.kt @@ -1,7 +1,9 @@ package maestro.orchestra.yaml import com.fasterxml.jackson.annotation.JsonCreator +import maestro.orchestra.yaml.schema.YamlRequiresOneOf +@YamlRequiresOneOf("file", "commands", exclusive = true) data class YamlRunFlow( val file: String? = null, val `when`: YamlCondition? = null, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetAirplaneMode.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetAirplaneMode.kt index b67a900b5c..545b7f005e 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetAirplaneMode.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetAirplaneMode.kt @@ -7,10 +7,13 @@ import com.fasterxml.jackson.databind.DeserializationContext import com.fasterxml.jackson.databind.JsonDeserializer import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import com.fasterxml.jackson.databind.node.TextNode import maestro.orchestra.AirplaneValue +import maestro.orchestra.yaml.schema.YamlValues @JsonDeserialize(using = YamlSetAirplaneModeDeserializer::class) data class YamlSetAirplaneMode( + @YamlValues(AirplaneValue::class, spelledBy = "yamlValue") val value: AirplaneValue, val label: String? = null, val optional: Boolean = false, @@ -18,37 +21,47 @@ data class YamlSetAirplaneMode( companion object { @JvmStatic @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - fun parse(value: AirplaneValue): YamlSetAirplaneMode { + fun parse(@YamlValues(AirplaneValue::class, spelledBy = "yamlValue") value: AirplaneValue): YamlSetAirplaneMode { return YamlSetAirplaneMode(value) } } } +/** + * Accepts `setAirplaneMode: enabled` and the object form carrying `value`, `label` and `optional`. + * + * The accepted vocabulary is not spelled out here: it lives on [AirplaneValue] as `yamlValue`, read both by + * this deserializer and by the schema derived from these types, so the two cannot disagree. It is + * deliberately not a `@JsonProperty` on each constant -- that name is the MaestroCommand wire format. + */ class YamlSetAirplaneModeDeserializer : JsonDeserializer() { override fun deserialize(parser: JsonParser, ctxt: DeserializationContext): YamlSetAirplaneMode { - val mapper = (parser.codec as ObjectMapper) + val mapper = parser.codec as ObjectMapper val root: TreeNode = mapper.readTree(parser) - val input = root.fieldNames().asSequence().toList() - val label = getLabel(root) - when { - input.contains("value") -> { - val parsedValue = root.get("value").toString().replace("\"", "") - val returnValue = when (parsedValue) { - "enabled" -> AirplaneValue.Enable - "disabled" -> AirplaneValue.Disable - else -> throwInvalidInputException(input) - } - return YamlSetAirplaneMode(returnValue, label) - } - (root.isValueNode && root.toString().contains("enabled")) -> { - return YamlSetAirplaneMode(AirplaneValue.Enable, label) - } - (root.isValueNode && root.toString().contains("disabled")) -> { - return YamlSetAirplaneMode(AirplaneValue.Disable, label) - } - else -> throwInvalidInputException(input) + + if (root.isValueNode) { + return YamlSetAirplaneMode(toAirplaneValue(root)) } + + val valueNode = root.get("value") + ?: throwInvalidInputException(root.fieldNames().asSequence().toList()) + + return YamlSetAirplaneMode( + value = toAirplaneValue(valueNode), + label = root.get("label")?.let { mapper.convertValue(it, String::class.java) }, + optional = root.get("optional")?.let { mapper.convertValue(it, Boolean::class.java) } ?: false, + ) + } + + /** + * Looks the word up on [AirplaneValue] rather than letting Jackson convert it, so the constant names stay + * the MaestroCommand wire format while YAML keeps its own spelling. Still derived from the enum, so + * the parser and the schema cannot disagree. + */ + private fun toAirplaneValue(node: TreeNode): AirplaneValue { + val text = (node as? TextNode)?.textValue() ?: throwInvalidInputException(listOf(node.toString())) + return AirplaneValue.entries.firstOrNull { it.yamlValue == text } ?: throwInvalidInputException(listOf(text)) } private fun throwInvalidInputException(input: List): Nothing { @@ -60,13 +73,4 @@ class YamlSetAirplaneModeDeserializer : JsonDeserializer() "It seems you provided invalid input with: $input" ) } - - private fun getLabel(root: TreeNode): String? { - return if (root.path("label").isMissingNode) { - null - } else { - root.path("label").toString().replace("\"", "") - } - } - } diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetDarkMode.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetDarkMode.kt index 9a7fbe3887..5a80116363 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetDarkMode.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetDarkMode.kt @@ -7,10 +7,13 @@ import com.fasterxml.jackson.databind.DeserializationContext import com.fasterxml.jackson.databind.JsonDeserializer import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import com.fasterxml.jackson.databind.node.TextNode import maestro.orchestra.DarkModeValue +import maestro.orchestra.yaml.schema.YamlValues @JsonDeserialize(using = YamlSetDarkModeDeserializer::class) data class YamlSetDarkMode( + @YamlValues(DarkModeValue::class, spelledBy = "yamlValue") val value: DarkModeValue, val label: String? = null, val optional: Boolean = false, @@ -18,37 +21,47 @@ data class YamlSetDarkMode( companion object { @JvmStatic @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - fun parse(value: DarkModeValue): YamlSetDarkMode { + fun parse(@YamlValues(DarkModeValue::class, spelledBy = "yamlValue") value: DarkModeValue): YamlSetDarkMode { return YamlSetDarkMode(value) } } } +/** + * Accepts `setDarkMode: enabled` and the object form carrying `value`, `label` and `optional`. + * + * The accepted vocabulary is not spelled out here: it lives on [DarkModeValue] as `yamlValue`, read both by + * this deserializer and by the schema derived from these types, so the two cannot disagree. It is + * deliberately not a `@JsonProperty` on each constant -- that name is the MaestroCommand wire format. + */ class YamlSetDarkModeDeserializer : JsonDeserializer() { override fun deserialize(parser: JsonParser, ctxt: DeserializationContext): YamlSetDarkMode { - val mapper = (parser.codec as ObjectMapper) + val mapper = parser.codec as ObjectMapper val root: TreeNode = mapper.readTree(parser) - val input = root.fieldNames().asSequence().toList() - val label = getLabel(root) - when { - input.contains("value") -> { - val parsedValue = root.get("value").toString().replace("\"", "") - val returnValue = when (parsedValue) { - "enabled" -> DarkModeValue.Enable - "disabled" -> DarkModeValue.Disable - else -> throwInvalidInputException(input) - } - return YamlSetDarkMode(returnValue, label) - } - (root.isValueNode && root.toString().contains("enabled")) -> { - return YamlSetDarkMode(DarkModeValue.Enable, label) - } - (root.isValueNode && root.toString().contains("disabled")) -> { - return YamlSetDarkMode(DarkModeValue.Disable, label) - } - else -> throwInvalidInputException(input) + + if (root.isValueNode) { + return YamlSetDarkMode(toDarkModeValue(root)) } + + val valueNode = root.get("value") + ?: throwInvalidInputException(root.fieldNames().asSequence().toList()) + + return YamlSetDarkMode( + value = toDarkModeValue(valueNode), + label = root.get("label")?.let { mapper.convertValue(it, String::class.java) }, + optional = root.get("optional")?.let { mapper.convertValue(it, Boolean::class.java) } ?: false, + ) + } + + /** + * Looks the word up on [DarkModeValue] rather than letting Jackson convert it, so the constant names stay + * the MaestroCommand wire format while YAML keeps its own spelling. Still derived from the enum, so + * the parser and the schema cannot disagree. + */ + private fun toDarkModeValue(node: TreeNode): DarkModeValue { + val text = (node as? TextNode)?.textValue() ?: throwInvalidInputException(listOf(node.toString())) + return DarkModeValue.entries.firstOrNull { it.yamlValue == text } ?: throwInvalidInputException(listOf(text)) } private fun throwInvalidInputException(input: List): Nothing { @@ -61,13 +74,4 @@ class YamlSetDarkModeDeserializer : JsonDeserializer() { "It seems you provided invalid input with: $input" ) } - - private fun getLabel(root: TreeNode): String? { - return if (root.path("label").isMissingNode) { - null - } else { - root.path("label").toString().replace("\"", "") - } - } - } diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetOrientation.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetOrientation.kt index f3252cec9e..c841f791fb 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetOrientation.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSetOrientation.kt @@ -9,9 +9,11 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.fasterxml.jackson.databind.node.TextNode import maestro.device.DeviceOrientation +import maestro.orchestra.yaml.schema.YamlValues @JsonDeserialize(using = YamlSetOrientationDeserializer::class) data class YamlSetOrientation( + @YamlValues(DeviceOrientation::class) val orientation: String, val label: String? = null, val optional: Boolean = false, @@ -19,7 +21,7 @@ data class YamlSetOrientation( companion object { @JvmStatic @JsonCreator(mode = JsonCreator.Mode.DELEGATING) - fun parse(orientation: String) = YamlSetOrientation( + fun parse(@YamlValues(DeviceOrientation::class) orientation: String) = YamlSetOrientation( orientation = orientation, ) } diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSwipe.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSwipe.kt index 5ffac7b5ef..4c574ca314 100644 --- a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSwipe.kt +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/YamlSwipe.kt @@ -8,16 +8,18 @@ import com.fasterxml.jackson.databind.JsonDeserializer import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.annotation.JsonDeserialize import maestro.SwipeDirection +import maestro.orchestra.yaml.schema.YamlVariant import maestro.directionValueOfOrNull @JsonDeserialize(using = YamlSwipeDeserializer::class) -interface YamlSwipe { +sealed interface YamlSwipe { val duration: Long val label: String? val optional: Boolean val waitToSettleTimeoutMs: Int? } +@YamlVariant("byDirection") data class YamlSwipeDirection( val direction: SwipeDirection, override val duration: Long = DEFAULT_DURATION_IN_MILLIS, @@ -26,6 +28,7 @@ data class YamlSwipeDirection( override val waitToSettleTimeoutMs: Int? = null, ) : YamlSwipe +@YamlVariant("byCoordinates") data class YamlCoordinateSwipe( val start: String, val end: String, @@ -35,6 +38,7 @@ data class YamlCoordinateSwipe( override val waitToSettleTimeoutMs: Int? = null, ) : YamlSwipe +@YamlVariant("byRelativeCoordinates") data class YamlRelativeCoordinateSwipe( val start: String, val end: String, @@ -45,6 +49,7 @@ data class YamlRelativeCoordinateSwipe( ) : YamlSwipe @JsonDeserialize(`as` = YamlSwipeElement::class) +@YamlVariant("byElement") data class YamlSwipeElement( @JsonFormat(with = [JsonFormat.Feature.ACCEPT_CASE_INSENSITIVE_PROPERTIES]) val direction: SwipeDirection, diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/FlowCommandSchema.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/FlowCommandSchema.kt new file mode 100644 index 0000000000..29f206304f --- /dev/null +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/FlowCommandSchema.kt @@ -0,0 +1,329 @@ +package maestro.orchestra.yaml.schema + +import com.fasterxml.jackson.annotation.JsonAlias +import com.fasterxml.jackson.annotation.JsonCreator +import com.fasterxml.jackson.annotation.JsonInclude +import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.ObjectMapper +import maestro.orchestra.yaml.YamlElementSelector +import maestro.orchestra.yaml.YamlElementSelectorUnion +import maestro.orchestra.yaml.YamlFluentCommand +import maestro.orchestra.yaml.stringCommands +import kotlin.reflect.KClass +import kotlin.reflect.KFunction +import kotlin.reflect.KParameter +import kotlin.reflect.full.companionObject +import kotlin.reflect.full.findAnnotation +import kotlin.reflect.full.memberProperties +import kotlin.reflect.full.primaryConstructor +import kotlin.reflect.jvm.javaField + +/** How a YAML value is written. */ +enum class ArgumentKind { + STRING, + NUMBER, + BOOLEAN, + + /** A fixed vocabulary; the accepted words are in [ArgumentSchema.values]. */ + ENUM, + + /** A list of values. */ + ARRAY, + + /** An element selector — a string, or a map of [FlowCommandSchema.selectorArguments]. */ + SELECTOR, + + /** A map of further arguments. */ + OBJECT, + + /** Any scalar; the parser coerces whatever it is given (`inputText: 42`). */ + ANY, +} + +data class ArgumentSchema( + val name: String, + val kind: ArgumentKind, + val required: Boolean, + /** + * The accepted YAML words, for [ArgumentKind.ENUM] arguments only. Read from the `@JsonProperty` + * wire name of each enum constant, so this cannot disagree with what the parser accepts. + */ + val values: List? = null, + /** + * Other spellings the parser also accepts for this argument, from `@JsonAlias`. Null when there are + * none. [name] is the spelling to write; these are the ones a consumer must not reject. + */ + val aliases: List? = null, +) + +/** One of the alternative shapes a command accepts, e.g. `swipe` by direction vs. by coordinates. */ +data class VariantSchema( + val name: String, + val arguments: List, +) + +/** + * A rule the parser enforces across several of a command's arguments, from [YamlRequiresOneOf]. The + * arguments are individually [ArgumentSchema.required]`= false` — each may be omitted, but not all of + * them at once, and when [exclusive] not more than one of them at a time. + */ +data class OneOfSchema( + val names: List, + val exclusive: Boolean, +) + +/** The single-value form of a command, e.g. `openLink: https://example.com`. */ +data class ShorthandSchema( + val kind: ArgumentKind, + val values: List? = null, +) + +data class CommandSchema( + /** The name as written in YAML, e.g. `tapOn`. */ + val name: String, + + /** The command's value is an element selector, e.g. `tapOn: "Login"`. */ + val selector: Boolean, + + /** The command may be written with no value at all, e.g. `- back`. */ + val bareString: Boolean, + + /** Present when the command also accepts a single value instead of a map. */ + val shorthand: ShorthandSchema?, + + /** + * The command's named arguments, minus [FlowCommandSchema.commonArguments]. When [variants] is + * non-empty these are only the arguments every variant shares. + */ + val arguments: List, + + /** Non-empty when the command accepts several alternative shapes. */ + val variants: List, + + /** The command's one-of rule, or null when it declares none. */ + val requiredOneOf: OneOfSchema? = null, +) + +/** + * The Maestro flow-command surface, derived by reflection from the very types the YAML parser uses: + * [YamlFluentCommand]'s constructor, each command's `Yaml*` data class, and the `stringCommands` map + * the parser consults for bare-string commands. Nothing here is hand-maintained, so it cannot drift + * from the parser. + * + * This is an API, not an artifact: any JVM consumer already depending on `dev.mobile:maestro-orchestra` + * calls [commands] (or [asJson] for the same thing as JSON). There is no schema file to publish, no URL + * to fetch and no path to hardcode — the surface ships inside the jar and moves with the dependency. + */ +object FlowCommandSchema { + + /** + * The shape of the document [asJson] produces, not the command surface it describes. Bumped when a + * field is added or renamed here, so a consumer diffing published JSON can tell a schema-format + * change from Maestro gaining a command. + */ + const val VERSION = 1 + + + /** Arguments every command inherits. Consumers render these once rather than per command. */ + val commonArguments: List = listOf( + ArgumentSchema("label", ArgumentKind.STRING, required = false), + ArgumentSchema("optional", ArgumentKind.BOOLEAN, required = false), + ) + + private val commonArgumentNames = commonArguments.map { it.name }.toSet() + + /** + * The arguments an [ArgumentKind.SELECTOR] value accepts in its map form. `tapOn`, `assertVisible` + * and their siblings take one of these instead of named arguments of their own, so a consumer that + * only reads [CommandSchema.arguments] sees nothing at all for the most-used commands in Maestro. + * Rendered once here rather than repeated under every selector command. + */ + val selectorArguments: List by lazy { argumentsOf(YamlElementSelector::class) } + + fun commands(): List { + return YamlFluentCommand::class.primaryConstructor!!.parameters + .mapNotNull { parameter -> + // `_sourceInfo` and friends are parser bookkeeping, not commands. + val name = parameter.name?.takeUnless { it.startsWith("_") } ?: return@mapNotNull null + val type = parameter.type.classifier as? KClass<*> ?: return@mapNotNull null + schemaOf(name, type, parameter) + } + } + + fun asJson(): String { + val document = mapOf( + "version" to VERSION, + "commonArguments" to commonArguments, + "selectorArguments" to selectorArguments, + "commands" to commands(), + ) + return ObjectMapper() + .setSerializationInclusion(JsonInclude.Include.NON_NULL) + .writerWithDefaultPrettyPrinter() + .writeValueAsString(document) + } + + /** + * Derives one command from the type [YamlFluentCommand] declares for it. Visible to tests so the + * derivation can be driven with a `Yaml*` shape that does not exist yet -- the only way to ask + * what happens when a command is added or an argument renamed or retyped. + */ + internal fun schemaOf(name: String, type: KClass<*>, declaredBy: KParameter? = null): CommandSchema { + val bareString = name in stringCommands + val kind = kindOf(type) + + if (kind == ArgumentKind.SELECTOR) { + return CommandSchema(name, selector = true, bareString, null, emptyList(), emptyList()) + } + + // A command whose value is a plain scalar, e.g. `openLink: https://example.com`. It may still + // have a closed vocabulary its own type cannot express -- `action` is a String on + // YamlFluentCommand -- so the declaring parameter is consulted, not only the type. + if (kind != ArgumentKind.OBJECT) { + val values = declaredBy?.let { valuesOf(it, type) } ?: enumValuesOf(type) + val shorthand = ShorthandSchema(if (values != null) ArgumentKind.ENUM else kind, values) + return CommandSchema(name, selector = false, bareString, shorthand, emptyList(), emptyList()) + } + + val subclasses = type.sealedSubclasses + val shorthand = shorthandOf(type) + if (subclasses.isEmpty()) { + return CommandSchema(name, false, bareString, shorthand, argumentsOf(type), emptyList(), requiredOneOf(type)) + } + + // A sealed command type is one command with alternative shapes. Arguments every shape accepts + // are hoisted onto the command; each variant keeps only what distinguishes it. + val perVariant = subclasses.associateWith { argumentsOf(it) } + val shared = perVariant.values + .reduce { acc, arguments -> acc.filter { it in arguments } } + return CommandSchema( + name = name, + selector = false, + bareString = bareString, + shorthand = shorthand, + arguments = shared, + // sealedSubclasses has no documented order, so name them in one, or a compiler upgrade + // silently reorders every published document. + variants = perVariant.map { (subclass, arguments) -> + VariantSchema(variantNameOf(subclass), arguments - shared.toSet()) + }.sortedBy { it.name }, + requiredOneOf = requiredOneOf(type), + ) + } + + private fun argumentsOf(type: KClass<*>): List { + return type.primaryConstructor?.parameters.orEmpty() + .mapNotNull { parameter -> + val name = wireNameOf(parameter, type)?.takeUnless { it in commonArgumentNames } ?: return@mapNotNull null + val argumentType = parameter.type.classifier as? KClass<*> + val values = valuesOf(parameter, argumentType) + ArgumentSchema( + name = name, + kind = if (values != null) ArgumentKind.ENUM else kindOf(argumentType), + // Required in YAML means the parser cannot fill it in: no Kotlin default AND not + // nullable. A nullable parameter without a default still deserializes when the key + // is absent, because Jackson supplies null -- `- launchApp` alone is valid YAML. + required = !parameter.isOptional && !parameter.type.isMarkedNullable, + values = values, + aliases = annotationOf(JsonAlias::class, parameter, type)?.value?.toList()?.ifEmpty { null }, + ) + } + } + + /** + * The name [subclass] is published under. Declared with [YamlVariant] rather than derived, so a + * consumer never sees a Kotlin class name; the fallback exists only to keep the failure legible. + */ + internal fun variantNameOf(subclass: KClass<*>): String = + subclass.findAnnotation()?.name ?: subclass.simpleName!! + + /** The one-of rule [type] declares, or null when it declares none. */ + private fun requiredOneOf(type: KClass<*>): OneOfSchema? { + val declared = type.findAnnotation() ?: return null + return declared.names.toList().ifEmpty { null }?.let { OneOfSchema(it, declared.exclusive) } + } + + /** + * The key the parser reads this argument from. Jackson keys off `@JsonProperty` when there is one and + * only falls back to the Kotlin parameter name, so the schema has to do the same -- otherwise renaming + * a parameter while keeping its YAML spelling makes the schema advertise a key the parser rejects as + * an unknown property. An empty value means `@JsonProperty` without a name, which is Jackson's own + * "use the default" and leaves the parameter name in place. + */ + private fun wireNameOf(parameter: KParameter, type: KClass<*>): String? = + annotationOf(JsonProperty::class, parameter, type)?.value?.takeUnless { it.isEmpty() } + ?: parameter.name + + /** + * [annotation] as it applies to [parameter], wherever it was written. Kotlin's default target for these + * is the value parameter, but `@field:` and `@get:` are legal and Jackson honours them just the same -- + * reading only the parameter would leave the schema advertising the name the rename moved away from. + */ + private fun annotationOf(annotation: KClass, parameter: KParameter, type: KClass<*>): A? { + parameter.annotations.filterIsInstance(annotation.java).firstOrNull()?.let { return it } + val property = type.memberProperties.firstOrNull { it.name == parameter.name } ?: return null + return property.annotations.filterIsInstance(annotation.java).firstOrNull() + ?: property.getter.annotations.filterIsInstance(annotation.java).firstOrNull() + ?: property.javaField?.getAnnotation(annotation.java) + } + + /** + * The value form a command accepts alongside its map form, read from the `@JsonCreator(DELEGATING)` + * factory the parser already uses for it. + */ + private fun shorthandOf(type: KClass<*>): ShorthandSchema? { + val creator = type.companionObject + ?.members + ?.filterIsInstance>() + ?.firstOrNull { it.findAnnotation()?.mode == JsonCreator.Mode.DELEGATING } + ?: return null + val parameter = creator.parameters.singleOrNull { it.kind == KParameter.Kind.VALUE } ?: return null + val valueType = parameter.type.classifier as? KClass<*> ?: return null + val values = valuesOf(parameter, valueType) + return ShorthandSchema(if (values != null) ArgumentKind.ENUM else kindOf(valueType), values) + } + + /** + * The vocabulary a value accepts: the constants of its own type when that is an enum, or of the enum + * a `String`-typed value names with [YamlValues] because it has to stay a `String` to keep accepting + * `${VAR}`. Null when the value has no closed vocabulary. + */ + private fun valuesOf(parameter: KParameter, type: KClass<*>?): List? { + val declared = parameter.findAnnotation() ?: return type?.let(::enumValuesOf) + return enumValuesOf(declared.of, declared.spelledBy) + } + + private fun kindOf(type: KClass<*>?): ArgumentKind = when { + type == null -> ArgumentKind.OBJECT + type == YamlElementSelectorUnion::class -> ArgumentKind.SELECTOR + type.java.isEnum -> ArgumentKind.ENUM + type == String::class -> ArgumentKind.STRING + type == Int::class || type == Long::class || type == Double::class || type == Float::class -> ArgumentKind.NUMBER + type == Boolean::class -> ArgumentKind.BOOLEAN + type == Any::class -> ArgumentKind.ANY + Collection::class.java.isAssignableFrom(type.java) || type.java.isArray -> ArgumentKind.ARRAY + else -> ArgumentKind.OBJECT + } + + /** + * The YAML words an enum accepts, taken from each constant's `@JsonProperty` and falling back to the + * constant name — or, when [spelledBy] names one, read off that property of each constant instead. + * Null for anything that is not an enum. + */ + private fun enumValuesOf(type: KClass<*>, spelledBy: String = ""): List? { + if (!type.java.isEnum) return null + if (spelledBy.isNotEmpty()) { + val spelling = type.memberProperties.singleOrNull { it.name == spelledBy } + ?: error( + "@YamlValues(of = ${type.simpleName}::class, spelledBy = \"$spelledBy\") names a property " + + "${type.simpleName} does not have. Available: ${type.memberProperties.map { it.name }.sorted()}" + ) + return type.java.enumConstants.map { spelling.getter.call(it).toString() } + } + // enumConstants is declaration order; getFields() is unspecified, so both paths use the former. + return type.java.enumConstants.map { constant -> + val name = (constant as Enum<*>).name + type.java.getField(name).getAnnotation(JsonProperty::class.java)?.value ?: name + } + } +} diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlRequiresOneOf.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlRequiresOneOf.kt new file mode 100644 index 0000000000..1a1c27db75 --- /dev/null +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlRequiresOneOf.kt @@ -0,0 +1,24 @@ +package maestro.orchestra.yaml.schema + +/** + * Names arguments the parser enforces a rule across: at least one must be present, and with + * [exclusive], no more than one. + * + * [FlowCommandSchema] derives `required` from the Kotlin constructor: an argument is required when + * Jackson cannot build the object without it. Some commands accept every argument being absent as far + * as deserialization is concerned and only reject the result later, in `toCommands` — `runFlow` needs + * a `file` or `commands` and refuses both, `extendedWaitUntil` needs `visible` or `notVisible` and + * accepts both. Those rules live inside a function body, where reflection cannot see them, so the + * schema would otherwise publish `runFlow: {}` as a valid command. This annotation is the declaration + * that closes that gap, the way [YamlValues] closes it for a vocabulary the parser checks by hand. + */ +@Target(AnnotationTarget.CLASS) +@Retention(AnnotationRetention.RUNTIME) +annotation class YamlRequiresOneOf( + + /** The argument names, by their YAML spelling. At least one must appear. */ + vararg val names: String, + + /** Whether the parser also rejects more than one of [names] being present. */ + val exclusive: Boolean = false, +) diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlValues.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlValues.kt new file mode 100644 index 0000000000..72648163b7 --- /dev/null +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlValues.kt @@ -0,0 +1,30 @@ +package maestro.orchestra.yaml.schema + +import kotlin.reflect.KClass + +/** + * Names the closed vocabulary a `String`-typed YAML field is validated against, so [FlowCommandSchema] + * can report the field as [ArgumentKind.ENUM] with those words. + * + * Some fields have to stay `String` even though the parser only accepts a fixed set of words: + * `setOrientation` also accepts `${VAR}` interpolation, `selector.traits` holds several words separated + * by spaces, and `pressKey` is a `String` for historical reasons — it does *not* accept interpolation, + * because `KeyCode.getByName` runs before substitution does. Typing them as the enum would break those + * shapes; leaving the vocabulary only inside the parser's lookup would leave the schema blind to it. + * This annotation is the declaration that closes that gap. + */ +@Target(AnnotationTarget.VALUE_PARAMETER) +@Retention(AnnotationRetention.RUNTIME) +annotation class YamlValues( + + /** The enum the parser looks the field's value up in. */ + val of: KClass>, + + /** + * The property holding each constant's YAML spelling, when that is neither the constant's + * `@JsonProperty` wire name nor its name. `KeyCode` is written `Volume Up`, not `VOLUME_UP`, and + * cannot say so with `@JsonProperty` because Jackson also serializes it as `PressKeyCommand.code`, + * where the constant name is the wire name. Empty means the usual rule applies. + */ + val spelledBy: String = "", +) diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlVariant.kt b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlVariant.kt new file mode 100644 index 0000000000..6675bafd27 --- /dev/null +++ b/maestro-orchestra/src/main/java/maestro/orchestra/yaml/schema/YamlVariant.kt @@ -0,0 +1,12 @@ +package maestro.orchestra.yaml.schema + +/** + * The name one alternative shape of a command is published under. + * + * [FlowCommandSchema] would otherwise fall back to the Kotlin class name, putting `YamlSwipeElement` + * into a document a consumer reads and, through it, into generated docs and error messages. These names + * are part of the published surface, so they are declared rather than derived. + */ +@Target(AnnotationTarget.CLASS) +@Retention(AnnotationRetention.RUNTIME) +annotation class YamlVariant(val name: String) diff --git a/maestro-orchestra/src/test/java/maestro/orchestra/yaml/YamlSetModeTest.kt b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/YamlSetModeTest.kt new file mode 100644 index 0000000000..677c69f000 --- /dev/null +++ b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/YamlSetModeTest.kt @@ -0,0 +1,110 @@ +package maestro.orchestra.yaml + +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.google.common.truth.Truth.assertThat +import maestro.orchestra.AirplaneValue +import maestro.orchestra.DarkModeValue +import maestro.orchestra.SetAirplaneModeCommand +import maestro.orchestra.SetDarkModeCommand +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.nio.file.Paths + +/** + * `setDarkMode` and `setAirplaneMode` accept a bare value or a map, and their accepted vocabulary is + * the `yamlValue` of each [DarkModeValue] / [AirplaneValue] constant -- not the constant names, which + * are the MaestroCommand wire format and are pinned separately by the last test here. + */ +class YamlSetModeTest { + + private val flowPath = Paths.get("test.yaml") + + @Test + fun `setDarkMode accepts a bare value`() { + val command = parseSingle("setDarkMode: enabled") + + assertThat(command).isEqualTo(SetDarkModeCommand(DarkModeValue.Enable)) + } + + @Test + fun `setDarkMode accepts the map form`() { + val command = parseSingle( + """ + setDarkMode: + value: disabled + label: "Turn dark mode off" + """.trimIndent() + ) + + assertThat(command).isEqualTo(SetDarkModeCommand(DarkModeValue.Disable, label = "Turn dark mode off")) + } + + /** Regression: the map form read `value` and `label` but silently dropped `optional`. */ + @Test + fun `setDarkMode keeps optional from the map form`() { + val command = parseSingle( + """ + setDarkMode: + value: enabled + optional: true + """.trimIndent() + ) + + assertThat(command).isEqualTo(SetDarkModeCommand(DarkModeValue.Enable, optional = true)) + } + + @Test + fun `setDarkMode rejects an unknown value`() { + assertThrows { parseSingle("setDarkMode: sometimes") } + } + + @Test + fun `setAirplaneMode accepts a bare value`() { + val command = parseSingle("setAirplaneMode: disabled") + + assertThat(command).isEqualTo(SetAirplaneModeCommand(AirplaneValue.Disable)) + } + + @Test + fun `setAirplaneMode keeps optional from the map form`() { + val command = parseSingle( + """ + setAirplaneMode: + value: enabled + optional: true + """.trimIndent() + ) + + assertThat(command).isEqualTo(SetAirplaneModeCommand(AirplaneValue.Enable, optional = true)) + } + + @Test + fun `setAirplaneMode rejects an unknown value`() { + assertThrows { parseSingle("setAirplaneMode: maybe") } + } + + /** + * The YAML word and the MaestroCommand word are different, and only the YAML one is free to move. + * The constant names are what `SetDarkModeCommand.value` / `SetAirplaneModeCommand.value` serialize + * to on the wire the backend persists and the worker sends, so a `@JsonProperty` renaming them there + * would reject every command already stored or in flight. Pinned here because nothing else looks at + * that wire, and the YAML tests above pass either way. + */ + @Test + fun `the MaestroCommand wire keeps the constant names`() { + val mapper = jacksonObjectMapper() + + assertThat(mapper.writeValueAsString(SetDarkModeCommand(DarkModeValue.Enable))) + .contains(""""value":"Enable"""") + assertThat(mapper.readValue(""" {"value":"Disable"} """, SetDarkModeCommand::class.java).value) + .isEqualTo(DarkModeValue.Disable) + + assertThat(mapper.writeValueAsString(SetAirplaneModeCommand(AirplaneValue.Enable))) + .contains(""""value":"Enable"""") + assertThat(mapper.readValue(""" {"value":"Disable"} """, SetAirplaneModeCommand::class.java).value) + .isEqualTo(AirplaneValue.Disable) + } + + private fun parseSingle(command: String) = + MaestroFlowParser.parseCommand(flowPath, "com.example.app", command).single().asCommand() +} diff --git a/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/E2eFlowConformanceTest.kt b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/E2eFlowConformanceTest.kt new file mode 100644 index 0000000000..228f759b11 --- /dev/null +++ b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/E2eFlowConformanceTest.kt @@ -0,0 +1,92 @@ +package maestro.orchestra.yaml.schema + +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory +import com.google.common.truth.Truth.assertThat +import maestro.orchestra.yaml.stringCommands +import org.junit.jupiter.api.Test +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import kotlin.io.path.extension +import kotlin.io.path.name +import kotlin.streams.asSequence + +/** + * Every flow in `e2e/` is written by hand, for real apps, by people who were not thinking about the + * schema. Nothing parses them outside the E2E job, which needs devices and takes half an hour, so + * this is the only fast check that the surface the schema publishes is the surface real flows use. + */ +class E2eFlowConformanceTest { + + private val mapper = ObjectMapper(YAMLFactory()) + private val schema = FlowCommandSchema.commands().associateBy { it.name } + + @Test + fun `every command an e2e flow writes is a command the schema declares`() { + val unknown = sortedSetOf() + + for (file in flowFiles()) { + for ((command, _) in commandsIn(file).orEmpty()) { + if (command !in schema && command !in stringCommands) unknown += "$command (${file.name})" + } + } + + assertThat(unknown).isEmpty() + } + + @Test + fun `every argument an e2e flow writes is an argument the schema declares`() { + val unknown = sortedSetOf() + val common = FlowCommandSchema.commonArguments.map { it.name }.toSet() + val selector = FlowCommandSchema.selectorArguments.map { it.name }.toSet() + + for (file in flowFiles()) { + for ((command, arguments) in commandsIn(file).orEmpty()) { + val declared = schema[command] ?: continue + val known = buildSet { + addAll(common) + declared.arguments.forEach { add(it.name); addAll(it.aliases.orEmpty()) } + declared.variants.forEach { v -> v.arguments.forEach { add(it.name); addAll(it.aliases.orEmpty()) } } + if (declared.selector) addAll(selector) + } + arguments.filterNot { it in known }.forEach { unknown += "$command.$it (${file.name})" } + } + } + + assertThat(unknown).isEmpty() + } + + /** Every YAML under e2e/, minus the workspace/config files that are not flows. */ + private fun flowFiles(): List { + val root = Paths.get("..", "e2e").toAbsolutePath().normalize() + if (!Files.isDirectory(root)) return emptyList() + return Files.walk(root).asSequence() + .filter { Files.isRegularFile(it) && it.extension in setOf("yaml", "yml") } + .filterNot { it.name == "config.yaml" || it.name == "workspace.yaml" } + .sorted() + .toList() + } + + /** The `command name -> argument names` pairs a flow file writes, or null when it is not a flow. */ + private fun commandsIn(file: Path): List>>? { + val documents = runCatching { + mapper.factory.createParser(file.toFile()).use { parser -> + mapper.readValues(parser, JsonNode::class.java).readAll() + } + }.getOrNull() ?: return null + val commands = documents.lastOrNull { it.isArray } ?: return null + return commands.mapNotNull { node -> + when { + node.isTextual -> node.asText() to emptyList() + node.isObject && node.size() == 1 -> { + val name = node.fieldNames().next() + val value = node.get(name) + name to if (value != null && value.isObject) value.fieldNames().asSequence().toList() else emptyList() + } + else -> null + } + } + } +} diff --git a/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/FlowCommandSchemaEvolutionTest.kt b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/FlowCommandSchemaEvolutionTest.kt new file mode 100644 index 0000000000..e1ef7e86b0 --- /dev/null +++ b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/FlowCommandSchemaEvolutionTest.kt @@ -0,0 +1,359 @@ +package maestro.orchestra.yaml.schema + +import com.fasterxml.jackson.annotation.JsonAlias +import com.fasterxml.jackson.annotation.JsonCreator +import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.exc.MismatchedInputException +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.google.common.truth.Truth.assertThat +import maestro.orchestra.LaunchAppCommand +import maestro.orchestra.yaml.MaestroFlowParser +import maestro.orchestra.yaml.YamlElementSelectorUnion +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import java.nio.file.Paths +import kotlin.reflect.KClass + +/** + * Every other test in this package reads its expectations off the `Yaml*` types that exist **today**, + * so none of them can answer the question that decides whether the schema is safe to publish: what + * happens the next time somebody changes one? + * + * These drive [FlowCommandSchema.schemaOf] with synthetic `Yaml*` shapes instead — a command being + * added, an argument being renamed, an argument being retyped — and assert the derived schema says + * what the parser would actually accept for them. + */ +class FlowCommandSchemaEvolutionTest { + + private val flowPath = Paths.get("test.yaml") + + // ------------------------------------------------------------------- adding a command + + /** A command written `- newBare` and nothing else, like `back`. */ + data class YamlNewBare( + val label: String? = null, + val optional: Boolean = false, + ) + + data class YamlNewNested(val inner: String, val depth: Int = 0) + + /** A new command carrying one argument of each data type the parser can see. */ + data class YamlNewMixedTypes( + val text: String, + val count: Int, + val big: Long, + val ratio: Double, + val flag: Boolean, + val items: List = emptyList(), + val mapping: Map = emptyMap(), + val anything: Any? = null, + val target: YamlElementSelectorUnion? = null, + val nested: YamlNewNested? = null, + val label: String? = null, + val optional: Boolean = false, + ) + + enum class WireNamed { + @JsonProperty("fast") FAST, + @JsonProperty("slow") SLOW, + } + + enum class PlainNamed { ALPHA, BETA } + + enum class Described(val description: String) { + A("Letter A"), + B("Letter B"), + } + + /** A new command carrying enum-valued arguments in each of the three forms the schema supports. */ + data class YamlNewEnums( + val speed: WireNamed, + val mode: PlainNamed? = null, + @YamlValues(of = Described::class, spelledBy = "description") val letter: String, + @YamlValues(of = PlainNamed::class) val plain: String = "ALPHA", + val speeds: List = emptyList(), + val label: String? = null, + val optional: Boolean = false, + ) + + /** A new command with a single-value form, like `setDarkMode: enabled`. */ + data class YamlNewShorthand( + val speed: WireNamed, + val label: String? = null, + val optional: Boolean = false, + ) { + companion object { + @JvmStatic + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + fun parse(speed: WireNamed) = YamlNewShorthand(speed) + } + } + + @Test + fun `a command with no arguments is described as taking none`() { + val schema = FlowCommandSchema.schemaOf("newBare", YamlNewBare::class) + + assertThat(schema.name).isEqualTo("newBare") + assertThat(schema.selector).isFalse() + assertThat(schema.arguments).isEmpty() + assertThat(schema.variants).isEmpty() + assertThat(schema.shorthand).isNull() + } + + @Test + fun `a command whose whole value is a plain string is described as a string shorthand`() { + val schema = FlowCommandSchema.schemaOf("newPlainString", String::class) + + assertThat(schema.shorthand).isEqualTo(ShorthandSchema(ArgumentKind.STRING, null)) + assertThat(schema.arguments).isEmpty() + assertThat(schema.selector).isFalse() + } + + @Test + fun `each argument data type is described by its own kind`() { + val arguments = FlowCommandSchema.schemaOf("newMixed", YamlNewMixedTypes::class) + .arguments.associateBy { it.name } + + assertThat(arguments.keys).containsExactly( + "text", "count", "big", "ratio", "flag", "items", "mapping", "anything", "target", "nested", + ) + assertThat(arguments.getValue("text")).isEqualTo(argument("text", ArgumentKind.STRING, required = true)) + assertThat(arguments.getValue("count")).isEqualTo(argument("count", ArgumentKind.NUMBER, required = true)) + assertThat(arguments.getValue("big")).isEqualTo(argument("big", ArgumentKind.NUMBER, required = true)) + assertThat(arguments.getValue("ratio")).isEqualTo(argument("ratio", ArgumentKind.NUMBER, required = true)) + assertThat(arguments.getValue("flag")).isEqualTo(argument("flag", ArgumentKind.BOOLEAN, required = true)) + assertThat(arguments.getValue("items")).isEqualTo(argument("items", ArgumentKind.ARRAY, required = false)) + assertThat(arguments.getValue("anything")).isEqualTo(argument("anything", ArgumentKind.ANY, required = false)) + assertThat(arguments.getValue("target")).isEqualTo(argument("target", ArgumentKind.SELECTOR, required = false)) + + // A map and a nested object both flatten to OBJECT: the schema does not describe what may go + // inside either. Pinned so that stops being a surprise if a command starts relying on it. + assertThat(arguments.getValue("mapping").kind).isEqualTo(ArgumentKind.OBJECT) + assertThat(arguments.getValue("nested").kind).isEqualTo(ArgumentKind.OBJECT) + } + + @Test + fun `each way of declaring an enum vocabulary is described as an ENUM`() { + val arguments = FlowCommandSchema.schemaOf("newEnums", YamlNewEnums::class) + .arguments.associateBy { it.name } + + // An enum-typed argument: the words are the constants' @JsonProperty wire names. + assertThat(arguments.getValue("speed")) + .isEqualTo(argument("speed", ArgumentKind.ENUM, required = true, values = listOf("fast", "slow"))) + // ... falling back to the constant names when there is no @JsonProperty. + assertThat(arguments.getValue("mode")) + .isEqualTo(argument("mode", ArgumentKind.ENUM, required = false, values = listOf("ALPHA", "BETA"))) + // A String argument that names its vocabulary with @YamlValues, spelled by a property ... + assertThat(arguments.getValue("letter")) + .isEqualTo(argument("letter", ArgumentKind.ENUM, required = true, values = listOf("Letter A", "Letter B"))) + // ... and spelled by the constant names. + assertThat(arguments.getValue("plain")) + .isEqualTo(argument("plain", ArgumentKind.ENUM, required = false, values = listOf("ALPHA", "BETA"))) + + // A list of enums keeps neither the element type nor the vocabulary. + assertThat(arguments.getValue("speeds")).isEqualTo(argument("speeds", ArgumentKind.ARRAY, required = false)) + } + + @Test + fun `a delegating creator is described as a shorthand carrying its vocabulary`() { + val schema = FlowCommandSchema.schemaOf("newShorthand", YamlNewShorthand::class) + + assertThat(schema.shorthand).isEqualTo(ShorthandSchema(ArgumentKind.ENUM, listOf("fast", "slow"))) + } + + // -------------------------------------------------------------- modifying a command + + data class YamlRenameBefore( + val text: String, + val label: String? = null, + val optional: Boolean = false, + ) + + /** The Kotlin parameter was renamed; the YAML spelling was deliberately kept. */ + data class YamlRenameAfter( + @JsonProperty("text") val message: String, + val label: String? = null, + val optional: Boolean = false, + ) + + /** The new spelling is the parameter name; the old one stays accepted as an alias. */ + data class YamlRenameWithAlias( + @JsonAlias("text") val message: String, + val label: String? = null, + val optional: Boolean = false, + ) + + /** The same rename, written with Kotlin's `@field:` use-site target instead of the default. */ + data class YamlRenameOnField( + @field:JsonProperty("text") val message: String, + val label: String? = null, + val optional: Boolean = false, + ) + + /** And with `@get:`. */ + data class YamlRenameOnGetter( + @get:JsonProperty("text") val message: String, + val label: String? = null, + val optional: Boolean = false, + ) + + /** An alias written with a use-site target. */ + data class YamlAliasOnField( + @field:JsonAlias("text") val message: String, + val label: String? = null, + val optional: Boolean = false, + ) + + data class YamlRetypeBefore( + val amount: String, + val label: String? = null, + val optional: Boolean = false, + ) + + data class YamlRetypeToNumber( + val amount: Int, + val label: String? = null, + val optional: Boolean = false, + ) + + data class YamlRetypeToEnum( + val amount: WireNamed, + val label: String? = null, + val optional: Boolean = false, + ) + + /** `String` -> `String?`: the same kind, but no longer mandatory. */ + data class YamlRetypeToNullable( + val amount: String?, + val label: String? = null, + val optional: Boolean = false, + ) + + /** `spelledBy` names a property the enum does not have. */ + data class YamlVocabularyMisspelled( + @YamlValues(of = Described::class, spelledBy = "caption") val letter: String, + val label: String? = null, + val optional: Boolean = false, + ) + + /** + * Renaming a Kotlin parameter while keeping its YAML spelling is the ordinary way to rename a + * field without breaking flows. Jackson keys off `@JsonProperty`, so the schema has to as well — + * otherwise it starts advertising a key the parser rejects as an unknown property. + */ + @Test + fun `an argument renamed behind its YAML spelling keeps advertising the YAML spelling`() { + assertThat(names(YamlRenameBefore::class)).containsExactly("text") + assertThat(names(YamlRenameAfter::class)).containsExactly("text") + } + + /** The same rename, against Jackson itself: `text` is what is read, `message` is rejected. */ + @Test + fun `Jackson reads a renamed argument by its YAML spelling and not by the parameter name`() { + val mapper = jacksonObjectMapper() + + assertThat(mapper.readValue("""{"text":"hello"}""", YamlRenameAfter::class.java).message) + .isEqualTo("hello") + // MissingKotlinParameterException, not UnrecognizedPropertyException: `text` is required and absent, + // and Jackson stops at the creator before it ever reports `message` as unknown. Either way the + // rename moved the key -- and that masking is why the check in FlowCommandSchemaTest asks the + // resolved deserializer rather than reading exceptions. + assertThrows { + mapper.readValue("""{"message":"hello"}""", YamlRenameAfter::class.java) + } + } + + /** + * `@JsonAlias` is a spelling the parser accepts and the schema currently never mentions. + * `launchApp: {url: …}` is the documented form for web flows and parses today. + */ + @Test + fun `an argument's aliases are advertised alongside its name`() { + val renamed = FlowCommandSchema.schemaOf("modified", YamlRenameWithAlias::class).arguments.single() + assertThat(renamed.name).isEqualTo("message") + assertThat(renamed.aliases).containsExactly("text") + + // Against the real tree: `launchApp: {url: ...}` is the documented form for web flows. + MaestroFlowParser.parseCommand(flowPath, APP_ID, "launchApp:\n url: https://example.com") + val appId = FlowCommandSchema.commands().single { it.name == "launchApp" } + .arguments.single { it.name == "appId" } + assertThat(appId.aliases).containsExactly("url") + } + + /** + * Kotlin's default target for these annotations is the value parameter, but `@field:` and `@get:` are + * legal and Jackson binds by them just the same. Reading only the parameter would leave the schema + * advertising the name the rename moved away from -- and Kotlin has warned that the default target is + * itself due to change, which would make that the common case rather than the unusual one. + */ + @Test + fun `a rename written with a use-site target is seen too`() { + val mapper = jacksonObjectMapper() + + // What Jackson actually binds, so the assertions below are not just describing the implementation. + assertThat(mapper.readValue("""{"text":"hello"}""", YamlRenameOnField::class.java).message).isEqualTo("hello") + assertThrows { + mapper.readValue("""{"message":"hello"}""", YamlRenameOnField::class.java) + } + assertThat(mapper.readValue("""{"text":"hello"}""", YamlRenameOnGetter::class.java).message).isEqualTo("hello") + assertThat(mapper.readValue("""{"text":"hello"}""", YamlAliasOnField::class.java).message).isEqualTo("hello") + assertThat(mapper.readValue("""{"message":"hello"}""", YamlAliasOnField::class.java).message).isEqualTo("hello") + + assertThat(names(YamlRenameOnField::class)).containsExactly("text") + assertThat(names(YamlRenameOnGetter::class)).containsExactly("text") + + val aliased = FlowCommandSchema.schemaOf("modified", YamlAliasOnField::class).arguments.single() + assertThat(aliased.name).isEqualTo("message") + assertThat(aliased.aliases).containsExactly("text") + } + + @Test + fun `retyping an argument changes the kind it is described by`() { + fun kindOf(type: KClass<*>) = FlowCommandSchema.schemaOf("retyped", type).arguments.single().kind + + assertThat(kindOf(YamlRetypeBefore::class)).isEqualTo(ArgumentKind.STRING) + assertThat(kindOf(YamlRetypeToNumber::class)).isEqualTo(ArgumentKind.NUMBER) + assertThat(kindOf(YamlRetypeToEnum::class)).isEqualTo(ArgumentKind.ENUM) + assertThat(FlowCommandSchema.schemaOf("retyped", YamlRetypeToEnum::class).arguments.single().values) + .containsExactly("fast", "slow") + } + + @Test + fun `making an argument nullable stops it being required`() { + assertThat(FlowCommandSchema.schemaOf("retyped", YamlRetypeBefore::class).arguments.single().required) + .isTrue() + assertThat(FlowCommandSchema.schemaOf("retyped", YamlRetypeToNullable::class).arguments.single().required) + .isFalse() + } + + /** + * A `spelledBy` that no longer matches — because the property was renamed — must say so. It is + * reached from every `commands()` call, so the failure is the whole schema, and the message is + * the only thing pointing at which annotation is wrong. + */ + @Test + fun `a vocabulary spelled by a property the enum does not have says which annotation is wrong`() { + val thrown = assertThrows { + FlowCommandSchema.schemaOf("vocab", YamlVocabularyMisspelled::class) + } + + assertThat(thrown).hasMessageThat().contains("spelledBy = \"caption\"") + assertThat(thrown).hasMessageThat().contains("Described") + } + + // ------------------------------------------------------------------------- plumbing + + private fun names(type: KClass<*>) = + FlowCommandSchema.schemaOf("modified", type).arguments.map { it.name } + + private fun argument( + name: String, + kind: ArgumentKind, + required: Boolean, + values: List? = null, + ) = ArgumentSchema(name, kind, required, values) + + private companion object { + const val APP_ID = "com.example.app" + } +} diff --git a/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/FlowCommandSchemaTest.kt b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/FlowCommandSchemaTest.kt new file mode 100644 index 0000000000..4908799f3a --- /dev/null +++ b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/FlowCommandSchemaTest.kt @@ -0,0 +1,291 @@ +package maestro.orchestra.yaml.schema + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import com.fasterxml.jackson.databind.deser.BeanDeserializerBase +import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.google.common.truth.Truth.assertThat +import maestro.KeyCode +import maestro.device.DeviceOrientation +import maestro.orchestra.yaml.MaestroFlowParser +import maestro.orchestra.yaml.YamlFluentCommand +import maestro.orchestra.yaml.stringCommands +import maestro.utils.TempFileHandler +import org.junit.jupiter.api.Test +import java.nio.file.Path +import java.nio.file.Paths +import kotlin.reflect.KClass +import kotlin.reflect.full.primaryConstructor + +class FlowCommandSchemaTest { + + /** + * The guarantee that makes the schema worth publishing: every value it advertises is a value the + * parser actually accepts. Each declared enum value is fed back through real parsing, so an enum + * renamed without its `@JsonProperty` fails here instead of shipping a schema that lies. + */ + @Test + fun `every advertised enum value parses`() { + for (command in FlowCommandSchema.commands()) { + command.shorthand + ?.takeIf { it.kind == ArgumentKind.ENUM } + ?.values.orEmpty() + .forEach { value -> assertParses("${command.name}: $value") } + + for (shape in shapesOf(command)) { + shape.filter { it.kind == ArgumentKind.ENUM }.forEach { argument -> + argument.values.orEmpty().forEach { value -> + assertParses(render(command.name, shape, argument.name to value)) + } + } + } + } + } + + /** + * `pressKey.key` and `setOrientation.orientation` have to stay `String` to keep accepting `${VAR}`, + * so only their `@YamlValues` annotation tells the schema what the parser's `getByName` will accept. + * Both the map form and the shorthand must carry the vocabulary, and `KeyCode` must be spelled by + * its `description`, not its constant names. The words are read back off the enums rather than + * listed here, so this test cannot become a second copy of them. + */ + @Test + fun `a String field annotated with YamlValues advertises its vocabulary`() { + val commands = FlowCommandSchema.commands().associateBy { it.name } + + val keys = KeyCode.entries.map { it.description } + val pressKey = commands.getValue("pressKey") + assertThat(pressKey.arguments.single { it.name == "key" }) + .isEqualTo(ArgumentSchema("key", ArgumentKind.ENUM, required = true, values = keys)) + assertThat(pressKey.shorthand).isEqualTo(ShorthandSchema(ArgumentKind.ENUM, keys)) + + val orientations = DeviceOrientation.entries.map { it.name } + val setOrientation = commands.getValue("setOrientation") + assertThat(setOrientation.arguments.single { it.name == "orientation" }) + .isEqualTo(ArgumentSchema("orientation", ArgumentKind.ENUM, required = true, values = orientations)) + assertThat(setOrientation.shorthand).isEqualTo(ShorthandSchema(ArgumentKind.ENUM, orientations)) + } + + /** + * A hand-written deserializer is where the schema can go blind: it may accept shapes the data + * class does not reveal. Every command that has one must be describable — as a selector, through + * alternative shapes, through an enum vocabulary, or through a single-value form. Adding a custom + * deserializer without exposing what it accepts fails here. + */ + @Test + fun `commands with a custom deserializer are describable`() { + val schemas = FlowCommandSchema.commands().associateBy { it.name } + + val opaque = YamlFluentCommand::class.primaryConstructor!!.parameters + .mapNotNull { parameter -> + val name = parameter.name?.takeUnless { it.startsWith("_") } ?: return@mapNotNull null + val type = parameter.type.classifier as? KClass<*> ?: return@mapNotNull null + if (!type.java.isAnnotationPresent(JsonDeserialize::class.java)) return@mapNotNull null + + val schema = schemas.getValue(name) + val describable = schema.selector || + schema.variants.isNotEmpty() || + schema.shorthand != null || + shapesOf(schema).any { shape -> shape.any { it.kind == ArgumentKind.ENUM } } + name.takeUnless { describable } + } + + assertThat(opaque).isEmpty() + } + + /** + * [FlowCommandSchema.commonArguments] is the one hand-written claim in an object whose whole point is + * that nothing is hand-written: `argumentsOf` strips any parameter named `label` or `optional` and the + * schema then asserts every command takes both. The parser has FAIL_ON_UNKNOWN_PROPERTIES on, so a + * command that declares only one of them would have the schema advertising a key the parser rejects. + */ + @Test + fun `every command declares the arguments the schema says they all share`() { + val common = FlowCommandSchema.commonArguments.map { it.name } + val missing = mutableListOf() + + for (parameter in YamlFluentCommand::class.primaryConstructor!!.parameters) { + val name = parameter.name?.takeUnless { it.startsWith("_") } ?: continue + val type = parameter.type.classifier as? KClass<*> ?: continue + val schema = FlowCommandSchema.commands().single { it.name == name } + if (schema.selector || type.java.isEnum || type == String::class) continue + + // A sealed command declares nothing itself; each alternative shape has to carry them. + val shapes = if (type.sealedSubclasses.isEmpty()) listOf(type) else type.sealedSubclasses + for (shape in shapes) { + val declared = shape.primaryConstructor?.parameters.orEmpty().mapNotNull { it.name }.toSet() + (common - declared).forEach { missing += "$name(${shape.simpleName}).$it" } + } + } + + assertThat(missing).isEmpty() + } + + /** + * `commands()` drops any parameter whose classifier is not a `KClass`. Nothing in the tree hits that + * today, and every other test in this file rebuilds the same filter, so a command vanishing from the + * published schema would go unnoticed. This is the one assertion that would not. + */ + @Test + fun `the schema covers every command YamlFluentCommand declares`() { + val declared = YamlFluentCommand::class.primaryConstructor!!.parameters + .count { it.name?.startsWith("_") == false } + + assertThat(FlowCommandSchema.commands()).hasSize(declared) + } + + /** + * Variant names are published strings a consumer reads and generates from, so they must be YAML + * vocabulary rather than the Kotlin class names `sealedSubclasses` hands over. Nothing else would + * notice a new alternative shape shipping as `YamlSomethingSwipe`. + */ + @Test + fun `no variant is published under its Kotlin class name`() { + val leaked = FlowCommandSchema.commands() + .flatMap { command -> command.variants.map { "${command.name}.${it.name}" } } + .filter { it.substringAfter('.').startsWith("Yaml") } + + assertThat(leaked).isEmpty() + } + + @Test + fun `every bare-string command the parser accepts is a command the schema declares`() { + val declared = FlowCommandSchema.commands().map { it.name }.toSet() + + // `hide keyboard` is a spelling of `hideKeyboard`, not a command of its own. Any other key the + // parser accepts but YamlFluentCommand does not declare would be a command the schema misses. + assertThat(stringCommands.keys - declared).containsExactly("hide keyboard") + } + + /** + * Every argument name the schema advertises has to be a name Jackson will bind, and every name + * Jackson binds has to be advertised. Asked of the resolved deserializer -- the object that does + * the binding -- rather than of the `Yaml*` source, so a `@JsonProperty` rename or a `@JsonAlias` + * the schema does not know about shows up here instead of shipping. + * + * "Bound" is not the same as "rejected if absent from the schema" for every command: `swipe`'s + * hand-written deserializer ignores keys it does not know, where the rest of the parser has + * FAIL_ON_UNKNOWN_PROPERTIES on. The check is the same either way; only the consequence of failing + * it differs. + */ + @Test + fun `every advertised argument name is a name Jackson binds`() { + val mapper = jacksonObjectMapper() + val common = FlowCommandSchema.commonArguments.map { it.name }.toSet() + val schemas = FlowCommandSchema.commands().associateBy { it.name } + val mismatches = mutableListOf() + val readsItsKeysByHand = mutableListOf() + + for (parameter in YamlFluentCommand::class.primaryConstructor!!.parameters) { + val name = parameter.name?.takeUnless { it.startsWith("_") } ?: continue + val type = parameter.type.classifier as? KClass<*> ?: continue + val schema = schemas.getValue(name) + if (schema.selector || (schema.arguments.isEmpty() && schema.variants.isEmpty())) continue + + for ((shapeType, arguments) in typedShapesOf(schema, type)) { + val binder = binderFor(mapper, shapeType.java) + if (binder == null) { + readsItsKeysByHand += name + continue + } + val advertised = arguments.flatMap { listOf(it.name) + it.aliases.orEmpty() }.toSet() + advertised.filterNot { binder.findProperty(it) != null } + .forEach { mismatches += "$name.$it: advertised, not bound" } + (boundNames(binder) - advertised - common) + .forEach { mismatches += "$name.$it: bound, not advertised" } + } + } + + assertThat(mismatches).isEmpty() + assertThat(readsItsKeysByHand.distinct()).containsExactlyElementsIn(NOT_INTROSPECTABLE) + } + + /** The concrete type behind each shape, paired with the arguments the schema gives that shape. */ + private fun typedShapesOf(schema: CommandSchema, type: KClass<*>): List, List>> { + if (schema.variants.isEmpty()) return listOf(type to schema.arguments) + return type.sealedSubclasses.map { subclass -> + val variant = schema.variants.single { it.name == FlowCommandSchema.variantNameOf(subclass) } + subclass to (schema.arguments + variant.arguments) + } + } + + /** + * The deserializer Jackson resolves for [type], which is the thing that actually decides what binds. + * Null when the type has a hand-written `@JsonDeserialize(using = ...)` deserializer: it reads its + * keys itself, so there is no property list to compare against -- see [NOT_INTROSPECTABLE]. + */ + private fun binderFor(mapper: ObjectMapper, type: Class<*>): BeanDeserializerBase? { + val context = (mapper.deserializationContext as DefaultDeserializationContext) + .createInstance(mapper.deserializationConfig, null, mapper.injectableValues) + val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(type)) + return deserializer as? BeanDeserializerBase + } + + /** Every key [binder] accepts, `@JsonAlias` spellings included. */ + private fun boundNames(binder: BeanDeserializerBase): Set = + binder.properties().asSequence().map { it.name }.toSet() + + private fun shapesOf(command: CommandSchema): List> { + if (command.variants.isEmpty()) return listOf(command.arguments) + return command.variants.map { command.arguments + it.arguments } + } + + /** Renders `name:` with every required argument plus [override], so the parser sees a valid command. */ + private fun render(name: String, shape: List, override: Pair): String { + val arguments = shape + .filter { it.required || it.name == override.first } + .associate { it.name to if (it.name == override.first) override.second else placeholderFor(it) } + return arguments.entries.joinToString(prefix = "$name:\n", separator = "\n") { " ${it.key}: ${it.value}" } + } + + /** + * A real flow on disk. `runFlow`, `runScript` and `retry` take a path in a plain `String` argument + * and read it during `toCommands`, so a literal placeholder makes them fail for a reason that has + * nothing to do with the schema. + */ + private val referencedFlow: String by lazy { + TempFileHandler().createTempFile(suffix = ".yaml") + .apply { writeText("appId: com.example.app\n---\n- back\n") } + .absolutePath + } + + private fun placeholderFor(argument: ArgumentSchema): String = when (argument.kind) { + ArgumentKind.NUMBER -> "1" + ArgumentKind.BOOLEAN -> "true" + ArgumentKind.ENUM -> argument.values!!.first() + ArgumentKind.ARRAY -> "[]" + ArgumentKind.OBJECT -> "{}" + ArgumentKind.STRING -> "\"$referencedFlow\"" + ArgumentKind.SELECTOR, ArgumentKind.ANY -> "\"placeholder\"" + } + + /** + * Parses through [MaestroFlowParser.parseCommand], not `checkSyntax`. `checkSyntax` stops after + * deserializing into `YamlFluentCommand` and never runs `toCommands`, where a large share of + * Maestro's validation lives -- including every `getByName` lookup behind a `@YamlValues` field. + * Against `checkSyntax` this assertion is vacuous for exactly the arguments the annotation exists + * for, because they are declared `String` and deserialize whatever they are given. + */ + private fun assertParses(command: String) { + try { + MaestroFlowParser.parseCommand(FLOW_PATH, APP_ID, command) + } catch (e: Exception) { + throw AssertionError("The schema advertises a value the parser rejects:\n$command", e) + } + } + + private companion object { + private val FLOW_PATH: Path = Paths.get("test.yaml") + private const val APP_ID = "com.example.app" + + /** + * Commands whose accepted keys nothing can enumerate: their `@JsonDeserialize(using = ...)` + * deserializer reads the tree by hand, so Jackson resolves no bean binder and neither direction + * of the check above can run for them. Asserted rather than quietly skipped, so a fifth command + * cannot join them without someone deciding to let it. + */ + private val NOT_INTROSPECTABLE = + listOf("swipe", "setOrientation", "setAirplaneMode", "setDarkMode") + } +} diff --git a/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/RequiredClaimTest.kt b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/RequiredClaimTest.kt new file mode 100644 index 0000000000..b6a9515714 --- /dev/null +++ b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/RequiredClaimTest.kt @@ -0,0 +1,163 @@ +package maestro.orchestra.yaml.schema + +import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.assertWithMessage +import maestro.orchestra.yaml.MaestroFlowParser +import maestro.utils.TempFileHandler +import org.junit.jupiter.api.Test +import java.nio.file.Path +import java.nio.file.Paths + +/** + * The schema's positive claims are cheap to satisfy — a schema that under-claims passes every one of + * them. These pin the boundary instead: what the schema calls required must actually be rejected when + * omitted, and the form the schema says is enough must actually parse. + */ +class RequiredClaimTest { + + /** + * Both commands deserialize their map form through a delegating creator that reads the field with + * `getOrDefault(name, "")`, so omitting it yields an empty string rather than an error. The schema + * is right that the field is required; the parser is wrong to accept its absence. Tightening that + * changes parsing behaviour and is deliberately not part of this change — pinned here so the day it + * is fixed, this test fails and the exception gets removed rather than lingering. + */ + private val parserAcceptsOmission = listOf("inputText.text", "evalScript.script") + + + @Test + fun `every argument the schema calls required is rejected when omitted`() { + val accepted = mutableListOf() + + for (command in testableCommands()) { + for (omitted in command.arguments.filter { it.required }) { + if (parses(render(command, omit = omitted.name))) { + accepted += "${command.name}.${omitted.name}" + } + } + } + + assertThat(accepted).containsExactlyElementsIn(parserAcceptsOmission) + } + + /** + * The other direction, and the one a consumer feels first: a command written with only what the + * schema says it needs has to parse. This is one question per command rather than one per optional + * argument — optional arguments are never rendered, so the form under test is the same whichever + * one you think of as omitted. + */ + @Test + fun `every command parses carrying only what the schema says it requires`() { + val rejected = testableCommands() + .filterNot { parses(render(it)) } + .map { it.name } + + assertThat(rejected).isEmpty() + } + + /** + * [YamlRequiresOneOf] is a hand-written claim about a rule that lives inside `toCommands`, so unlike + * everything else in the schema it can simply be wrong: a typo names an argument that does not exist, + * and a rule on a command that does not need one is never noticed. Each part of the claim is checked + * against the parser here. + */ + @Test + fun `every one-of rule says what the parser actually enforces`() { + for (command in testableCommands()) { + val rule = command.requiredOneOf ?: continue + val declared = command.arguments.associateBy { it.name } + + assertWithMessage("${command.name} names arguments it does not have") + .that(rule.names - declared.keys).isEmpty() + rule.names.forEach { + assertWithMessage("${command.name}.$it is in a one-of group and also required") + .that(declared.getValue(it).required).isFalse() + } + + // The gap the annotation exists to close: without it the schema says the empty form is fine. + assertWithMessage("${command.name}: {} parses, so it needs no one-of rule") + .that(parses("${command.name}: {}")).isFalse() + + // Each member on its own is enough. + rule.names.forEach { member -> + assertWithMessage("${command.name} carrying only $member is rejected") + .that(parses(render(command, oneOfMembers = listOf(member)))).isTrue() + } + + // And `exclusive` says whether all of them together are refused. + if (rule.names.size > 1) { + assertWithMessage("${command.name} declares exclusive=${rule.exclusive}") + .that(parses(render(command, oneOfMembers = rule.names))).isEqualTo(!rule.exclusive) + } + } + } + + /** + * Commands with alternative shapes are excluded: dropping a variant's distinguishing argument + * leaves a shape a sibling variant legitimately accepts — omit `from` from a swipe-by-element and + * a valid swipe-by-direction remains — so "it still parsed" says nothing about that argument. + * Their arguments are covered positively by [FlowCommandSchemaTest]. Asserted rather than assumed, + * so a second variant command cannot join the exclusion silently. + */ + private fun testableCommands(): List { + val commands = FlowCommandSchema.commands() + assertThat(commands.filter { it.variants.isNotEmpty() }.map { it.name }).containsExactly("swipe") + return commands.filter { it.variants.isEmpty() } + } + + /** + * The map form carrying everything the schema says the command needs, minus [omit]: every required + * argument, plus one member of a [CommandSchema.requiredOneOf] group when the command declares one. + */ + private fun render( + command: CommandSchema, + omit: String? = null, + oneOfMembers: List = command.requiredOneOf?.names?.take(1).orEmpty(), + ): String { + val kept = command.arguments.filter { (it.required || it.name in oneOfMembers) && it.name != omit } + if (kept.isNotEmpty()) { + return kept.joinToString(prefix = "${command.name}:\n", separator = "\n") { " ${it.name}: ${placeholderFor(it)}" } + } + // A command with no named arguments at all may have no map form either -- `action` is a + // plain `String` on YamlFluentCommand -- so write its single-value form. Gated on there being + // no arguments rather than none *kept*: a command whose one required argument was just omitted + // still has a map form, and writing its shorthand would test a different, complete command. + val writesOnlyAValue = command.arguments.isEmpty() && command.variants.isEmpty() + val shorthand = command.shorthand?.takeIf { omit == null && writesOnlyAValue } + ?: return "${command.name}: {}" + return "${command.name}: ${placeholderFor(shorthand.kind, shorthand.values)}" + } + + /** + * A real flow on disk. `runFlow`, `runScript` and `retry` take a path in a plain `String` argument + * and read it during `toCommands`, so a literal placeholder makes them fail for a reason that has + * nothing to do with the schema. + */ + private val referencedFlow: String by lazy { + TempFileHandler().createTempFile(suffix = ".yaml") + .apply { writeText("appId: com.example.app\n---\n- back\n") } + .absolutePath + } + + private fun placeholderFor(argument: ArgumentSchema): String = + placeholderFor(argument.kind, argument.values) + + private fun placeholderFor(kind: ArgumentKind, values: List?): String = when (kind) { + ArgumentKind.NUMBER -> "1" + ArgumentKind.BOOLEAN -> "true" + ArgumentKind.ENUM -> values!!.first() + ArgumentKind.ARRAY -> "[]" + ArgumentKind.OBJECT -> "{}" + ArgumentKind.STRING -> "\"$referencedFlow\"" + ArgumentKind.SELECTOR, ArgumentKind.ANY -> "\"placeholder\"" + } + + /** See `FlowCommandSchemaTest.assertParses` for why this is `parseCommand` and not `checkSyntax`. */ + private fun parses(command: String): Boolean = + runCatching { MaestroFlowParser.parseCommand(FLOW_PATH, APP_ID, command) }.isSuccess + + private companion object { + private val FLOW_PATH: Path = Paths.get("test.yaml") + private const val APP_ID = "com.example.app" + } +} diff --git a/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/SelectorArgumentsTest.kt b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/SelectorArgumentsTest.kt new file mode 100644 index 0000000000..e9f9d29506 --- /dev/null +++ b/maestro-orchestra/src/test/java/maestro/orchestra/yaml/schema/SelectorArgumentsTest.kt @@ -0,0 +1,39 @@ +package maestro.orchestra.yaml.schema + +import com.google.common.truth.Truth.assertThat +import maestro.orchestra.yaml.MaestroFlowParser +import org.junit.jupiter.api.Test +import java.nio.file.Paths + +/** + * [FlowCommandSchema.selectorArguments] is what a consumer writes inside `tapOn`, `assertVisible` and + * their siblings. Those commands publish no arguments of their own, so without this the schema says + * nothing at all about the shape Maestro's most-used commands take. + */ +class SelectorArgumentsTest { + + @Test + fun `every advertised selector argument is one the parser accepts`() { + val rejected = FlowCommandSchema.selectorArguments.filterNot { argument -> + val yaml = "tapOn:\n ${argument.name}: ${placeholderFor(argument)}" + runCatching { MaestroFlowParser.parseCommand(Paths.get("t.yaml"), "com.example.app", yaml) }.isSuccess + } + + assertThat(rejected.map { it.name }).isEmpty() + } + + @Test + fun `the selector carries the fields a flow actually uses`() { + assertThat(FlowCommandSchema.selectorArguments.map { it.name }) + .containsAtLeast("text", "id", "index", "enabled", "checked", "below", "containsChild") + } + + private fun placeholderFor(argument: ArgumentSchema): String = when (argument.kind) { + ArgumentKind.NUMBER -> "1" + ArgumentKind.BOOLEAN -> "true" + ArgumentKind.ENUM -> argument.values!!.first() + ArgumentKind.ARRAY -> "[]" + ArgumentKind.OBJECT -> "{}" + ArgumentKind.STRING, ArgumentKind.SELECTOR, ArgumentKind.ANY -> "\"placeholder\"" + } +}