diff --git a/CHANGELOG.md b/CHANGELOG.md index 06849d1..928fb3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- The tap family (`tap`, `long-press`, `ios tap`) declares its shared flags once via `TapTargetingOptions` / `TapTimingOptions` option groups instead of three hand-kept copies (#42). Flag names, defaults, validation messages, and `--json` envelopes are unchanged; the only user-visible deltas are in `--help` output, where per-verb wording ("Tap the center…" / "Long-press the center…") unifies to verb-neutral phrasing ("Target the center…") and the timing flags render as a contiguous block after `--duration`. - `make e2e` now runs both the iOS and Android E2E suites in sequence (continuing past a platform failure and failing if either did); the iOS-only target is `make e2e-ios`, Android-only stays `make e2e-android`. - `scripts/test-runner.sh` keeps running after a failed suite and prints a full pass/fail map at the end (a release gate needs the whole picture, not the first crash). The hardcoded suite list gained the missing `KeyboardStateTests` and the new suites, and the misnamed `StreamVideoDebugTests` entry — which silently matched zero tests — now points at the real `StreamVideoDebugTest` suite. diff --git a/Sources/SimUse/Commands/Button.swift b/Sources/SimUse/Commands/Button.swift index c36f936..d7e6ed6 100644 --- a/Sources/SimUse/Commands/Button.swift +++ b/Sources/SimUse/Commands/Button.swift @@ -67,12 +67,21 @@ struct Button: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimButtonCommand { var sub = IOSSimButtonCommand() sub.buttonType = buttonType sub.duration = duration sub.device = device sub.json = json - return try await sub.execute() + return sub } private func executeAndroid() throws -> ExecutionResult { diff --git a/Sources/SimUse/Commands/DescribeUI.swift b/Sources/SimUse/Commands/DescribeUI.swift index 705139f..e8ddd16 100644 --- a/Sources/SimUse/Commands/DescribeUI.swift +++ b/Sources/SimUse/Commands/DescribeUI.swift @@ -107,6 +107,15 @@ struct DescribeUI: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimDescribeUICommand { var sub = IOSSimDescribeUICommand() sub.point = point sub.maxProbes = maxProbes @@ -115,7 +124,7 @@ struct DescribeUI: SimUseExecutableCommand { sub.seedCellHeight = seedCellHeight sub.device = device sub.json = json - return try await sub.execute() + return sub } /// Android dispatch: routes through `AndroidDescribeUICommand.performDescribeUI` diff --git a/Sources/SimUse/Commands/Gesture.swift b/Sources/SimUse/Commands/Gesture.swift index 2ec5a60..5c52e4e 100644 --- a/Sources/SimUse/Commands/Gesture.swift +++ b/Sources/SimUse/Commands/Gesture.swift @@ -132,6 +132,15 @@ struct Gesture: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimGestureCommand { var sub = IOSSimGestureCommand() sub.preset = preset sub.screenWidth = screenWidth @@ -149,7 +158,7 @@ struct Gesture: SimUseExecutableCommand { sub.postDelay = postDelay sub.device = device sub.json = json - return try await sub.execute() + return sub } /// Android dispatch. Pre/post-delays use `Task.sleep` here (rather diff --git a/Sources/SimUse/Commands/KeyboardState.swift b/Sources/SimUse/Commands/KeyboardState.swift index c9d8009..a20d3e3 100644 --- a/Sources/SimUse/Commands/KeyboardState.swift +++ b/Sources/SimUse/Commands/KeyboardState.swift @@ -79,13 +79,22 @@ struct KeyboardState: SimUseExecutableCommand { imePackage: state.imePackage ) case .iOSSim, .none: - var sub = IOSSimKeyboardStateCommand() - sub.device = device - sub.json = json + let sub = makeIOSSubcommand() return try await sub.execute() } } + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimKeyboardStateCommand { + var sub = IOSSimKeyboardStateCommand() + sub.device = device + sub.json = json + return sub + } + func format(_ result: ExecutionResult) -> CommandOutput { .line(result.visible ? "soft" : "hidden") } diff --git a/Sources/SimUse/Commands/LongPress.swift b/Sources/SimUse/Commands/LongPress.swift index 6d54078..e5b9223 100644 --- a/Sources/SimUse/Commands/LongPress.swift +++ b/Sources/SimUse/Commands/LongPress.swift @@ -57,51 +57,7 @@ struct LongPress: SimUseExecutableCommand { )) var alias: String? - @Option(name: [.customShort("x"), .customLong("x")], help: "The X coordinate of the long-press. Accepts -x or --x.") - var pointX: Double? - - @Option(name: [.customShort("y"), .customLong("y")], help: "The Y coordinate of the long-press. Accepts -y or --y.") - var pointY: Double? - - @Option(name: .customLong("point"), help: ArgumentHelp( - "The point to long-press as a coordinate pair — same semantics as -x/-y; specify only one form.", - valueName: "x,y" - )) - var point: CoordinatePair? - - @Option(name: [.customLong("id")], help: "Long-press the center of the element matching AXUniqueId/resource-id literally. For the N-th outline entry, use the positional `@N` alias instead — `--id 42` matches the identifier string '42', NOT outline alias @42. Ignored if explicit coordinates (-x/-y or --point) are provided.") - var elementID: String? - - @Option(name: [.customLong("label")], help: "Long-press the center of the element matching AXLabel (accessibilityLabel). Ignored if explicit coordinates (-x/-y or --point) are provided.") - var elementLabel: String? - - @Option(name: [.customLong("value")], help: "Long-press the center of the element matching AXValue (the current value of a control). Ignored if explicit coordinates (-x/-y or --point) are provided.") - var elementValue: String? - - @Option(name: [.customLong("label-contains")], help: "Long-press the element whose AXLabel contains this case-sensitive substring. Useful when labels carry dynamic state (counters, timestamps). Mutually exclusive with --id/--label/--value/--label-regex.") - var labelContains: String? - - @Option(name: [.customLong("label-regex")], help: "Long-press the element whose AXLabel matches this ICU regex. Anchor with ^/$ for exact match. Mutually exclusive with --id/--label/--value/--label-contains.") - var labelRegex: String? - - @Option(name: [.customLong("element-type")], help: "Filter matches to elements of this accessibility type (e.g. Button, TextField). Narrows --id/--label/--value/--label-contains/--label-regex results when multiple elements match.") - var elementType: String? - - @Option( - name: .customLong("frame"), - parsing: .singleValue, - help: ArgumentHelp( - "Geometric AND-filter on frame bounds. Repeatable. Each value is a comma-separated list of `key=value` pairs. Keys: minX, maxX, minY, maxY. Values are absolute pixels (e.g. 700) or 0..1 fractions of the screen with an `r` suffix (e.g. 0.6r). Combine with selectors to disambiguate when several elements share a label/pattern but live in different screen regions.", - valueName: "key=value[,key=value]" - ) - ) - var frameSpecs: [String] = [] - - @Option(name: .customLong("pre-delay"), help: "Delay before the long-press in seconds.") - var preDelay: Double? - - @Option(name: .customLong("post-delay"), help: "Delay after the long-press in seconds.") - var postDelay: Double? + @OptionGroup var targeting: TapTargetingOptions @Option( name: .customLong("duration"), @@ -111,11 +67,7 @@ struct LongPress: SimUseExecutableCommand { ) var duration: Double = 0.8 - @Option(name: .customLong("wait-timeout"), help: "Maximum seconds to poll for the element before failing (0 = no waiting, default). Only applies to --id/--label/--value/--label-contains/--label-regex targeting.") - var waitTimeout: Double = 0 - - @Option(name: .customLong("poll-interval"), help: "Seconds between accessibility tree polls when --wait-timeout is active (default: 0.25).") - var pollInterval: Double = 0.25 + @OptionGroup var timing: TapTimingOptions @OptionGroup var multiTouch: MultiTouchOptions @@ -133,25 +85,16 @@ struct LongPress: SimUseExecutableCommand { typealias ExecutionResult = IOSSimTapCommand.ExecutionResult + /// Same shared group validators as `Tap` / `IOSSimTapCommand` — + /// same selector / coordinate / delay constraints apply, and the + /// duration default (0.8) is in the [0, 10] range so the validator + /// accepts it. ArgumentParser does not auto-validate nested option + /// groups, so these explicit calls are load-bearing + /// (`TapValidationParityTests` pins that every surface makes them). func validate() throws { - // Delegate to the shared tap rules — same selector / coordinate - // / delay constraints apply. The duration default (0.8) is in - // the [0, 10] range so the validator accepts it. - try IOSSimTapCommand.validateOptions( - alias: alias, - pointX: pointX, pointY: pointY, point: point, - elementID: elementID, - elementLabel: elementLabel, - elementValue: elementValue, - labelContains: labelContains, - labelRegex: labelRegex, - preDelay: preDelay, - postDelay: postDelay, - duration: duration, - waitTimeout: waitTimeout, - pollInterval: pollInterval, - frameSpecs: frameSpecs - ) + try targeting.validate(alias: alias) + try timing.validate() + try TapTimingOptions.validateDuration(duration) try multiTouch.validate() } @@ -168,32 +111,22 @@ struct LongPress: SimUseExecutableCommand { .line("✓ Long-press at (\(result.x), \(result.y)) completed successfully") } - /// iOS path: hand off to `IOSSimTapCommand` with `--duration` - /// carried through. That sub-command already splits the HID event - /// into down → sleep → up when duration > 0, which is exactly the - /// long-press recipe. + /// iOS path: hand off to the tap executor with `--duration` carried + /// through — it already splits the HID event into down → sleep → up + /// when duration > 0, which is exactly the long-press recipe. The + /// parsed groups are handed over as values, so no backend command + /// instance is hand-built and there is no per-field copy to + /// forget (#42). private func executeIOSSim() async throws -> ExecutionResult { - var sub = IOSSimTapCommand() - sub.alias = alias - sub.pointX = pointX - sub.pointY = pointY - sub.point = point - sub.elementID = elementID - sub.elementLabel = elementLabel - sub.elementValue = elementValue - sub.labelContains = labelContains - sub.labelRegex = labelRegex - sub.elementType = elementType - sub.frameSpecs = frameSpecs - sub.preDelay = preDelay - sub.postDelay = postDelay - sub.duration = duration - sub.waitTimeout = waitTimeout - sub.pollInterval = pollInterval - sub.multiTouch = multiTouch - sub.device = device - sub.json = json - return try await sub.execute() + try await IOSSimTapCommand.performTap( + alias: alias, + targeting: targeting, + timing: timing, + duration: duration, + multiTouch: multiTouch, + device: device, + json: json + ) } /// Android path: same selector resolution as `tap`, then a @@ -204,21 +137,21 @@ struct LongPress: SimUseExecutableCommand { /// across calls). private func executeAndroid() throws -> ExecutionResult { let frameFilter: SelectorFrameFilter? = { - guard !frameSpecs.isEmpty else { return nil } - return (try? SelectorFrameFilter(specs: frameSpecs)) + guard !targeting.frameSpecs.isEmpty else { return nil } + return (try? SelectorFrameFilter(specs: targeting.frameSpecs)) }() let selector = AndroidSelector( - id: elementID, - label: elementLabel, - labelContains: labelContains, - labelRegex: labelRegex, - value: elementValue, + id: targeting.elementID, + label: targeting.elementLabel, + labelContains: targeting.labelContains, + labelRegex: targeting.labelRegex, + value: targeting.elementValue, valueContains: nil, valueRegex: nil, - elementType: elementType, + elementType: targeting.elementType, frame: frameFilter ) - let explicit = try TapCoordinateResolver.resolve(x: pointX, y: pointY, point: point) + let explicit = try TapCoordinateResolver.resolve(x: targeting.pointX, y: targeting.pointY, point: targeting.point) let result = try AndroidTapCommand.performTap( udid: device.resolved, alias: alias, diff --git a/Sources/SimUse/Commands/MultiTouch.swift b/Sources/SimUse/Commands/MultiTouch.swift index a81cd7e..9483598 100644 --- a/Sources/SimUse/Commands/MultiTouch.swift +++ b/Sources/SimUse/Commands/MultiTouch.swift @@ -129,6 +129,15 @@ struct MultiTouch: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimMultiTouchCommand { var sub = IOSSimMultiTouchCommand() sub.x1 = x1 sub.y1 = y1 @@ -147,7 +156,7 @@ struct MultiTouch: SimUseExecutableCommand { sub.postDelay = postDelay sub.device = device sub.json = json - return try await sub.execute() + return sub } private func executeAndroid() async throws -> ExecutionResult { diff --git a/Sources/SimUse/Commands/Paste.swift b/Sources/SimUse/Commands/Paste.swift index 54974a9..8cd9369 100644 --- a/Sources/SimUse/Commands/Paste.swift +++ b/Sources/SimUse/Commands/Paste.swift @@ -141,11 +141,7 @@ struct Paste: SimUseExecutableCommand { // iOS-only soft-keyboard probe — Android goes through // ACTION_PASTE and has no equivalent HID-keyboard trap. guard !PlatformRouter.looksLikeAndroid(device.resolved) else { return } - var sub = IOSSimPasteCommand() - sub.viaMenu = viaMenu - sub.device = device - sub.json = json - await sub.clientPreflight() + await makeIOSSubcommand().clientPreflight() } func execute() async throws -> ExecutionResult { @@ -158,6 +154,18 @@ struct Paste: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. Also feeds `clientPreflight`, + /// which previously copied only the three fields it read — a full + /// copy means the preflight can never trap if it starts reading + /// another field. + func makeIOSSubcommand() -> IOSSimPasteCommand { var sub = IOSSimPasteCommand() sub.text = text sub.useStdin = useStdin @@ -171,7 +179,7 @@ struct Paste: SimUseExecutableCommand { sub.menuTimeout = menuTimeout sub.device = device sub.json = json - return try await sub.execute() + return sub } private func executeAndroid() throws -> ExecutionResult { diff --git a/Sources/SimUse/Commands/RecordVideo.swift b/Sources/SimUse/Commands/RecordVideo.swift index 6562599..bd1cc53 100644 --- a/Sources/SimUse/Commands/RecordVideo.swift +++ b/Sources/SimUse/Commands/RecordVideo.swift @@ -78,6 +78,15 @@ struct RecordVideo: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimRecordVideoCommand { var sub = IOSSimRecordVideoCommand() sub.fps = fps sub.quality = quality @@ -85,7 +94,7 @@ struct RecordVideo: SimUseExecutableCommand { sub.output = output sub.device = device sub.json = json - return try await sub.execute() + return sub } // MARK: - Android diff --git a/Sources/SimUse/Commands/Screenshot.swift b/Sources/SimUse/Commands/Screenshot.swift index 1f500df..de29657 100644 --- a/Sources/SimUse/Commands/Screenshot.swift +++ b/Sources/SimUse/Commands/Screenshot.swift @@ -57,11 +57,20 @@ struct Screenshot: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimScreenshotCommand { var sub = IOSSimScreenshotCommand() sub.output = output sub.device = device sub.json = json - return try await sub.execute() + return sub } private func executeAndroid() throws -> ExecutionResult { diff --git a/Sources/SimUse/Commands/Swipe.swift b/Sources/SimUse/Commands/Swipe.swift index 0fd7e1c..fb55c3d 100644 --- a/Sources/SimUse/Commands/Swipe.swift +++ b/Sources/SimUse/Commands/Swipe.swift @@ -80,6 +80,15 @@ struct Swipe: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimSwipeCommand { var sub = IOSSimSwipeCommand() sub.coordinates = coordinates sub.duration = duration @@ -88,7 +97,7 @@ struct Swipe: SimUseExecutableCommand { sub.postDelay = postDelay sub.device = device sub.json = json - return try await sub.execute() + return sub } /// Android dispatch. `duration` (iOS seconds) is re-mapped to diff --git a/Sources/SimUse/Commands/Tap.swift b/Sources/SimUse/Commands/Tap.swift index 207a287..5b6e7a0 100644 --- a/Sources/SimUse/Commands/Tap.swift +++ b/Sources/SimUse/Commands/Tap.swift @@ -70,51 +70,7 @@ struct Tap: SimUseExecutableCommand { )) var alias: String? - @Option(name: [.customShort("x"), .customLong("x")], help: "The X coordinate of the point to tap. Accepts -x or --x.") - var pointX: Double? - - @Option(name: [.customShort("y"), .customLong("y")], help: "The Y coordinate of the point to tap. Accepts -y or --y.") - var pointY: Double? - - @Option(name: .customLong("point"), help: ArgumentHelp( - "The point to tap as a coordinate pair — same semantics as -x/-y; specify only one form.", - valueName: "x,y" - )) - var point: CoordinatePair? - - @Option(name: [.customLong("id")], help: "Tap the center of the element matching AXUniqueId/resource-id literally. For the N-th outline entry, use the positional `@N` alias instead — `--id 42` matches the identifier string '42', NOT outline alias @42. Ignored if explicit coordinates (-x/-y or --point) are provided.") - var elementID: String? - - @Option(name: [.customLong("label")], help: "Tap the center of the element matching AXLabel (accessibilityLabel). Ignored if explicit coordinates (-x/-y or --point) are provided.") - var elementLabel: String? - - @Option(name: [.customLong("value")], help: "Tap the center of the element matching AXValue (the current value of a control). Ignored if explicit coordinates (-x/-y or --point) are provided.") - var elementValue: String? - - @Option(name: [.customLong("label-contains")], help: "Tap the element whose AXLabel contains this case-sensitive substring. Useful when labels carry dynamic state (counters, timestamps). Mutually exclusive with --id/--label/--value/--label-regex.") - var labelContains: String? - - @Option(name: [.customLong("label-regex")], help: "Tap the element whose AXLabel matches this ICU regex. Anchor with ^/$ for exact match. Mutually exclusive with --id/--label/--value/--label-contains.") - var labelRegex: String? - - @Option(name: [.customLong("element-type")], help: "Filter matches to elements of this accessibility type (e.g. Button, TextField, Switch). Narrows --id/--label/--value/--label-contains/--label-regex results when multiple elements match.") - var elementType: String? - - @Option( - name: .customLong("frame"), - parsing: .singleValue, - help: ArgumentHelp( - "Geometric AND-filter on frame bounds. Repeatable. Each value is a comma-separated list of `key=value` pairs. Keys: minX, maxX, minY, maxY. Values are absolute pixels (e.g. 700) or 0..1 fractions of the screen with an `r` suffix (e.g. 0.6r). Combine with selectors to disambiguate when several elements share a label/pattern but live in different screen regions.", - valueName: "key=value[,key=value]" - ) - ) - var frameSpecs: [String] = [] - - @Option(name: .customLong("pre-delay"), help: "Delay before tapping in seconds.") - var preDelay: Double? - - @Option(name: .customLong("post-delay"), help: "Delay after tapping in seconds.") - var postDelay: Double? + @OptionGroup var targeting: TapTargetingOptions @Option( name: .customLong("duration"), @@ -124,11 +80,7 @@ struct Tap: SimUseExecutableCommand { ) var duration: Double? - @Option(name: .customLong("wait-timeout"), help: "Maximum seconds to poll for the element before failing (0 = no waiting, default). Only applies to --id/--label/--value/--label-contains/--label-regex targeting.") - var waitTimeout: Double = 0 - - @Option(name: .customLong("poll-interval"), help: "Seconds between accessibility tree polls when --wait-timeout is active (default: 0.25).") - var pollInterval: Double = 0.25 + @OptionGroup var timing: TapTimingOptions @OptionGroup var multiTouch: MultiTouchOptions @@ -146,22 +98,14 @@ struct Tap: SimUseExecutableCommand { typealias ExecutionResult = IOSSimTapCommand.ExecutionResult + /// Same shared group validators as `IOSSimTapCommand.validate()` — + /// ArgumentParser does not auto-validate nested option groups, so + /// these explicit calls are load-bearing (`TapValidationParityTests` + /// pins that every surface makes them). func validate() throws { - try IOSSimTapCommand.validateOptions( - alias: alias, - pointX: pointX, pointY: pointY, point: point, - elementID: elementID, - elementLabel: elementLabel, - elementValue: elementValue, - labelContains: labelContains, - labelRegex: labelRegex, - preDelay: preDelay, - postDelay: postDelay, - duration: duration, - waitTimeout: waitTimeout, - pollInterval: pollInterval, - frameSpecs: frameSpecs - ) + try targeting.validate(alias: alias) + try timing.validate() + try TapTimingOptions.validateDuration(duration) try multiTouch.validate() } @@ -182,35 +126,21 @@ struct Tap: SimUseExecutableCommand { .line("✓ Tap at (\(result.x), \(result.y)) completed successfully") } - /// Forward to the iOS Simulator backend. - /// Constructs an `IOSSimTapCommand`, copies the resolved flags - /// across, and runs its `execute()`. Validation has already passed - /// on the top-level struct, so the sub-command's `validate()` is - /// intentionally skipped — ArgumentParser only calls `validate()` - /// on the root parsed command, and re-running it here would double - /// up the work for no benefit. + /// Forward to the iOS Simulator backend through its typed executor + /// entry point — the parsed groups are handed over as values, so no + /// backend command instance is hand-built and there is no per-field + /// copy to forget (#42). Validation has already passed on this + /// struct; `performTap` does not re-run it. private func executeIOSSim() async throws -> ExecutionResult { - var sub = IOSSimTapCommand() - sub.alias = alias - sub.pointX = pointX - sub.pointY = pointY - sub.point = point - sub.elementID = elementID - sub.elementLabel = elementLabel - sub.elementValue = elementValue - sub.labelContains = labelContains - sub.labelRegex = labelRegex - sub.elementType = elementType - sub.frameSpecs = frameSpecs - sub.preDelay = preDelay - sub.postDelay = postDelay - sub.duration = duration - sub.waitTimeout = waitTimeout - sub.pollInterval = pollInterval - sub.multiTouch = multiTouch - sub.device = device - sub.json = json - return try await sub.execute() + try await IOSSimTapCommand.performTap( + alias: alias, + targeting: targeting, + timing: timing, + duration: duration, + multiTouch: multiTouch, + device: device, + json: json + ) } /// Forward to the Android backend. Symmetric to `executeIOSSim` — @@ -224,21 +154,21 @@ struct Tap: SimUseExecutableCommand { /// branch needs the explicit round. private func executeAndroid() throws -> ExecutionResult { let frameFilter: SelectorFrameFilter? = { - guard !frameSpecs.isEmpty else { return nil } - return (try? SelectorFrameFilter(specs: frameSpecs)) + guard !targeting.frameSpecs.isEmpty else { return nil } + return (try? SelectorFrameFilter(specs: targeting.frameSpecs)) }() let selector = AndroidSelector( - id: elementID, - label: elementLabel, - labelContains: labelContains, - labelRegex: labelRegex, - value: elementValue, + id: targeting.elementID, + label: targeting.elementLabel, + labelContains: targeting.labelContains, + labelRegex: targeting.labelRegex, + value: targeting.elementValue, valueContains: nil, valueRegex: nil, - elementType: elementType, + elementType: targeting.elementType, frame: frameFilter ) - let explicit = try TapCoordinateResolver.resolve(x: pointX, y: pointY, point: point) + let explicit = try TapCoordinateResolver.resolve(x: targeting.pointX, y: targeting.pointY, point: targeting.point) let result = try AndroidTapCommand.performTap( udid: device.resolved, alias: alias, diff --git a/Sources/SimUse/Commands/Touch.swift b/Sources/SimUse/Commands/Touch.swift index 042f835..9fdc782 100644 --- a/Sources/SimUse/Commands/Touch.swift +++ b/Sources/SimUse/Commands/Touch.swift @@ -102,6 +102,15 @@ struct Touch: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimTouchCommand { var sub = IOSSimTouchCommand() sub.pointX = pointX sub.pointY = pointY @@ -110,7 +119,7 @@ struct Touch: SimUseExecutableCommand { sub.delay = delay sub.device = device sub.json = json - return try await sub.execute() + return sub } private func executeAndroid() throws -> ExecutionResult { diff --git a/Sources/SimUse/Commands/Type.swift b/Sources/SimUse/Commands/Type.swift index 6973bdb..6600b88 100644 --- a/Sources/SimUse/Commands/Type.swift +++ b/Sources/SimUse/Commands/Type.swift @@ -86,13 +86,22 @@ struct Type: SimUseExecutableCommand { } private func executeIOSSim() async throws -> ExecutionResult { + let sub = makeIOSSubcommand() + return try await sub.execute() + } + + /// Construct the backend command and copy every parsed flag across. + /// A missed field stays in ArgumentParser's wrapper-definition state + /// and traps on first read (#42) — pinned by + /// `ForwarderInitializationGuardTests`. + func makeIOSSubcommand() -> IOSSimTypeCommand { var sub = IOSSimTypeCommand() sub.text = text sub.useStdin = useStdin sub.inputFile = inputFile sub.device = device sub.json = json - return try await sub.execute() + return sub } private func executeAndroid() throws -> ExecutionResult { diff --git a/Sources/SimUseCore/Options/TapTargetingOptions.swift b/Sources/SimUseCore/Options/TapTargetingOptions.swift new file mode 100644 index 0000000..4feca6f --- /dev/null +++ b/Sources/SimUseCore/Options/TapTargetingOptions.swift @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: Apache-2.0 +import ArgumentParser +import Foundation + +/// Shared targeting flag surface of the tap family (`tap`, +/// `long-press`, `ios tap`): explicit coordinates, the five +/// accessibility selectors, and the type / frame disambiguators. +/// Declared once so every surface parses identically and the top-level +/// forwarders transfer the whole parsed group instead of copying +/// fields one by one (#42). +/// +/// Two tap-family flags deliberately stay per-command: the positional +/// alias (its help text names the verb) and `--duration` (different +/// default and help between `tap` and `long-press`). +public struct TapTargetingOptions: ParsableArguments { + @Option(name: [.customShort("x"), .customLong("x")], help: "The X coordinate of the target point. Accepts -x or --x.") + public var pointX: Double? + + @Option(name: [.customShort("y"), .customLong("y")], help: "The Y coordinate of the target point. Accepts -y or --y.") + public var pointY: Double? + + @Option(name: .customLong("point"), help: ArgumentHelp( + "The target point as a coordinate pair — same semantics as -x/-y; specify only one form.", + valueName: "x,y" + )) + public var point: CoordinatePair? + + @Option(name: [.customLong("id")], help: "Target the center of the element matching AXUniqueId/resource-id literally. For the N-th outline entry, use the positional `@N` alias instead — `--id 42` matches the identifier string '42', NOT outline alias @42. Ignored if explicit coordinates (-x/-y or --point) are provided.") + public var elementID: String? + + @Option(name: [.customLong("label")], help: "Target the center of the element matching AXLabel (accessibilityLabel). Ignored if explicit coordinates (-x/-y or --point) are provided.") + public var elementLabel: String? + + @Option(name: [.customLong("value")], help: "Target the center of the element matching AXValue (the current value of a control). Ignored if explicit coordinates (-x/-y or --point) are provided.") + public var elementValue: String? + + @Option(name: [.customLong("label-contains")], help: "Target the element whose AXLabel contains this case-sensitive substring. Useful when labels carry dynamic state (counters, timestamps). Mutually exclusive with --id/--label/--value/--label-regex.") + public var labelContains: String? + + @Option(name: [.customLong("label-regex")], help: "Target the element whose AXLabel matches this ICU regex. Anchor with ^/$ for exact match. Mutually exclusive with --id/--label/--value/--label-contains.") + public var labelRegex: String? + + @Option(name: [.customLong("element-type")], help: "Filter matches to elements of this accessibility type (e.g. Button, TextField, Switch). Narrows --id/--label/--value/--label-contains/--label-regex results when multiple elements match.") + public var elementType: String? + + @Option( + name: .customLong("frame"), + parsing: .singleValue, + help: ArgumentHelp( + "Geometric AND-filter on frame bounds. Repeatable. Each value is a comma-separated list of `key=value` pairs. Keys: minX, maxX, minY, maxY. Values are absolute pixels (e.g. 700) or 0..1 fractions of the screen with an `r` suffix (e.g. 0.6r). Combine with selectors to disambiguate when several elements share a label/pattern but live in different screen regions.", + valueName: "key=value[,key=value]" + ) + ) + public var frameSpecs: [String] = [] + + public init() {} + + /// Shared targeting rules for every tap-family surface. Not the + /// `ParsableArguments.validate()` witness — ArgumentParser (1.5.0) + /// does not auto-validate nested option groups, and the + /// alias-conflict rules need the per-command positional, so each + /// command calls this explicitly from its own `validate()` (the + /// same convention `MultiTouchOptions.validate()` uses). + public func validate(alias: String?) throws { + if let alias { + guard OutlineAliasResolver.looksLikeAlias(alias) else { + throw ValidationError("Positional alias '\(alias)' must be `@N`, `#N`, `#N@M`, or `#`.") + } + var conflicts: [String] = [] + if pointX != nil { conflicts.append("-x") } + if pointY != nil { conflicts.append("-y") } + if point != nil { conflicts.append("--point") } + if elementID != nil { conflicts.append("--id") } + if elementLabel != nil { conflicts.append("--label") } + if elementValue != nil { conflicts.append("--value") } + if labelContains != nil { conflicts.append("--label-contains") } + if labelRegex != nil { conflicts.append("--label-regex") } + if !conflicts.isEmpty { + throw ValidationError("Alias '\(alias)' cannot be combined with \(conflicts.joined(separator: ", ")).") + } + } else if pointX != nil || pointY != nil || point != nil { + _ = try TapCoordinateResolver.resolve(x: pointX, y: pointY, point: point) + } else { + let selectors: [(String, String?)] = [ + ("--id", elementID), + ("--label", elementLabel), + ("--value", elementValue), + ("--label-contains", labelContains), + ("--label-regex", labelRegex), + ] + let provided = selectors.filter { $0.1 != nil } + if provided.isEmpty { + throw ValidationError("Either provide an `@N` / `#N` / `#N@M` alias, coordinates (--point x,y or both -x/-y), or use --id/--label/--value/--label-contains/--label-regex to tap an element.") + } + if provided.count > 1 { + let names = provided.map(\.0).joined(separator: ", ") + throw ValidationError("Use only one of --id, --label, --value, --label-contains, --label-regex (got: \(names)).") + } + for (name, raw) in provided { + if let raw, raw.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { + throw ValidationError("\(name) must not be empty.") + } + } + if let labelRegex { + do { + _ = try NSRegularExpression(pattern: labelRegex, options: []) + } catch { + throw ValidationError("--label-regex '\(labelRegex)' is not a valid regular expression: \(error.localizedDescription)") + } + } + } + + if !frameSpecs.isEmpty { + // `SelectorFrameFilter` mirrors the iOS-side + // `AccessibilityTargetResolver.FrameFilter` spec syntax and + // error messages verbatim, so validating here keeps the + // user-facing text identical while staying platform-neutral. + do { + _ = try SelectorFrameFilter(specs: frameSpecs) + } catch let error as SelectorFrameFilter.ParseError { + throw ValidationError(error.message) + } + + if pointX != nil || pointY != nil || point != nil { + throw ValidationError("--frame cannot be combined with explicit -x/-y/--point coordinates (those bypass the AX tree).") + } + if let alias, case .some(let parsed) = OutlineAliasResolver.parse(alias) { + switch parsed { + case .at, .list: + throw ValidationError("--frame cannot be combined with the @N / #N / #N@M alias forms (they resolve to cached coordinates without consulting the AX tree). Use --label / --label-contains / --label-regex / --id / # with --frame instead.") + case .id: + break + } + } + } + } + + /// True when explicit coordinates were provided in either form. + public var hasExplicitCoordinates: Bool { + pointX != nil || pointY != nil || point != nil + } +} diff --git a/Sources/SimUseCore/Options/TapTimingOptions.swift b/Sources/SimUseCore/Options/TapTimingOptions.swift new file mode 100644 index 0000000..a6ec84d --- /dev/null +++ b/Sources/SimUseCore/Options/TapTimingOptions.swift @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: Apache-2.0 +import ArgumentParser +import Foundation + +/// Shared timing flag surface of the tap family (`tap`, `long-press`, +/// `ios tap`): delays around the action and the element-wait polling +/// knobs. Declared once so every surface parses identically and the +/// top-level forwarders transfer the whole parsed group (#42). +/// +/// `--duration` is deliberately NOT part of this group — `tap` (nil +/// default, single combined HID event) and `long-press` (0.8s default) +/// disagree on both default and help text, so each command declares it +/// and runs `TapTimingOptions.validateDuration` alongside `validate()`. +public struct TapTimingOptions: ParsableArguments { + @Option(name: .customLong("pre-delay"), help: "Delay before the action in seconds.") + public var preDelay: Double? + + @Option(name: .customLong("post-delay"), help: "Delay after the action in seconds.") + public var postDelay: Double? + + @Option(name: .customLong("wait-timeout"), help: "Maximum seconds to poll for the element before failing (0 = no waiting, default). Only applies to --id/--label/--value/--label-contains/--label-regex targeting.") + public var waitTimeout: Double = 0 + + @Option(name: .customLong("poll-interval"), help: "Seconds between accessibility tree polls when --wait-timeout is active (default: 0.25).") + public var pollInterval: Double = 0.25 + + public init() {} + + /// ArgumentParser (1.5.0) does not auto-validate nested option + /// groups — each command must call this explicitly from its own + /// `validate()`, the same convention `MultiTouchOptions.validate()` + /// uses. `TapValidationParityTests` pins that all three surfaces do. + public func validate() throws { + if let preDelay { + guard preDelay >= 0 && preDelay <= 10.0 else { + throw ValidationError("Pre-delay must be between 0 and 10 seconds.") + } + } + + if let postDelay { + guard postDelay >= 0 && postDelay <= 10.0 else { + throw ValidationError("Post-delay must be between 0 and 10 seconds.") + } + } + + guard waitTimeout >= 0 else { + throw ValidationError("--wait-timeout must be non-negative.") + } + + if waitTimeout > 0 { + guard pollInterval > 0 else { + throw ValidationError("--poll-interval must be greater than 0 when --wait-timeout is active.") + } + } + } + + /// Range check for the per-command `--duration` flag (see the type + /// doc for why the flag itself is not in this group). + public static func validateDuration(_ duration: Double?) throws { + if let duration { + guard duration >= 0 && duration <= 10.0 else { + throw ValidationError("--duration must be between 0 and 10 seconds.") + } + } + } +} diff --git a/Sources/iOSSimBackend/Batch/Command+BatchConvertible.swift b/Sources/iOSSimBackend/Batch/Command+BatchConvertible.swift index b2db0e0..49454ab 100644 --- a/Sources/iOSSimBackend/Batch/Command+BatchConvertible.swift +++ b/Sources/iOSSimBackend/Batch/Command+BatchConvertible.swift @@ -39,19 +39,19 @@ extension IOSSimTapCommand: BatchConvertible { // selectors (issue #34), raw for explicit --point/-x/-y. let resolvedPoint: (x: Double, y: Double) - if let explicit = try TapCoordinateResolver.resolve(x: pointX, y: pointY, point: point) { + if let explicit = try TapCoordinateResolver.resolve(x: targeting.pointX, y: targeting.pointY, point: targeting.point) { resolvedPoint = (explicit.x, explicit.y) } else { let query: AccessibilityQuery - if let elementID { + if let elementID = targeting.elementID { query = .id(elementID) - } else if let elementLabel { + } else if let elementLabel = targeting.elementLabel { query = .label(elementLabel) - } else if let elementValue { + } else if let elementValue = targeting.elementValue { query = .value(elementValue) - } else if let labelContains { + } else if let labelContains = targeting.labelContains { query = .labelContains(labelContains) - } else if let labelRegex { + } else if let labelRegex = targeting.labelRegex { query = .labelRegex(pattern: labelRegex) } else { throw CLIError(errorDescription: "Unexpected state: no coordinates and no element query.") @@ -62,7 +62,7 @@ extension IOSSimTapCommand: BatchConvertible { simulatorUDID: context.simulatorUDID, waitTimeout: context.waitTimeout, pollInterval: context.pollInterval, - elementType: elementType, + elementType: targeting.elementType, frameFilter: frameFilter, rootsProvider: { forceRefresh in let roots = try await context.accessibilityRoots(logger: logger, forceRefresh: forceRefresh) @@ -87,20 +87,20 @@ extension IOSSimTapCommand: BatchConvertible { // sleep → up so UIKit recognisers see a real hold. Pre/post // delays bracket the whole sequence as host sleeps. var primitives: [BatchPrimitive] = [] - if let preDelay, preDelay > 0 { + if let preDelay = timing.preDelay, preDelay > 0 { primitives.append(.hostSleep(preDelay)) } primitives.append(.hidBarrier(.touchDownAt(x: resolvedPoint.x, y: resolvedPoint.y))) primitives.append(.hostSleep(duration)) primitives.append(.hidBarrier(.touchUpAt(x: resolvedPoint.x, y: resolvedPoint.y))) - if let postDelay, postDelay > 0 { + if let postDelay = timing.postDelay, postDelay > 0 { primitives.append(.hostSleep(postDelay)) } return primitives } let tapEvent = FBSimulatorHIDEvent.tapAt(x: resolvedPoint.x, y: resolvedPoint.y) - return [.hidMergeable(buildDelayedEvent(preDelay: preDelay, mainEvent: tapEvent, postDelay: postDelay))] + return [.hidMergeable(buildDelayedEvent(preDelay: timing.preDelay, mainEvent: tapEvent, postDelay: timing.postDelay))] } } diff --git a/Sources/iOSSimBackend/Extensions/AsyncParsableCommand+Setup.swift b/Sources/iOSSimBackend/Extensions/AsyncParsableCommand+Setup.swift index c979c3f..5817d08 100644 --- a/Sources/iOSSimBackend/Extensions/AsyncParsableCommand+Setup.swift +++ b/Sources/iOSSimBackend/Extensions/AsyncParsableCommand+Setup.swift @@ -7,24 +7,32 @@ import SimUseCore extension AsyncParsableCommand { public func setup(logger: SimUseLogger) async throws { - // Check Xcode availability - do { - let isXcodeAvailable: NSString = try await FutureBridge.value(FBXcodeDirectory.xcodeSelectDeveloperDirectory()) - if isXcodeAvailable.length == 0 { - logger.error().log("Xcode is not available, idb will not be able to use Simulators") - throw CLIError(errorDescription: "Xcode is not available, idb will not be able to use Simulators") - } - } catch { - logger.error().log("Xcode is not available, idb will not be able to use Simulators: \(error.localizedDescription)") + try await performEssentialSetup(logger: logger) + } +} + +/// Free-function form of `setup(logger:)` — the body never touched the +/// command instance, and typed executor entry points that are not +/// command instances (e.g. `IOSSimTapCommand.performTap`) need the same +/// preflight. +public func performEssentialSetup(logger: SimUseLogger) async throws { + // Check Xcode availability + do { + let isXcodeAvailable: NSString = try await FutureBridge.value(FBXcodeDirectory.xcodeSelectDeveloperDirectory()) + if isXcodeAvailable.length == 0 { + logger.error().log("Xcode is not available, idb will not be able to use Simulators") throw CLIError(errorDescription: "Xcode is not available, idb will not be able to use Simulators") } - - // Load essential frameworks - do { - try FBSimulatorControlFrameworkLoader.essentialFrameworks.loadPrivateFrameworks(logger) - } catch { - logger.info().log("Essential private frameworks failed to loaded.") - throw CLIError(errorDescription: "Essential private frameworks failed to loaded.") - } + } catch { + logger.error().log("Xcode is not available, idb will not be able to use Simulators: \(error.localizedDescription)") + throw CLIError(errorDescription: "Xcode is not available, idb will not be able to use Simulators") + } + + // Load essential frameworks + do { + try FBSimulatorControlFrameworkLoader.essentialFrameworks.loadPrivateFrameworks(logger) + } catch { + logger.info().log("Essential private frameworks failed to loaded.") + throw CLIError(errorDescription: "Essential private frameworks failed to loaded.") } } \ No newline at end of file diff --git a/Sources/iOSSimBackend/Verbs/IOSSimTapCommand.swift b/Sources/iOSSimBackend/Verbs/IOSSimTapCommand.swift index 5aac2fa..8607dd0 100644 --- a/Sources/iOSSimBackend/Verbs/IOSSimTapCommand.swift +++ b/Sources/iOSSimBackend/Verbs/IOSSimTapCommand.swift @@ -21,51 +21,7 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { )) public var alias: String? - @Option(name: [.customShort("x"), .customLong("x")], help: "The X coordinate of the point to tap. Accepts -x or --x.") - public var pointX: Double? - - @Option(name: [.customShort("y"), .customLong("y")], help: "The Y coordinate of the point to tap. Accepts -y or --y.") - public var pointY: Double? - - @Option(name: .customLong("point"), help: ArgumentHelp( - "The point to tap as a coordinate pair — same semantics as -x/-y; specify only one form.", - valueName: "x,y" - )) - public var point: CoordinatePair? - - @Option(name: [.customLong("id")], help: "Tap the center of the element matching AXUniqueId/resource-id literally. For the N-th outline entry, use the positional `@N` alias instead — `--id 42` matches the identifier string '42', NOT outline alias @42. Ignored if explicit coordinates (-x/-y or --point) are provided.") - public var elementID: String? - - @Option(name: [.customLong("label")], help: "Tap the center of the element matching AXLabel (accessibilityLabel). Ignored if explicit coordinates (-x/-y or --point) are provided.") - public var elementLabel: String? - - @Option(name: [.customLong("value")], help: "Tap the center of the element matching AXValue (the current value of a control). Ignored if explicit coordinates (-x/-y or --point) are provided.") - public var elementValue: String? - - @Option(name: [.customLong("label-contains")], help: "Tap the element whose AXLabel contains this case-sensitive substring. Useful when labels carry dynamic state (counters, timestamps). Mutually exclusive with --id/--label/--value/--label-regex.") - public var labelContains: String? - - @Option(name: [.customLong("label-regex")], help: "Tap the element whose AXLabel matches this ICU regex. Anchor with ^/$ for exact match. Mutually exclusive with --id/--label/--value/--label-contains.") - public var labelRegex: String? - - @Option(name: [.customLong("element-type")], help: "Filter matches to elements of this accessibility type (e.g. Button, TextField, Switch). Narrows --id/--label/--value/--label-contains/--label-regex results when multiple elements match.") - public var elementType: String? - - @Option( - name: .customLong("frame"), - parsing: .singleValue, - help: ArgumentHelp( - "Geometric AND-filter on frame bounds. Repeatable. Each value is a comma-separated list of `key=value` pairs. Keys: minX, maxX, minY, maxY. Values are absolute pixels (e.g. 700) or 0..1 fractions of the screen with an `r` suffix (e.g. 0.6r). Combine with selectors to disambiguate when several elements share a label/pattern but live in different screen regions.", - valueName: "key=value[,key=value]" - ) - ) - public var frameSpecs: [String] = [] - - @Option(name: .customLong("pre-delay"), help: "Delay before tapping in seconds.") - public var preDelay: Double? - - @Option(name: .customLong("post-delay"), help: "Delay after tapping in seconds.") - public var postDelay: Double? + @OptionGroup public var targeting: TapTargetingOptions @Option( name: .customLong("duration"), @@ -75,11 +31,7 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { ) public var duration: Double? - @Option(name: .customLong("wait-timeout"), help: "Maximum seconds to poll for the element before failing (0 = no waiting, default). Only applies to --id/--label/--value/--label-contains/--label-regex targeting.") - public var waitTimeout: Double = 0 - - @Option(name: .customLong("poll-interval"), help: "Seconds between accessibility tree polls when --wait-timeout is active (default: 0.25).") - public var pollInterval: Double = 0.25 + @OptionGroup public var timing: TapTimingOptions @OptionGroup public var multiTouch: MultiTouchOptions @@ -98,9 +50,13 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { public var simulatorUDIDForDaemon: String? { device.resolved } public var frameFilter: AccessibilityTargetResolver.FrameFilter? { - // Validation guarantees parse success; force-try keeps execute() - // free of throws-only-for-validate paths. - let filter = (try? AccessibilityTargetResolver.FrameFilter(specs: frameSpecs)) ?? .init() + Self.frameFilter(from: targeting) + } + + static func frameFilter(from targeting: TapTargetingOptions) -> AccessibilityTargetResolver.FrameFilter? { + // Validation guarantees parse success; force-try keeps the + // execution path free of throws-only-for-validate branches. + let filter = (try? AccessibilityTargetResolver.FrameFilter(specs: targeting.frameSpecs)) ?? .init() return filter.isEmpty ? nil : filter } @@ -125,145 +81,51 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { } } + /// The rules themselves live on the shared groups + /// (`TapTargetingOptions.validate(alias:)` / + /// `TapTimingOptions.validate()`) so all three tap surfaces run the + /// same table. ArgumentParser does not auto-validate nested option + /// groups — the explicit calls here are load-bearing, and + /// `TapValidationParityTests` pins that every surface makes them. public func validate() throws { - try Self.validateOptions( + try targeting.validate(alias: alias) + try timing.validate() + try TapTimingOptions.validateDuration(duration) + try multiTouch.validate() + } + + public func execute() async throws -> ExecutionResult { + try await Self.performTap( alias: alias, - pointX: pointX, pointY: pointY, point: point, - elementID: elementID, - elementLabel: elementLabel, - elementValue: elementValue, - labelContains: labelContains, - labelRegex: labelRegex, - preDelay: preDelay, - postDelay: postDelay, + targeting: targeting, + timing: timing, duration: duration, - waitTimeout: waitTimeout, - pollInterval: pollInterval, - frameSpecs: frameSpecs + multiTouch: multiTouch, + device: device, + json: json ) - try multiTouch.validate() } - /// Shared validation factored out as a static so the top-level - /// cross-platform forwarder (`Sources/SimUse/Commands/Tap.swift`) - /// runs the same rules without re-implementing them. - public static func validateOptions( + /// Typed executor entry point — the iOS mirror of + /// `AndroidTapCommand.performTap`. Both `sim-use ios tap` (via + /// `execute()` above) and the top-level `tap` / `long-press` + /// forwarders call this directly with their parsed groups, so no + /// backend command instance is ever hand-built and the + /// wrapper-definition trap (#41/#42) cannot occur on this path. + /// Callers are expected to have run the shared validators first. + public static func performTap( alias: String?, - pointX: Double?, - pointY: Double?, - point: CoordinatePair?, - elementID: String?, - elementLabel: String?, - elementValue: String?, - labelContains: String?, - labelRegex: String?, - preDelay: Double?, - postDelay: Double?, + targeting: TapTargetingOptions, + timing: TapTimingOptions, duration: Double?, - waitTimeout: Double, - pollInterval: Double, - frameSpecs: [String] - ) throws { - if let alias { - guard OutlineAliasResolver.looksLikeAlias(alias) else { - throw ValidationError("Positional alias '\(alias)' must be `@N`, `#N`, `#N@M`, or `#`.") - } - var conflicts: [String] = [] - if pointX != nil { conflicts.append("-x") } - if pointY != nil { conflicts.append("-y") } - if point != nil { conflicts.append("--point") } - if elementID != nil { conflicts.append("--id") } - if elementLabel != nil { conflicts.append("--label") } - if elementValue != nil { conflicts.append("--value") } - if labelContains != nil { conflicts.append("--label-contains") } - if labelRegex != nil { conflicts.append("--label-regex") } - if !conflicts.isEmpty { - throw ValidationError("Alias '\(alias)' cannot be combined with \(conflicts.joined(separator: ", ")).") - } - } else if pointX != nil || pointY != nil || point != nil { - _ = try TapCoordinateResolver.resolve(x: pointX, y: pointY, point: point) - } else { - let selectors: [(String, String?)] = [ - ("--id", elementID), - ("--label", elementLabel), - ("--value", elementValue), - ("--label-contains", labelContains), - ("--label-regex", labelRegex), - ] - let provided = selectors.filter { $0.1 != nil } - if provided.isEmpty { - throw ValidationError("Either provide an `@N` / `#N` / `#N@M` alias, coordinates (--point x,y or both -x/-y), or use --id/--label/--value/--label-contains/--label-regex to tap an element.") - } - if provided.count > 1 { - let names = provided.map(\.0).joined(separator: ", ") - throw ValidationError("Use only one of --id, --label, --value, --label-contains, --label-regex (got: \(names)).") - } - for (name, raw) in provided { - if let raw, raw.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { - throw ValidationError("\(name) must not be empty.") - } - } - if let labelRegex { - do { - _ = try NSRegularExpression(pattern: labelRegex, options: []) - } catch { - throw ValidationError("--label-regex '\(labelRegex)' is not a valid regular expression: \(error.localizedDescription)") - } - } - } - - if let preDelay = preDelay { - guard preDelay >= 0 && preDelay <= 10.0 else { - throw ValidationError("Pre-delay must be between 0 and 10 seconds.") - } - } - - if let postDelay = postDelay { - guard postDelay >= 0 && postDelay <= 10.0 else { - throw ValidationError("Post-delay must be between 0 and 10 seconds.") - } - } - - if let duration { - guard duration >= 0 && duration <= 10.0 else { - throw ValidationError("--duration must be between 0 and 10 seconds.") - } - } - - guard waitTimeout >= 0 else { - throw ValidationError("--wait-timeout must be non-negative.") - } - - if waitTimeout > 0 { - guard pollInterval > 0 else { - throw ValidationError("--poll-interval must be greater than 0 when --wait-timeout is active.") - } - } - - if !frameSpecs.isEmpty { - do { - _ = try AccessibilityTargetResolver.FrameFilter(specs: frameSpecs) - } catch let error as AccessibilityTargetResolver.FrameFilter.ParseError { - throw ValidationError(error.message) - } - - if pointX != nil || pointY != nil || point != nil { - throw ValidationError("--frame cannot be combined with explicit -x/-y/--point coordinates (those bypass the AX tree).") - } - if let alias, case .some(let parsed) = OutlineAliasResolver.parse(alias) { - switch parsed { - case .at, .list: - throw ValidationError("--frame cannot be combined with the @N / #N / #N@M alias forms (they resolve to cached coordinates without consulting the AX tree). Use --label / --label-contains / --label-regex / --id / # with --frame instead.") - case .id: - break - } - } - } - } - - public func execute() async throws -> ExecutionResult { + multiTouch: MultiTouchOptions, + device: DeviceOptions, + json: JSONOutputOptions + ) async throws -> ExecutionResult { let logger = SimUseLogger() - try await setup(logger: logger) + let jsonOutput = json.enabled + let frameFilter = Self.frameFilter(from: targeting) + try await performEssentialSetup(logger: logger) try await performGlobalSetup(logger: logger) let resolvedPoint: (x: Double, y: Double) @@ -307,9 +169,9 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { let hidTarget = try await AccessibilityPoller.resolveWithPollingHIDTarget( query: .id(uniqueId), simulatorUDID: device.resolved, - waitTimeout: waitTimeout, - pollInterval: pollInterval, - elementType: elementType, + waitTimeout: timing.waitTimeout, + pollInterval: timing.pollInterval, + elementType: targeting.elementType, frameFilter: frameFilter, logger: logger ) @@ -326,22 +188,22 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { case nil: throw CLIError(errorDescription: "Internal error: alias '\(alias)' passed validation but could not be parsed.") } - } else if let explicit = try TapCoordinateResolver.resolve(x: pointX, y: pointY, point: point) { + } else if let explicit = try TapCoordinateResolver.resolve(x: targeting.pointX, y: targeting.pointY, point: targeting.point) { resolvedPoint = (x: explicit.x, y: explicit.y) resolvedDescription = "(\(explicit.x), \(explicit.y))" resolvedAdvisory = nil calibration = nil } else { let query: AccessibilityQuery - if let elementID { + if let elementID = targeting.elementID { query = .id(elementID) - } else if let elementLabel { + } else if let elementLabel = targeting.elementLabel { query = .label(elementLabel) - } else if let elementValue { + } else if let elementValue = targeting.elementValue { query = .value(elementValue) - } else if let labelContains { + } else if let labelContains = targeting.labelContains { query = .labelContains(labelContains) - } else if let labelRegex { + } else if let labelRegex = targeting.labelRegex { query = .labelRegex(pattern: labelRegex) } else { throw CLIError(errorDescription: "Unexpected state: no coordinates and no element query.") @@ -351,9 +213,9 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { let hidTarget = try await AccessibilityPoller.resolveWithPollingHIDTarget( query: query, simulatorUDID: device.resolved, - waitTimeout: waitTimeout, - pollInterval: pollInterval, - elementType: elementType, + waitTimeout: timing.waitTimeout, + pollInterval: timing.pollInterval, + elementType: targeting.elementType, frameFilter: frameFilter, logger: logger ) @@ -380,7 +242,7 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { logger.info().log("Orientation \(calibration.orientation.rawValue): dispatching HID at (\(dispatchPoint.x), \(dispatchPoint.y))") } - if let preDelay = preDelay, preDelay > 0 { + if let preDelay = timing.preDelay, preDelay > 0 { logger.info().log("Pre-delay: \(preDelay)s") try await Task.sleep(nanoseconds: UInt64(preDelay * 1_000_000_000)) } @@ -447,7 +309,7 @@ public struct IOSSimTapCommand: SimUseExecutableCommand { ) } - if let postDelay = postDelay, postDelay > 0 { + if let postDelay = timing.postDelay, postDelay > 0 { logger.info().log("Post-delay: \(postDelay)s") try await Task.sleep(nanoseconds: UInt64(postDelay * 1_000_000_000)) } diff --git a/Tests/ForwarderInitializationGuardTests.swift b/Tests/ForwarderInitializationGuardTests.swift new file mode 100644 index 0000000..d34f692 --- /dev/null +++ b/Tests/ForwarderInitializationGuardTests.swift @@ -0,0 +1,201 @@ +// SPDX-License-Identifier: Apache-2.0 +@testable import SimUse +@testable import iOSSimBackend +import ArgumentParser +import Foundation +import SimUseCore +import Testing + +// Guards the construct-and-assign forwarder pattern (#42): a top-level +// cross-platform verb builds its iOS backend command via the empty +// `init()` and copies every parsed property by hand. A missed +// `sub.field = field` line leaves that property in ArgumentParser's +// wrapper-definition state, and the first read traps with "can't read a +// value from a parsable argument definition" — at runtime, on a device, +// where no unit test used to look. +// +// The tap family (`tap` / `long-press`) has no guard test here by +// design: it forwards through `IOSSimTapCommand.performTap`, handing +// its parsed groups over as values — no backend instance is ever +// hand-built on that path. Verbs migrated to that executor pattern +// drop out of this suite together with their copy code. +// +// Each test parses a valid argv, calls the forwarder's +// `makeIOSSubcommand()`, and audits the result with `Mirror`: every +// ArgumentParser property wrapper must be out of definition state. +// Parsing initializes *every* wrapper (absent flags land in their +// nil/default value state), so any valid argv exercises the full copy +// surface — the audit is about wrapper state, not values. Value +// fidelity (copying from the *right* source field) stays with the +// per-verb parity tests. +// +// The audit peeks at ArgumentParser internals (`_parsedValue`, pinned +// at 1.5.0) and fails closed: an unrecognized wrapper shape or a walk +// that finds zero wrappers is a test failure, never a silent pass — an +// ArgumentParser upgrade can break this test loudly, but cannot quietly +// blind it. + +/// Result of Mirror-walking a `ParsableArguments` instance. +private struct WrapperAudit { + /// Property paths still in wrapper-definition state — reading any + /// of these at runtime would trap. + var uninitialized: [String] = [] + /// Paths that look like ArgumentParser wrappers but whose internals + /// don't match the pinned 1.5.0 shape. + var malformed: [String] = [] + /// Wrappers successfully classified; zero means the walk is blind. + var recognized = 0 + + init(of instance: Any) { + walk(instance, path: "") + } + + private mutating func walk(_ instance: Any, path: String) { + for child in Mirror(reflecting: instance).children { + guard let label = child.label, label.hasPrefix("_") else { continue } + let name = path + label.dropFirst() + let typeName = String(describing: type(of: child.value)) + let isWrapper = ["Option<", "Argument<", "Flag<", "OptionGroup<"] + .contains { typeName.hasPrefix($0) } + guard isWrapper else { continue } + + guard let parsed = Mirror(reflecting: child.value).children + .first(where: { $0.label == "_parsedValue" }) + else { + malformed.append("\(name): \(typeName) has no _parsedValue") + continue + } + let parsedMirror = Mirror(reflecting: parsed.value) + guard parsedMirror.displayStyle == .enum, + let caseChild = parsedMirror.children.first, + let caseLabel = caseChild.label + else { + malformed.append("\(name): Parsed shape unrecognized") + continue + } + switch caseLabel { + case "value": + recognized += 1 + // An OptionGroup in value state carries a nested + // ParsableArguments whose own wrappers may still be in + // definition state — recurse. + if let nested = caseChild.value as? any ParsableArguments { + walk(nested, path: name + ".") + } + case "definition": + recognized += 1 + uninitialized.append(name) + default: + malformed.append("\(name): unknown Parsed case '\(caseLabel)'") + } + } + } +} + +private func assertFullyInitialized( + _ instance: some ParsableArguments, + sourceLocation: SourceLocation = #_sourceLocation +) { + let audit = WrapperAudit(of: instance) + #expect( + audit.malformed.isEmpty, + "ArgumentParser internals no longer match the pinned 1.5.0 shape — update WrapperAudit: \(audit.malformed)", + sourceLocation: sourceLocation + ) + #expect( + audit.recognized > 0, + "Audit found no ArgumentParser wrappers at all — the guard is blind (internals renamed?)", + sourceLocation: sourceLocation + ) + #expect( + audit.uninitialized.isEmpty, + "Properties still in wrapper-definition state — missed `sub.field = field` in makeIOSSubcommand? \(audit.uninitialized)", + sourceLocation: sourceLocation + ) +} + +@Suite("Forwarder initialization guard") +struct ForwarderInitializationGuardTests { + private let iosUDID = "9CD7C6E7-45B3-4E59-BBF2-4D12A9457CD0" + + // Meta-test: the audit must actually detect definition state, or + // every green result below is vacuous. A hand-built command with + // nothing assigned is the maximal violation. + @Test("audit detects a hand-built, unassigned command") + func auditDetectsDefinitionState() { + let audit = WrapperAudit(of: IOSSimTapCommand()) + #expect(!audit.uninitialized.isEmpty) + #expect(audit.malformed.isEmpty) + } + + // Meta-test: a parsed command (never hand-copied) must audit clean — + // proves the walk doesn't misclassify nil/default values as + // uninitialized. + @Test("audit passes a directly parsed command") + func auditPassesParsedCommand() throws { + assertFullyInitialized(try IOSSimTapCommand.parse(["@1", "--udid", iosUDID])) + } + + @Test("paste forwarder copies every field") + func paste() throws { + assertFullyInitialized(try Paste.parse(["hello", "--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("screenshot forwarder copies every field") + func screenshot() throws { + assertFullyInitialized(try Screenshot.parse(["--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("swipe forwarder copies every field") + func swipe() throws { + assertFullyInitialized( + try Swipe.parse(["100,200", "300,400", "--udid", iosUDID]).makeIOSSubcommand() + ) + } + + @Test("type forwarder copies every field") + func type() throws { + assertFullyInitialized(try Type.parse(["hello", "--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("record-video forwarder copies every field") + func recordVideo() throws { + assertFullyInitialized(try RecordVideo.parse(["--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("multi-touch forwarder copies every field") + func multiTouch() throws { + assertFullyInitialized(try MultiTouch.parse([ + "--x1", "195", "--y1", "422", "--x2", "195", "--y2", "522", + "--x1-end", "195", "--y1-end", "222", "--x2-end", "195", "--y2-end", "622", + "--udid", iosUDID, + ]).makeIOSSubcommand()) + } + + @Test("button forwarder copies every field") + func button() throws { + assertFullyInitialized(try Button.parse(["home", "--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("touch forwarder copies every field") + func touch() throws { + assertFullyInitialized(try Touch.parse([ + "-x", "100", "-y", "200", "--down", "--up", "--udid", iosUDID, + ]).makeIOSSubcommand()) + } + + @Test("describe-ui forwarder copies every field") + func describeUI() throws { + assertFullyInitialized(try DescribeUI.parse(["--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("keyboard-state forwarder copies every field") + func keyboardState() throws { + assertFullyInitialized(try KeyboardState.parse(["--udid", iosUDID]).makeIOSSubcommand()) + } + + @Test("gesture forwarder copies every field") + func gesture() throws { + assertFullyInitialized(try Gesture.parse(["scroll-up", "--udid", iosUDID]).makeIOSSubcommand()) + } +} diff --git a/Tests/TapForwarderTests.swift b/Tests/TapForwarderTests.swift index 1b13876..4464e48 100644 --- a/Tests/TapForwarderTests.swift +++ b/Tests/TapForwarderTests.swift @@ -10,9 +10,11 @@ import Testing // Pins the contract between the top-level `Tap` and the // `IOSSimTapCommand` it forwards to: // -// * Both surfaces validate input through one shared rules table -// (`IOSSimTapCommand.validateOptions`). The top-level command's -// `validate()` is required to delegate, not to re-implement. +// * Both surfaces validate input through the shared group validators +// (`TapTargetingOptions.validate(alias:)` / +// `TapTimingOptions.validate()`). Cross-surface message equality is +// pinned by `TapValidationParityTests`; the cases below pin the +// rules themselves at the group level. // * `PlatformRouter.resolve(udid:)` is what picks the branch in // `Tap.execute()`. The test cases below cover the two real-world // UDID shapes plus the empty / typo case (defers to iOS so the @@ -69,48 +71,29 @@ struct TapForwarderTests { // MARK: - Validation parity - @Test("Top-level Tap validation delegates to IOSSimTapCommand") - func topLevelTapDelegatesValidation() throws { - // Construct a Tap with conflicting selectors. The top-level - // `validate()` should throw the same message - // `IOSSimTapCommand.validateOptions` would throw — proves - // the delegation is not a re-implementation. + @Test("Shared targeting validator rejects conflicting selectors") + func conflictingSelectorsRejected() throws { + // The rules live on the shared group — parse a targeting group + // with conflicting selectors and run the validator directly. + // (Direct `TapTargetingOptions()` construction would trap on + // first read — the #41/#42 failure class — so tests go through + // `parse` like every real surface does.) + let targeting = try TapTargetingOptions.parse(["--id", "Some", "--label", "Other"]) do { - try IOSSimTapCommand.validateOptions( - alias: nil, - pointX: nil, pointY: nil, point: nil, - elementID: "Some", - elementLabel: "Other", - elementValue: nil, - labelContains: nil, - labelRegex: nil, - preDelay: nil, postDelay: nil, duration: nil, - waitTimeout: 0, - pollInterval: 0.25, - frameSpecs: [] - ) + try targeting.validate(alias: nil) Issue.record("expected a ValidationError") } catch let error as ValidationError { #expect(error.message.contains("Use only one of")) + } catch { + Issue.record("expected ValidationError, got \(type(of: error))") } } @Test("Alias-with-coordinates is rejected by shared validator") - func aliasWithCoordsConflict() { + func aliasWithCoordsConflict() throws { + let targeting = try TapTargetingOptions.parse(["-x", "100", "-y", "200"]) do { - try IOSSimTapCommand.validateOptions( - alias: "@2", - pointX: 100, pointY: 200, point: nil, - elementID: nil, - elementLabel: nil, - elementValue: nil, - labelContains: nil, - labelRegex: nil, - preDelay: nil, postDelay: nil, duration: nil, - waitTimeout: 0, - pollInterval: 0.25, - frameSpecs: [] - ) + try targeting.validate(alias: "@2") Issue.record("expected a ValidationError") } catch let error as ValidationError { #expect(error.message.contains("Alias '@2' cannot be combined")) @@ -121,44 +104,24 @@ struct TapForwarderTests { @Test("Wait timeout requires positive poll interval") func waitTimeoutRequiresPollInterval() { + // `TapTimingOptions.validate()` is the ParsableArguments + // witness, and a *standalone* `parse` of the group wraps it in + // a synthetic root command — so the rule fires during parse + // (unlike when the group is nested in a command, where the + // command's explicit `timing.validate()` call is load-bearing). do { - try IOSSimTapCommand.validateOptions( - alias: nil, - pointX: nil, pointY: nil, point: nil, - elementID: "X", - elementLabel: nil, - elementValue: nil, - labelContains: nil, - labelRegex: nil, - preDelay: nil, postDelay: nil, duration: nil, - waitTimeout: 5, - pollInterval: 0, - frameSpecs: [] - ) - Issue.record("expected a ValidationError") - } catch let error as ValidationError { - #expect(error.message.contains("--poll-interval")) + _ = try TapTimingOptions.parse(["--wait-timeout", "5", "--poll-interval", "0"]) + Issue.record("expected a validation failure") } catch { - Issue.record("expected ValidationError, got \(type(of: error))") + #expect(TapTimingOptions.message(for: error).contains("--poll-interval")) } } @Test("Empty selector value is rejected") - func emptySelectorValueRejected() { + func emptySelectorValueRejected() throws { + let targeting = try TapTargetingOptions.parse(["--id", " "]) do { - try IOSSimTapCommand.validateOptions( - alias: nil, - pointX: nil, pointY: nil, point: nil, - elementID: " ", - elementLabel: nil, - elementValue: nil, - labelContains: nil, - labelRegex: nil, - preDelay: nil, postDelay: nil, duration: nil, - waitTimeout: 0, - pollInterval: 0.25, - frameSpecs: [] - ) + try targeting.validate(alias: nil) Issue.record("expected a ValidationError") } catch let error as ValidationError { #expect(error.message.contains("must not be empty")) @@ -259,12 +222,12 @@ struct TapForwarderTests { let subCmd = try IOSSimTapCommand.parse(argv) // Cross-check a sample of fields landed in both structs. - #expect(topLevel.labelContains == "foo") - #expect(subCmd.labelContains == "foo") - #expect(topLevel.elementType == "Button") - #expect(subCmd.elementType == "Button") - #expect(topLevel.frameSpecs == ["minX=10,maxX=200"]) - #expect(subCmd.frameSpecs == ["minX=10,maxX=200"]) + #expect(topLevel.targeting.labelContains == "foo") + #expect(subCmd.targeting.labelContains == "foo") + #expect(topLevel.targeting.elementType == "Button") + #expect(subCmd.targeting.elementType == "Button") + #expect(topLevel.targeting.frameSpecs == ["minX=10,maxX=200"]) + #expect(subCmd.targeting.frameSpecs == ["minX=10,maxX=200"]) #expect(topLevel.duration == 0.05) #expect(subCmd.duration == 0.05) #expect(topLevel.jsonOutput == true) @@ -279,9 +242,9 @@ struct TapForwarderTests { // through `IOSSimTapCommand` itself. let expected = CoordinatePair(x: 100, y: 200) let iosArgv = ["--point", "100,200", "--udid", "9CD7C6E7-45B3-4E59-BBF2-4D12A9457CD0"] - #expect(try Tap.parse(iosArgv).point == expected) - #expect(try IOSSimTapCommand.parse(iosArgv).point == expected) - #expect(try LongPress.parse(iosArgv).point == expected) + #expect(try Tap.parse(iosArgv).targeting.point == expected) + #expect(try IOSSimTapCommand.parse(iosArgv).targeting.point == expected) + #expect(try LongPress.parse(iosArgv).targeting.point == expected) let androidArgv = ["--point", "100,200", "--udid", "emulator-5554"] #expect(try AndroidTapCommand.parse(androidArgv).point == expected) @@ -338,12 +301,12 @@ struct TapForwarderTests { let topTap = try Tap.parse(argv) let topLong = try LongPress.parse(argv) - #expect(topLong.labelContains == topTap.labelContains) - #expect(topLong.elementType == topTap.elementType) - #expect(topLong.frameSpecs == topTap.frameSpecs) - #expect(topLong.preDelay == topTap.preDelay) - #expect(topLong.waitTimeout == topTap.waitTimeout) - #expect(topLong.pollInterval == topTap.pollInterval) + #expect(topLong.targeting.labelContains == topTap.targeting.labelContains) + #expect(topLong.targeting.elementType == topTap.targeting.elementType) + #expect(topLong.targeting.frameSpecs == topTap.targeting.frameSpecs) + #expect(topLong.timing.preDelay == topTap.timing.preDelay) + #expect(topLong.timing.waitTimeout == topTap.timing.waitTimeout) + #expect(topLong.timing.pollInterval == topTap.timing.pollInterval) #expect(topLong.jsonOutput == topTap.jsonOutput) // Default --duration: Tap leaves it nil (no hold), long-press diff --git a/Tests/TapValidationParityTests.swift b/Tests/TapValidationParityTests.swift new file mode 100644 index 0000000..bbe2608 --- /dev/null +++ b/Tests/TapValidationParityTests.swift @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: Apache-2.0 +@testable import SimUse +@testable import iOSSimBackend +import ArgumentParser +import Foundation +import SimUseCore +import Testing + +// ArgumentParser (1.5.0) does not auto-validate nested option groups — +// each of the three tap surfaces must call the shared group validators +// (`TapTargetingOptions.validate(alias:)` / `TapTimingOptions.validate()` +// / `TapTimingOptions.validateDuration`) explicitly from its own +// `validate()`. A surface that drops a call still parses fine and loses +// validation silently; that is the regression this suite exists to +// catch. The same invalid argv must fail — with the same message — on +// `tap`, `long-press`, and `ios tap`. +@Suite("Tap validation parity across surfaces") +struct TapValidationParityTests { + private static let udid = ["--udid", "9CD7C6E7-45B3-4E59-BBF2-4D12A9457CD0"] + + /// argv tails every tap surface must reject. `fragment` anchors + /// which rule fired; an empty fragment means the message text is + /// pinned only by cross-surface equality (resolver-owned wording). + private static let rejectedArgvs: [(argv: [String], fragment: String)] = [ + (["@1", "--label", "Foo"], "cannot be combined with --label"), + (["--label", "a", "--value", "b"], "Use only one of"), + (["--label", " "], "--label must not be empty."), + (["-x", "100"], ""), + (["--label", "a", "--label-regex", "(["], ""), + (["--label", "a", "--pre-delay", "11"], "Pre-delay must be between 0 and 10 seconds."), + (["--label", "a", "--post-delay", "11"], "Post-delay must be between 0 and 10 seconds."), + (["--label", "a", "--duration", "11"], "--duration must be between 0 and 10 seconds."), + (["--label", "a", "--wait-timeout=-1"], "--wait-timeout must be non-negative."), + (["--label", "a", "--wait-timeout", "1", "--poll-interval", "0"], "--poll-interval must be greater than 0"), + (["--label", "a", "--frame", "minX=abc"], "is not a number"), + (["--label", "a", "--frame", "banana=1"], "is unknown"), + (["-x", "1", "-y", "2", "--frame", "minY=1"], "--frame cannot be combined with explicit"), + (["@1", "--frame", "minY=1"], "cannot be combined with the @N / #N / #N@M alias forms"), + ] + + /// nil when the argv parses; the rendered parser message otherwise. + private func failureMessage(_ type: C.Type, _ argv: [String]) -> String? { + do { + _ = try type.parse(argv) + return nil + } catch { + return type.message(for: error) + } + } + + @Test("same invalid argv fails with the same message on every surface") + func rejectedParityAcrossSurfaces() { + for (argv, fragment) in Self.rejectedArgvs { + let full = argv + Self.udid + let tap = failureMessage(Tap.self, full) + let longPress = failureMessage(LongPress.self, full) + let iosTap = failureMessage(IOSSimTapCommand.self, full) + + #expect(tap != nil, "tap accepted invalid argv \(argv)") + #expect(longPress != nil, "long-press accepted invalid argv \(argv)") + #expect(iosTap != nil, "ios tap accepted invalid argv \(argv)") + #expect(tap == longPress, "tap vs long-press message mismatch for \(argv): '\(tap ?? "nil")' vs '\(longPress ?? "nil")'") + #expect(tap == iosTap, "tap vs ios tap message mismatch for \(argv): '\(tap ?? "nil")' vs '\(iosTap ?? "nil")'") + if !fragment.isEmpty { + #expect(tap?.contains(fragment) == true, "message for \(argv) missing '\(fragment)': '\(tap ?? "nil")'") + } + } + } + + @Test("a kitchen-sink valid argv parses on every surface") + func acceptedParityAcrossSurfaces() { + // One selector + type filter + frame + full timing block — + // valid on all three surfaces (mirrors flagSurfaceParses, which + // pins field values; this pins acceptance stays in sync with + // the rejection table above). + let argv = [ + "--label-contains", "foo", + "--element-type", "Button", + "--frame", "minY=0.5r", + "--pre-delay", "0.1", "--post-delay", "0.1", + "--duration", "0.5", + "--wait-timeout", "1.0", "--poll-interval", "0.5", + ] + Self.udid + #expect(failureMessage(Tap.self, argv) == nil) + #expect(failureMessage(LongPress.self, argv) == nil) + #expect(failureMessage(IOSSimTapCommand.self, argv) == nil) + } +} diff --git a/docs/ai/xxxx-tap-optiongroup-consolidation/README.md b/docs/ai/xxxx-tap-optiongroup-consolidation/README.md new file mode 100644 index 0000000..417cc73 --- /dev/null +++ b/docs/ai/xxxx-tap-optiongroup-consolidation/README.md @@ -0,0 +1,238 @@ +# Tap-family shared OptionGroups and forwarder copy elimination + +Plan and work record for issue #42 — consolidate the tap-family flag +surface into shared `OptionGroup`s and remove the forwarder copy hazard. +Written before implementation and kept as the design rationale; all +three steps below are implemented (the *(plan)* markers record what was +intended — the shipped code matches). Prerequisites #40 (`tap --point`) +and #41 (`android tap` direct-init trap) are both merged. + +## Outcome + +- Step 1 (guardrail), Step 2 (shared groups), Step 3 (executor entry + point) landed as three commits on one branch. Net effect: the three + tap surfaces share one declaration of 14 flags; the tap family + forwards parsed groups by value through + `IOSSimTapCommand.performTap` (no construct-and-assign left); the 11 + verbs still on the copy pattern are pinned by the reflection guard. +- Verification: unit suite 1079 (baseline) → 1094 green; before/after + `--help` golden diff showed only the two documented deltas (wording, + ordering); live spot-check on a booted simulator covered + `tap --point`, `tap @N`, `long-press` by selector, and both `--json` + envelope paths; full E2E run green before the PR. +- Implementation findings worth keeping: ArgumentParser runs a group's + protocol `validate()` witness when the group is parsed *standalone* + (it wraps it in a synthetic root command) but not when nested — the + explicit per-surface calls remain load-bearing. `Paste.clientPreflight` + turned out to be a deliberately partial copy (three fields); it now + reuses the full-copy `makeIOSSubcommand()`. + +## Problem + +Three surfaces declare the same ~15 tap flags by hand, and the top-level +forwarders bridge them by constructing the backend command via its empty +`init()` and assigning every property individually: + +| Surface | File | Role | +|---|---|---| +| `sim-use tap` | `Sources/SimUse/Commands/Tap.swift` | cross-platform forwarder (19-field copy in `executeIOSSim`) | +| `sim-use long-press` | `Sources/SimUse/Commands/LongPress.swift` | same copy block with `duration` carried through | +| `sim-use ios tap` | `Sources/iOSSimBackend/Verbs/IOSSimTapCommand.swift` | backend + the actual `execute()` | + +A `ParsableArguments` property that is never assigned stays in +wrapper-definition state; the first read traps with ArgumentParser's +*"can't read a value from a parsable argument definition"* fatal — the +failure class of #41. The hazard: add a flag to the shared surface, forget +one `sub.field = field` line, and every existing test stays green. The +parity tests (`flagSurfaceParses`, `pointFlagParsesEverywhere` in +`Tests/TapForwarderTests.swift`) only pin that the same argv *parses* on +every surface; nothing executes the forwarder copy without a live device. + +The pattern is repo-wide: **14 construct-and-assign sites across 13 +forwarders** (Paste has two paths) — Tap, LongPress, Paste, Screenshot, +Swipe, Type, RecordVideo, MultiTouch, Button, Touch, DescribeUI, +KeyboardState, Gesture. + +## Decision history + +The issue proposed two layers: shared `OptionGroup`s (structural) plus a +reflection-based forwarder-completeness test (guardrail). A review comment +(@hiSandog) pushed further: the shared group should be *the* parsed value +passed into the backend, not copied into a reconstructed backend command — +and the missing test is one that asserts the backend receives the full +value object. + +**Adopted, with one adjustment.** The clincher for the structural point is +in-repo precedent: `AndroidTapCommand.performTap(udid:alias:x:y:selector:duration:multiTouch:controller:)` +already is that pattern — a typed static entry point the forwarder calls +directly, with an injectable `controller` seam that +`androidTapDefaultMultiTouchDoesNotTrap` exercises without a device. The +iOS side gets the same shape. The adjustment: "execute a forwarded command +and assert" cannot run at unit level (the iOS entry point does real HID +dispatch), so real execution stays with the scripted E2E layer +(`make e2e-ios`) and the unit layer asserts at the entry-point boundary +instead. Once forwarders pass whole groups, tap-family completeness is a +compile-time property; the reflection guardrail covers the 13 forwarders +that keep the copy pattern until each is migrated. + +## Plan + +Three steps, each landable as its own PR, in this order. + +### Step 1 — reflection guardrail *(plan)* + +Safety net first: it protects both the 13 non-migrated forwarders and the +step-2/3 refactor itself. + +- `Tests/` gains a generic `assertFullyInitialized(_: some ParsableArguments)` + helper. It `Mirror`-walks the instance's property wrappers (`Argument`, + `Option`, `Flag`, `OptionGroup`), recurses into nested + `ParsableArguments`, and fails on any wrapper whose internal + `_parsedValue` is still in `definition` state. +- **Fail closed:** an unrecognized wrapper shape (e.g. after an + ArgumentParser upgrade renames internals) is a test *failure*, never a + silent pass — the guard cannot rot into a no-op. The dependency is + version-pinned, so this fires loudly at upgrade time, which is the + acceptable trade-off for peeking at internals. +- Each forwarder's construct+copy block is extracted into a testable + `makeIOSSubcommand()` (pure, no device access), and each forwarder gets + one test: parse **any valid argv** → `makeIOSSubcommand()` → + `assertFullyInitialized`. A maximal argv is neither possible (tap's + targeting flags are mutually exclusive, and `parse` runs `validate()`) + nor needed: parsing initializes *every* wrapper — absent flags land in + their nil/default `.value` state — so a forgotten `sub.field = field` + line stays in `.definition` state regardless of what the argv + contained. Because `Mirror` enumerates whatever wrappers actually + exist, future fields are covered automatically — no manual field list + that can itself be forgotten. +- **Known limit:** the guard checks initialization, not value fidelity — + copying from the wrong source field (`sub.pointX = pointY`) passes it. + Value fidelity stays with the parity tests' field assertions. + +### Step 2 — shared OptionGroups *(plan)* + +Following the `MultiTouchOptions` / `SwipeCoordinateOptions` / +`DeviceOptions` / `JSONOutputOptions` precedent, two new groups under +`Sources/SimUseCore/Options/`: + +- **`TapTargetingOptions`** — `-x`/`-y`/`--point`, the five selectors + (`--id`/`--label`/`--value`/`--label-contains`/`--label-regex`), + `--element-type`, `--frame`. +- **`TapTimingOptions`** — `--pre-delay`/`--post-delay`/`--wait-timeout`/ + `--poll-interval`. + +`IOSSimTapCommand.validateOptions` splits onto the groups, mirroring +`MultiTouchOptions.validate()`: selector/coordinate/frame exclusivity +rules → `TapTargetingOptions.validate(alias:)` — the alias-conflict and +frame×alias rules cross the group boundary (the positional stays +per-command), so `alias` comes in as a parameter; delay/poll range rules → +`TapTimingOptions.validate()`. The `--duration` range check stays with the +commands that own the flag. + +**ArgumentParser (pinned 1.5.0) does not auto-validate option groups** — +only the root command's `validate()` runs (`CommandParser.swift:188`); +the repo already calls `multiTouch.validate()` explicitly for exactly +this reason. Each of the three surfaces must call the group validators +from its own `validate()`; forgetting one silently drops validation on +that surface while parsing still succeeds. The validation-parity tests +below exist to catch precisely this failure mode. + +Deliberate per-command leftovers (the exceptions need a reason; this is +it): + +- **`--duration`** — differs between `tap` (nil default, latency-focused + help) and `long-press` (0.8 default, threshold-focused help). +- **`alias` positional** — stays per-command by default (verb-specific + help wording). Whether it joins the group is decided in the same + golden-file diff review that pins the ordering delta below. + +**Known surface deltas** (accepted, pinned by the help golden-file diff): + +1. *Wording* — the issue's scope note says help text must not change, but + `Tap` and `LongPress` currently word their flag help by verb ("Tap the + center…" vs "Long-press the center…"). Sharing a group forces one + wording; it becomes verb-neutral ("Target the element matching + AXLabel…") rather than being worked around with generic help + parameterization. +2. *Ordering* — `--duration` currently renders between `--post-delay` and + `--wait-timeout` in help output; with the timing flags grouped and + `--duration` per-command it must move relative to the group block. + Grouping reorders the help listing; the diff review checks + flag-by-flag content, not position. + +Flag names, defaults, validation messages, and `--json` envelopes remain +byte-identical. + +### Step 3 — tap-family executor entry point *(plan)* + +Mirror the Android shape on the iOS side: + +```swift +extension IOSSimTapCommand { + public static func performTap( + alias: String?, + targeting: TapTargetingOptions, + timing: TapTimingOptions, + duration: Double?, + multiTouch: MultiTouchOptions, + device: DeviceOptions, + json: JSONOutputOptions + ) async throws -> ExecutionResult +} +``` + +- `IOSSimTapCommand.execute()` becomes a thin call passing its own parsed + values — `sim-use ios tap` behavior unchanged. +- `Tap.executeIOSSim` / `LongPress.executeIOSSim` call `performTap` + directly. **The construct-and-assign block is deleted for the tap + family**; no `IOSSimTapCommand` instance is ever hand-built, so the + uninitialized-wrapper state cannot exist on this path. +- The step-1 guardrail tests for `Tap`/`LongPress` are deleted together + with the copy code they guard. +- The daemon path is unaffected: the daemon re-parses argv in its own + process, so execution always starts from a properly parsed instance. + +New group fields now appear on every surface and flow through forwarding +with zero forwarder edits; the residual risk is confined to the two +explicit loose parameters (`alias`, `duration`), which are visible in the +function signature rather than buried in a 19-line copy block. + +## Testing + +| Layer | What | When | +|---|---|---| +| Existing parity tests | same argv parses on all surfaces (`flagSurfaceParses`, `pointFlagParsesEverywhere`) | keep, unchanged | +| Step-1 guardrail | valid argv → `makeIOSSubcommand()` → `assertFullyInitialized`, per forwarder | new; tap-family cases retired in step 3 | +| Validation unit tests | move with the logic onto `TapTargetingOptions` / `TapTimingOptions`; error messages pinned byte-identical | step 2 | +| Validation-parity tests | table-driven: same invalid argv → same `ValidationError` text on all three surfaces (catches a surface that forgot to call a group validator) | step 2, new | +| Help output check | capture `--help` for `tap` / `long-press` / `ios tap` before step 2; diff after — only the two documented deltas (verb-neutral wording, group reordering) may appear | step 2 | +| Scripted E2E | `make e2e-ios` exercises real forwarded execution (HID dispatch) — this is where the comment's "execute a forwarded command" lives | before each PR merge | +| Live spot-check | `tap @N`, `long-press --label …`, `tap --point x,y` on a booted simulator | per CLAUDE.md verification baseline | + +Each step must pass `make build` + `make test` standalone. CHANGELOG: +one `### Changed` entry under `[Unreleased]` when step 2 lands (help +wording unification is the only user-visible delta); steps 1 and 3 are +internal. + +## Scope notes + +- **`AndroidTapCommand` stays out of the shared targeting group** — it + takes Int pixel coordinates and has Android-only `--value-contains` / + `--value-regex`. Whether it later shares the group with extras alongside + is a separate decision. +- Pre-existing inconsistency, noted but not fixed here: the top-level + `tap` forwarder hardcodes `valueContains: nil, valueRegex: nil` when + building `AndroidSelector`, so those selectors exist only on + `sim-use android tap`. +- The other 13 forwarders keep construct-and-assign + guardrail; + migrating each verb to an executor entry point is follow-up work, + verb-by-verb, in separate issues. + +## Open items + +- Decide during step 2 whether `alias` joins `TapTargetingOptions` + (folded into the help golden-file diff review). +- File follow-up issues for executor-pattern migration of the remaining + verbs once the tap family proves the shape. +- Consider whether `TapTimingOptions` fits any Android verbs when the + Android grouping question is revisited.