diff --git a/.github/scripts/folio-run.sh b/.github/scripts/folio-run.sh index de00edd..c3e2893 100755 --- a/.github/scripts/folio-run.sh +++ b/.github/scripts/folio-run.sh @@ -28,15 +28,12 @@ case "$platform" in examples/folio/app/androidApp/build/outputs/apk/debug/androidApp-debug.apk) ;; ios) - # --clear-data=false because the caller has just installed a fresh build (a - # freshly installed app IS clear state). The in-run reinstall path is worth - # avoiding here: `simctl uninstall` + `install` immediately followed by the - # XCTest runner's own launch hits "app.folio is unknown to FrontBoard" - # perhaps half the time. The launch RPC is bounded now, so that surfaces - # as an error rather than an indefinite hang, but a failed leg is still a - # failed leg and a fresh install is already clear state. + # Clear state is left at its default, and no --ios-app-path is passed, so + # the driver wipes the app's data container rather than reinstalling. That + # is what the calibrated numbers were measured from, and it keeps the run + # away from the `simctl uninstall` + `install` path that races FrontBoard + # ("app.folio is unknown to FrontBoard"), which needs an app path to reach. folio_args+=(--platform ios - --clear-data=false --ios-device "${IOS_DEVICE:-iPhone 16 Pro}") ;; web) @@ -192,7 +189,12 @@ if [ "$platform" = "android" ]; then *) echo "folio/android: the harness failed with exit $code" >&2; exit "$code" ;; esac if ! grep -q '"AddTransactionScreen"' "$trace"; then - echo "folio/android: the run never reached AddTransactionScreen, so it never got past login" >&2 + # Where it stopped is a much longer question than this gate answers, so + # name the routes the trace holds and leave the diagnosis to the reader. + reached=$(grep -oE '"[A-Za-z0-9]+Screen"' "$trace" | tr -d '"' | + awk '!seen[$0]++' | paste -sd, -) || reached="" + echo "folio/android: the run never reached AddTransactionScreen over $steps steps" >&2 + echo "folio/android: routes the trace does record: ${reached:-none}" >&2 exit 1 fi echo "folio/android: healthy run over $steps steps, reached the transaction screen" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4bef62..765f689 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,25 +19,25 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true - name: Set up JDK 17 - uses: actions/setup-java@v4 + uses: actions/setup-java@v5 with: distribution: temurin java-version: "17" - name: Set up Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@v4 - name: Set up Node 22 - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: "22" cache: npm @@ -49,7 +49,7 @@ jobs: bun-version: "1.3.13" - name: Cache bun store - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('replay-ui/bun.lock') }} @@ -74,7 +74,7 @@ jobs: echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" - name: Cache Gradle - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: | ~/.gradle/caches @@ -95,22 +95,34 @@ jobs: - name: Run tests run: make test + # folio is its own gradle build, and the metro plugin it compiles with + # needs a 21 runtime where the sidecar toolchain pins 17. Switching + # JAVA_HOME after `make test` rather than installing both up front + # leaves every step above this one on exactly the JDK it ran on before. + - name: Set up JDK 21 for folio + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: "21" + + - name: Run folio's unit tests + run: make test-folio + browser: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true - # Pin stable: the action's default (latest) pulls a dev Chromium whose - # remote-debugging socket is flaky under the driver, even though the - # browser otherwise launches headless. + # Pin stable: a dev Chromium's remote-debugging socket is flaky under + # the driver, even though the browser otherwise launches headless. - name: Set up Chrome - uses: browser-actions/setup-chrome@v1 + uses: browser-actions/setup-chrome@v2 with: chrome-version: stable diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index b3aa99d..b28cd02 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -22,7 +22,7 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Install pandoc run: sudo apt-get update && sudo apt-get install -y pandoc @@ -30,7 +30,7 @@ jobs: - name: Build site run: make docs - - uses: actions/upload-pages-artifact@v3 + - uses: actions/upload-pages-artifact@v5 with: path: build/site @@ -41,5 +41,5 @@ jobs: name: github-pages url: ${{ steps.deployment.outputs.page_url }} steps: - - uses: actions/deploy-pages@v4 + - uses: actions/deploy-pages@v5 id: deployment diff --git a/.github/workflows/folio.yml b/.github/workflows/folio.yml index fa237b0..72facbc 100644 --- a/.github/workflows/folio.yml +++ b/.github/workflows/folio.yml @@ -41,16 +41,16 @@ jobs: if: ${{ inputs.platforms == 'all' || inputs.platforms == 'android' }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true - name: Set up the JDKs - uses: actions/setup-java@v4 + uses: actions/setup-java@v5 with: distribution: temurin # The metro gradle plugin folio builds with needs a 21 runtime; the @@ -60,7 +60,7 @@ jobs: 21 - name: Set up Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@v4 - name: Set up bun uses: oven-sh/setup-bun@v2 @@ -68,7 +68,7 @@ jobs: bun-version: "1.3.13" - name: Cache Gradle - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: | ~/.gradle/caches @@ -109,7 +109,7 @@ jobs: - name: Upload the run if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: folio-android path: runs/ @@ -120,10 +120,10 @@ jobs: if: ${{ inputs.platforms == 'all' || inputs.platforms == 'ios' }} runs-on: macos-15 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true @@ -139,7 +139,7 @@ jobs: run: brew install facebook/fb/idb-companion xcodegen just - name: Set up the JDKs - uses: actions/setup-java@v4 + uses: actions/setup-java@v5 with: distribution: temurin # The metro gradle plugin folio builds with needs a 21 runtime; the @@ -152,13 +152,13 @@ jobs: # project, which configures :app:androidApp and so needs an Android SDK # even on this leg. - name: Set up Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@v4 # Both asset tarballs are built by the prepare scripts, and the runner # bundle is an xcodebuild of companion/Sources. Keyed on the scripts and # the versions the Makefile embeds, so a later run reuses them. - name: Cache the companion and runner bundles - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: | internal/driver/ioscompanion/companionassets/assets @@ -181,9 +181,8 @@ jobs: env: IOS_DEVICE: iPhone 16 Pro - # `just ios` leaves the app running, and the run's own clear-data - # reinstall on top of a live app has raced FrontBoard into refusing the - # launch ("app.folio is unknown to FrontBoard") with the run then hanging. + # `just ios` leaves the app running, and the run's first act is to clear + # its state. Stopping it here means the run always opens the same way. - name: Stop the app before the run run: xcrun simctl terminate booted app.folio || true @@ -197,7 +196,7 @@ jobs: - name: Upload the run if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: folio-ios path: runs/ @@ -208,16 +207,16 @@ jobs: if: ${{ inputs.platforms == 'all' || inputs.platforms == 'web' }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true - name: Set up the JDKs - uses: actions/setup-java@v4 + uses: actions/setup-java@v5 with: distribution: temurin # The metro gradle plugin folio builds with needs a 21 runtime; the @@ -232,7 +231,7 @@ jobs: bun-version: "1.3.13" - name: Cache Gradle - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: | ~/.gradle/caches @@ -242,7 +241,7 @@ jobs: folio-gradle-${{ runner.os }}- - name: Set up Chrome - uses: browser-actions/setup-chrome@v1 + uses: browser-actions/setup-chrome@v2 with: chrome-version: stable @@ -271,7 +270,7 @@ jobs: - name: Upload the run if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: folio-web path: runs/ diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 442acae..93f2c61 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,7 +22,7 @@ jobs: env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: ref: ${{ inputs.tag || github.ref }} @@ -33,7 +33,7 @@ jobs: echo "version=${raw#v}" >> "$GITHUB_OUTPUT" - name: Set up Node 22 - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: "22" registry-url: "https://registry.npmjs.org" @@ -65,28 +65,28 @@ jobs: permissions: contents: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: ref: ${{ inputs.tag || github.ref }} fetch-depth: 0 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true - name: Set up JDK 17 - uses: actions/setup-java@v4 + uses: actions/setup-java@v5 with: distribution: temurin java-version: "17" - name: Set up Android SDK - uses: android-actions/setup-android@v3 + uses: android-actions/setup-android@v4 - name: Cache Gradle - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: | ~/.gradle/caches @@ -99,7 +99,7 @@ jobs: run: make sidecar - name: Run GoReleaser - uses: goreleaser/goreleaser-action@v6 + uses: goreleaser/goreleaser-action@v7 with: version: "~> v2" args: release --clean diff --git a/.github/workflows/replay-ui.yml b/.github/workflows/replay-ui.yml index e72d477..6fb4f4a 100644 --- a/.github/workflows/replay-ui.yml +++ b/.github/workflows/replay-ui.yml @@ -27,10 +27,10 @@ jobs: timeout-minutes: 45 runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v7 with: go-version-file: go.mod cache: true @@ -43,7 +43,7 @@ jobs: # Pinned stable plus the AppArmor sysctl: the same setup ci.yml's browser # job needs to get headless Chrome up on ubuntu-latest. - name: Set up Chrome - uses: browser-actions/setup-chrome@v1 + uses: browser-actions/setup-chrome@v2 with: chrome-version: stable @@ -135,7 +135,7 @@ jobs: - name: Upload runs if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: replay-ui-runs path: runs/ diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..8755b39 --- /dev/null +++ b/LICENSE @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright 2026 Priyanshu Jain + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/Makefile b/Makefile index 46ad520..a6607fa 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,7 @@ WEB_DIST := replay-ui/dist GOLINES := $(shell $(GO) env GOPATH)/bin/golines -.PHONY: bootstrap proto sidecar sidecar-embed sanderling sanderling-web sanderling-android sanderling-ios install test test-go test-browser test-companion test-kotlin test-spec-api test-ci-scripts spec-typecheck web-test web-typecheck web-build web-dev replay-dev docs clean release-cli release-npm-dry fmt fmt-go fmt-kotlin fmt-ts fmt-swift +.PHONY: bootstrap proto sidecar sidecar-embed sanderling sanderling-web sanderling-android sanderling-ios install test test-go test-browser test-companion test-kotlin test-folio test-spec-api test-ci-scripts spec-typecheck web-test web-typecheck web-build web-dev replay-dev docs clean release-cli release-npm-dry fmt fmt-go fmt-kotlin fmt-ts fmt-swift bootstrap: $(GO) mod download @@ -145,6 +145,14 @@ test-companion: $(COMPANION_EMBED) $(RUNNER_EMBED) test-kotlin: ANDROID_HOME=$(ANDROID_HOME) $(GRADLE) :sidecar:test +# folio is its own gradle build, so nothing in the root build runs its tests. +# Kept out of `test` because the metro plugin folio compiles with needs a 21+ +# runtime, where the sidecar toolchain pins 17: folding this in would raise the +# JDK floor of the target everyone runs constantly. CI runs it as its own step. +test-folio: + cd examples/folio && ANDROID_HOME=$(ANDROID_HOME) $(GRADLE) \ + :core:testDebugUnitTest :app:shared:testDebugUnitTest + # The CI scripts that read a trace and decide whether a green leg is # evidence. bash and python3 only, which is all a runner has. test-ci-scripts: diff --git a/companion/Sources/AppLifecycle.swift b/companion/Sources/AppLifecycle.swift index 065abc5..9ebffb9 100644 --- a/companion/Sources/AppLifecycle.swift +++ b/companion/Sources/AppLifecycle.swift @@ -9,6 +9,7 @@ enum AppLifecycle { } static func launch(bundleIdentifier: String, foregroundIfRunning: Bool) throws { + var reached = XCUIApplication.State.unknown try onMainCatching { let application = XCUIApplication(bundleIdentifier: bundleIdentifier) if foregroundIfRunning { @@ -18,6 +19,17 @@ enum AppLifecycle { } else { application.launch() } + reached = application.state + } + // A refused launch is recorded as a test issue that never throws, so + // without this the runner answers ok for an app that is not running + // and the host learns nothing until its own bound expires. + switch reached { + case .runningForeground, .runningBackground, .runningBackgroundSuspended: + return + default: + throw LifecycleError.failed( + "\(bundleIdentifier) is \(name(of: reached)) after launch") } } @@ -54,6 +66,21 @@ enum AppLifecycle { return result } + private static func name(of state: XCUIApplication.State) -> String { + switch state { + case .runningForeground: + return "foreground" + case .runningBackground: + return "background" + case .runningBackgroundSuspended: + return "suspended" + case .notRunning: + return "not running" + default: + return "unknown" + } + } + // onMainCatching runs automation work on the main thread and converts a // framework assertion into a thrown error so the server survives it. private static func onMainCatching(_ work: @escaping () -> Void) throws { diff --git a/docs/development/ci.md b/docs/development/ci.md index e82641c..b2dcf3e 100644 --- a/docs/development/ci.md +++ b/docs/development/ci.md @@ -96,27 +96,90 @@ The wasmJs app is served with `Cross-Origin-Opener-Policy` and cross-origin isolation. Served without them the app loads a blank canvas and every step observes an empty accessibility tree. -The seeds are calibrated, not guessed. On an M-series mac, web seed 3 convicts at -step 185-187 and ios seed 7 at step 97-101, each 3 runs out of 3 and each with a -delta of exactly twice the typed amount. Both run a 240-step budget. Keep them -pinned: honest evidence is rare, and across 2261 ios steps only one submit tap -landing on Home had a single-submit window. +The seeds are calibrated, not guessed, and every number here says which host it +was measured on, because the hosts do not agree. On an M3 mac driving iOS 26.1 +simulators, ios seed 7 convicts at step 97-101, 11 runs out of 11 from a cleared +install, each on both properties and each with the balance moving by exactly +twice the typed amount: 199 typed, 39800 cents moved, one account's transaction +count rising by two against a window holding one submit. Web seed 3 convicts at +step 185-187 on that mac and at step 192 on the ubuntu runner. Both legs run a +240-step budget. -Android runs seed 9 over 200 steps: its conviction lands at step 178, so a +What those numbers assume is a cleared starting state, and that is the only thing +that moved them. Measured four ways on one simulator, seed 7 convicts at step 97 +from a fresh install with clear-state on, at 100 from a fresh install with it +off, and at 97 from a dirty container with it on. It walks 240 steps clean +exactly once: dirty container, clear-state off, where the app opens already +signed in on the previous run's accounts and the walk diverges at step 1. The leg +therefore clears state for itself rather than relying on how it was called. + +Do not read a mac number as a statement about CI. **The ios leg does not +currently convict on the runner at all**, and no seed fixes that. Seed 7 and seed +28 were both dispatched against macos-15 and both ran 240 steps clean, from the +state the mac convicts from. + +Seed 7 diverges: the two walks agree action for action through step 48, where a +double-tapped submit lands, and there the mac's next snapshot showed Home while +the runner's still showed the transaction screen. Past that they are unrelated +walks. + +Seed 28 is the informative one, because it did not diverge. It double-tapped +Submit on the runner at step 32, which is exactly where it convicts on the mac 8 +runs out of 8. The counting invariant still could not judge it, and the trace +says why. `submitCommitsOneTransactionPerAction` only evaluates when a Home +reading arrives, and that run went from step 19 to step 136 without once +returning Home: + + step 19 null -> {Checking: 0} submits 0 + step 136 {Checking: 0} -> {Checking: 15} submits 37 + step 164 {Checking: 15} -> {Checking: 19} submits 7 + step 222 {..., Travel: 0} -> {Checking: 25, Travel: 0} submits 13 + step 239 {Checking: 25} -> {Checking: 26, Travel: 0} submits 1 + +A rise of 15 against a window of 37 is not a violation, and neither is 4 against +7, 6 against 13, or 1 against 1. The property is sound; it needs a window holding +roughly one submit before it can convict, and whether the walk closes the window +soon after a double tap is timing dependent. + +So reaching the bug is necessary and not sufficient. Across 2411 swept steps only +14 double-tapped Submit at all, and only one of those landed in a window the +counting invariant could judge. Until the property can attribute a submit without +waiting for Home, treat an ios pass as evidence and an ios failure as unproven. +The transaction rows carry a `LedgerRow` test tag on the ledger screen, which the +walk visits far more often than Home, so a count that does not depend on Home is +available; it needs per-account attribution, since the ledger shows one account +where Home shows all of them. + +Android runs seed 9 over 200 steps. Its conviction lands around step 178, and a shorter budget would never see the bonus. A full run costs about five minutes. -Repeating the ios leg by hand is not the same as running it in CI: with -`--clear-data=false` a second local run inherits the first one's accounts, so -`simctl uninstall` before each repeat or the numbers drift. +That step number was measured on a local emulator with animations ON, and the CI +job sets `disable-animations: true`, so it does not describe the CI leg. The +worry that follows is that zeroing the 700ms Compose fade would stop the leg +exercising the cross-fade wait entirely. The first real dispatch says otherwise: +its 200-step trace carries 4 `transitional` steps, so the wait still fires, just +far less often than it does locally. Treat the android number as an order of +magnitude, not a pin. It is a health gate, so nothing keys on it. -The ios leg passes `--clear-data=false`, because the job installs a fresh build -immediately before the run and a freshly installed app is already clear state. -The in-run reinstall is worth avoiding: `simctl uninstall` + `install` followed -straight away by the XCTest runner's own launch fails with `app.folio is unknown -to FrontBoard` maybe half the time. That used to hang the run outright; the -launch RPC is bounded now, so it fails in about 90 seconds with a real error -instead, but a failing leg is still a failing leg. The job timeouts are the -backstop if it happens anyway. +Repeating the ios leg by hand needs nothing special now, because the run clears +the app's state itself. It used to: `just ios` installs over the top without +uninstalling and folio's signed-in session survives that, so a repeat under the +old `--clear-data=false` opened on the previous run's Home screen and diverged at +step 1. That is how the leg came to look dead while the app and the seed were +both fine, and it is worth recognising: a leg that reports "the double-submit bug +was NOT found" from a machine that has been running the app all day is describing +the machine. + +The ios leg clears state and passes no `--ios-app-path`, which is deliberate: +without an app path the driver wipes the app's data container instead of +reinstalling, and the reinstall is the path that races FrontBoard. `simctl +uninstall` + `install` followed straight away by the XCTest runner's own launch +has failed with `app.folio is unknown to FrontBoard` about half the time on the +host that reported it. That race is untouched and still open; the leg simply +does not take that path. It did not reproduce here at all, in 20 consecutive +reinstall-and-launch cycles on iOS 26.1, 10 of them reinstalling on top of a +live app, so any fix for it has to be developed on a host that can still show it +failing. Only one sanderling run may drive a given simulator at a time. The driver takes an advisory lock on the target's UDID and a second run is refused with the lock @@ -177,3 +240,21 @@ do not raise the step budget blindly - run a seed sweep with the campaign tool (`cmd/internal-tools/campaign`), which exists for exactly this, and pin a seed that finds the bug with room to spare. A leg failing with "a predicate threw" is a different problem entirely and no seed will fix it. + +Sweep in the leg's own configuration, though. The campaign tool and the ios leg +now clear state the same way, so a swept seed means what the leg means, but the +starting frame is not a detail you can skip checking: while the leg still passed +`--clear-data=false`, seed 14 convicted at step 17 in 2 campaign runs out of 2 +and in 0 leg-shaped runs out of 3. Prefer the earliest conviction on offer over +the first one found, too. A run reproduces its trajectory on another host only +for as long as every snapshot agrees, and every step of prefix is another chance +for it not to: seeds convicting at steps 33, 60, 114, 187 and 189 all turned up +within the first 30, so an early one is usually there to be found. + +A short prefix is necessary and not sufficient, though, and ios is the standing +counter-example: seed 28 has the shortest prefix on offer, reproduced its walk on +the runner exactly, reached the bug at step 32, and still did not convict, +because the window the counting invariant had to judge it in was 117 steps wide. +Sweeping selects for a seed that reaches the bug. It cannot select for one whose +walk also closes the window, so when a property needs a window, check what the +window looked like and not only that the conviction happened. diff --git a/examples/folio/README.md b/examples/folio/README.md index e96158f..2bac9c5 100644 --- a/examples/folio/README.md +++ b/examples/folio/README.md @@ -37,7 +37,9 @@ IOS_DEVICE="iPhone 15" just ios # pick a different simulator `just ios` regenerates `app/iosApp/iosApp.xcodeproj` from `app/iosApp/project.yml`, builds the KMP framework (`Shared.framework` from `:app:shared`), links it -into the SwiftUI host, installs, and launches. +into the SwiftUI host, uninstalls any previous copy, installs, and launches. +The uninstall matters: folio's signed-in session survives an install over the +top, so without it a run opens on the last run's Home screen. ## Web diff --git a/examples/folio/app/shared/build.gradle.kts b/examples/folio/app/shared/build.gradle.kts index 9dd07b2..e6353c1 100644 --- a/examples/folio/app/shared/build.gradle.kts +++ b/examples/folio/app/shared/build.gradle.kts @@ -49,6 +49,9 @@ kotlin { implementation(libs.lifecycle.viewmodel.compose) implementation(libs.navigation.compose) } + commonTest.dependencies { + implementation(kotlin("test")) + } androidMain.dependencies { implementation(libs.androidx.activity.compose) } diff --git a/examples/folio/app/shared/src/commonMain/kotlin/app/folio/feature/ledger/AddTransactionViewModel.kt b/examples/folio/app/shared/src/commonMain/kotlin/app/folio/feature/ledger/AddTransactionViewModel.kt index fbb160d..20337c2 100644 --- a/examples/folio/app/shared/src/commonMain/kotlin/app/folio/feature/ledger/AddTransactionViewModel.kt +++ b/examples/folio/app/shared/src/commonMain/kotlin/app/folio/feature/ledger/AddTransactionViewModel.kt @@ -3,6 +3,7 @@ package app.folio.feature.ledger import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import app.folio.core.data.Account +import app.folio.core.data.MAX_TRANSACTION_AMOUNT_CENTS import app.folio.core.data.Repository import app.folio.core.data.TxnType import app.folio.navigation.Navigator @@ -87,6 +88,10 @@ class AddTransactionViewModel( form.update { it.copy(error = "Amount must be greater than zero") } return } + if (cents > MAX_TRANSACTION_AMOUNT_CENTS) { + form.update { it.copy(error = "Amount is too large (max \$1,000,000.00)") } + return + } viewModelScope.launch { try { repository.createTransaction(accountId, s.type, cents, s.note) diff --git a/examples/folio/app/shared/src/commonMain/kotlin/app/folio/util/Format.kt b/examples/folio/app/shared/src/commonMain/kotlin/app/folio/util/Format.kt index d40c9d4..bb0cb29 100644 --- a/examples/folio/app/shared/src/commonMain/kotlin/app/folio/util/Format.kt +++ b/examples/folio/app/shared/src/commonMain/kotlin/app/folio/util/Format.kt @@ -54,9 +54,8 @@ fun parseCents(input: String): Long? { val fracPadded = (frac + "00").substring(0, 2) val wholeLong = whole.toLongOrNull() ?: return null val fracLong = fracPadded.toLongOrNull() ?: return null - val total = wholeLong * 100 + fracLong - if (total < 0) return null - return total + if (wholeLong > (Long.MAX_VALUE - fracLong) / 100) return null + return wholeLong * 100 + fracLong } fun signedAmount(t: Transaction): Long = if (t.type == TxnType.credit) t.amount else -t.amount diff --git a/examples/folio/app/shared/src/commonTest/kotlin/app/folio/util/ParseCentsTest.kt b/examples/folio/app/shared/src/commonTest/kotlin/app/folio/util/ParseCentsTest.kt new file mode 100644 index 0000000..1580a34 --- /dev/null +++ b/examples/folio/app/shared/src/commonTest/kotlin/app/folio/util/ParseCentsTest.kt @@ -0,0 +1,38 @@ +package app.folio.util + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class ParseCentsTest { + @Test + fun parsesEverydayAmountsExactly() { + assertEquals(1L, parseCents("0.01")) + assertEquals(1234L, parseCents("12.34")) + assertEquals(1250L, parseCents("12.5")) + assertEquals(1200L, parseCents("12.0")) + assertEquals(100000L, parseCents("1000")) + assertEquals(123456L, parseCents("1,234.56")) + } + + @Test + fun rejectsEighteenDigitWholeThatWrapsToAPositiveLong() { + assertNull(parseCents("999999999999999999")) + } + + @Test + fun rejectsSeventeenDigitWholeThatWrapsToANegativeLong() { + assertNull(parseCents("99999999999999999")) + } + + @Test + fun rejectsNineteenDigitWholeThatNoLongerFitsALong() { + assertNull(parseCents("9999999999999999999")) + } + + @Test + fun acceptsTheLargestRepresentableAmountAndRejectsOneCentMore() { + assertEquals(Long.MAX_VALUE, parseCents("92233720368547758.07")) + assertNull(parseCents("92233720368547758.08")) + } +} diff --git a/examples/folio/core/build.gradle.kts b/examples/folio/core/build.gradle.kts index 375fba2..db2e18b 100644 --- a/examples/folio/core/build.gradle.kts +++ b/examples/folio/core/build.gradle.kts @@ -35,6 +35,10 @@ kotlin { api(libs.sqldelight.coroutines.extensions) api(libs.sqldelight.async.extensions) } + commonTest.dependencies { + implementation(kotlin("test")) + implementation(libs.kotlinx.coroutines.test) + } androidMain.dependencies { implementation(libs.sqldelight.android.driver) } diff --git a/examples/folio/core/src/commonMain/kotlin/app/folio/core/data/Repository.kt b/examples/folio/core/src/commonMain/kotlin/app/folio/core/data/Repository.kt index 020407e..ea8d8e0 100644 --- a/examples/folio/core/src/commonMain/kotlin/app/folio/core/data/Repository.kt +++ b/examples/folio/core/src/commonMain/kotlin/app/folio/core/data/Repository.kt @@ -6,6 +6,8 @@ import dev.zacsweers.metro.Inject import dev.zacsweers.metro.SingleIn import kotlinx.coroutines.flow.StateFlow +const val MAX_TRANSACTION_AMOUNT_CENTS = 100_000_000L + @SingleIn(AppScope::class) @Inject class Repository(private val store: LedgerStore) { @@ -27,6 +29,7 @@ class Repository(private val store: LedgerStore) { suspend fun createTransaction(accountId: String, type: TxnType, amount: Long, note: String): Transaction { require(amount > 0) { "Amount must be greater than zero" } + require(amount <= MAX_TRANSACTION_AMOUNT_CENTS) { "Amount is too large (max \$1,000,000.00)" } requireNotNull(getAccount(accountId)) { "Account not found" } val txn = Transaction( id = Platform.makeId(), diff --git a/examples/folio/core/src/commonTest/kotlin/app/folio/core/data/RepositoryTest.kt b/examples/folio/core/src/commonTest/kotlin/app/folio/core/data/RepositoryTest.kt new file mode 100644 index 0000000..65f9d81 --- /dev/null +++ b/examples/folio/core/src/commonTest/kotlin/app/folio/core/data/RepositoryTest.kt @@ -0,0 +1,66 @@ +package app.folio.core.data + +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +private class FakeLedgerStore : LedgerStore { + override val accounts = MutableStateFlow>(emptyList()) + override val transactions = MutableStateFlow>(emptyList()) + override val session = MutableStateFlow(null) + + override suspend fun accountExistsByName(name: String): Boolean = + accounts.value.any { it.name == name } + + override suspend fun insertAccount(id: String, name: String, createdAt: Long) { + accounts.value = accounts.value + Account(id, name, createdAt) + } + + override suspend fun insertTxn( + id: String, + accountId: String, + type: TxnType, + amount: Long, + note: String, + createdAt: Long, + ) { + transactions.value = transactions.value + Transaction(id, accountId, type, amount, note, createdAt) + } + + override suspend fun upsertSession(user: String, loggedInAt: Long) { + session.value = Session(user, loggedInAt) + } + + override suspend fun clearSession() { + session.value = null + } +} + +class RepositoryTest { + @Test + fun rejectsAmountAboveOneMillionDollars() = runTest { + val repository = Repository(FakeLedgerStore()) + val account = repository.createAccount("Checking") + + assertFailsWith { + repository.createTransaction(account.id, TxnType.credit, 100_000_001L, "") + } + assertFailsWith { + repository.createTransaction(account.id, TxnType.credit, Long.MAX_VALUE, "") + } + assertTrue(repository.transactions.value.isEmpty()) + } + + @Test + fun acceptsAmountAtOneMillionDollars() = runTest { + val repository = Repository(FakeLedgerStore()) + val account = repository.createAccount("Checking") + + repository.createTransaction(account.id, TxnType.credit, 100_000_000L, "rent") + + assertEquals(listOf(100_000_000L), repository.transactions.value.map { it.amount }) + } +} diff --git a/examples/folio/gradle/libs.versions.toml b/examples/folio/gradle/libs.versions.toml index 5e4f40b..4561bdb 100644 --- a/examples/folio/gradle/libs.versions.toml +++ b/examples/folio/gradle/libs.versions.toml @@ -14,6 +14,7 @@ sqlite-wasm = "3.53.0-build1" [libraries] kotlinx-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" } +kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinx-coroutines" } kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serialization-json", version.ref = "kotlinx-serialization" } sqldelight-runtime = { module = "app.cash.sqldelight:runtime", version.ref = "sqldelight" } diff --git a/examples/folio/justfile b/examples/folio/justfile index 4328766..4a00612 100644 --- a/examples/folio/justfile +++ b/examples/folio/justfile @@ -78,6 +78,13 @@ _ensure-device: echo "emulator did not finish booting in time (see /tmp/folio-emulator.log)" >&2 exit 1 +# Run folio's own unit tests. Named test-unit because `test` is the fuzz run. +test-unit: + #!/usr/bin/env bash + set -euo pipefail + export ANDROID_HOME="$(just _android-home)" + ./gradlew :core:testDebugUnitTest :app:shared:testDebugUnitTest + # Build the folio debug APK without installing it. build: #!/usr/bin/env bash @@ -130,6 +137,10 @@ ios: -destination 'platform=iOS Simulator,name={{ios_device}}' \ -derivedDataPath app/iosApp/build \ build | tail -5 + # Installing over the top keeps the data container, and folio's signed-in + # session with it, so a run started straight after would open on the last + # run's Home screen instead of Login and diverge at step 1. + xcrun simctl uninstall booted app.folio || true xcrun simctl install booted "{{ios_app}}" xcrun simctl launch booted app.folio diff --git a/examples/folio/sanderling/predicates.ts b/examples/folio/sanderling/predicates.ts index ab93c60..27bee6d 100644 --- a/examples/folio/sanderling/predicates.ts +++ b/examples/folio/sanderling/predicates.ts @@ -123,6 +123,51 @@ export function readHomeCards(args: { return { value: reading, carrier: reading, fresh: true }; } +// The balance of the ONE account the ledger and the add-transaction screen are +// showing, and the window it closes. +// +// It exists because the Home readings above close their window only when the +// walk goes back to Home, and a walk inside the transaction flow does not: a +// submit pops back to the ledger it came from, so the fuzzer can add +// transactions all day without Home ever being redrawn. The iOS run in #78 went +// 117 steps between two Home readings and accumulated 37 submits against a rise +// of 15 transactions, which is no evidence about any single action. Both +// screens here carry the account's own balance (LedgerBalance, +// TxnCurrentBalance), so the window between two readings holds one action. +// +// WHICH account is never asked, because inside a run of these two routes it +// cannot change. Route.Ledger is pushed only by tapping a card on Home, +// Route.AddTransaction only by the ledger's own button for its own account, and +// an accepted submit pops back to that same ledger. Reaching another account's +// ledger means passing through Home, and one action reads one frame, so a frame +// that is neither of these two routes always sits in between. Dropping the +// carrier on every such frame, transition frames included, is what makes the +// two compared numbers two readings of one account. +export interface AccountBalanceReading { + value: number | null; + carrier: number | null; + fresh: boolean; +} + +function showsOneAccount(route: string | null): boolean { + return route === "ledger" || route === "add-transaction"; +} + +export function readAccountBalance(args: { + route: string | null; + balanceText: string | undefined; + previousCarrier: number | null; +}): AccountBalanceReading { + const { route, balanceText, previousCarrier } = args; + if (!showsOneAccount(route)) return { value: null, carrier: null, fresh: false }; + const balance = parseAccountBalance(balanceText); + // Unreadable is unknown, not a new value: the balance node scrolls off the + // viewport like anything else. The account still cannot have changed, so the + // last number we read is carried across and the window stays open. + if (balance === null) return { value: previousCarrier, carrier: previousCarrier, fresh: false }; + return { value: balance, carrier: balance, fresh: true }; +} + // state.lastAction as the two hosts build it (internal/verifier/marshal.go // lastActionFields), read defensively: every field is what a Go struct decided // to emit, not something this file can trust a compile-time shape for. @@ -136,6 +181,7 @@ export interface ObservedAction { kind?: string; on?: string | object; applied?: true | null; + relaunched?: true | null; } function isTapOn(lastAction: ObservedAction | null, target: string): boolean { @@ -171,6 +217,19 @@ export function confirmedApplied(lastAction: ObservedAction | null): boolean { return lastAction != null && lastAction.applied === true; } +// The runner reports this when its foreground guard had to relaunch the app +// after the action. The action still happened, so it still counts toward how +// many submits a window could hold; what nobody can promise across it is that +// the process survived long enough to commit, or that Home is showing the same +// slice of the account list it was. +// +// `true | null` for the same reason `applied` is: web and iOS cannot read the +// foreground at all, so "no relaunch reported" is not "the app never +// restarted", and only an explicit true licenses declining. +export function acrossRelaunch(lastAction: ObservedAction | null): boolean { + return lastAction != null && lastAction.relaunched === true; +} + // Counts the submit actions inside the window the balance property compares // over: from the last Home total we read to this step, inclusive of this step's // action. @@ -190,16 +249,54 @@ export function confirmedApplied(lastAction: ObservedAction | null): boolean { // well have landed. Leaving it out is what convicted a healthy app: // committedTransactionsExceedSubmits saw a transaction rise of one against a // window of zero and called it a double submit. +// +// A submit the app must have refused does not count, for the mirror reason: it +// cannot have committed anything, so the bound it would raise is slack the app +// can hide a real double submit behind. See submitCouldCommit for what "must +// have refused" is allowed to mean. export function countSubmitsInWindow(args: { previousCount: number; lastAction: ObservedAction | null; + amountText?: string; fresh: boolean; }): { reported: number; next: number } { const { previousCount, lastAction, fresh } = args; - const reported = previousCount + (isTxnSubmitTap(lastAction) ? 1 : 0); + // A relaunch is the one thing that can put a form state on screen other than + // the one the tap read, so the field it draws proves nothing about it. + const refused = !acrossRelaunch(lastAction) && !submitCouldCommit(args.amountText); + const reported = previousCount + (isTxnSubmitTap(lastAction) && !refused ? 1 : 0); return { reported, next: fresh ? 0 : reported }; } +// Could the app have committed anything for that submit? The amount field as +// the LANDING frame shows it is the form state the tap read: the tap changes +// nothing about it, and one action runs per step, so nothing else could have. +// Off the transaction screen there is no field to read, and undefined is +// unknown, which counts. +// +// False only where Folio's own code must have refused. parseCents takes +// `^\d+(\.\d{1,2})?$` with commas stripped and refuses everything else, and +// AddTransactionViewModel refuses a parsed zero on top of that. An empty field +// never even reaches the parser: TxnSubmit is +// clickable(enabled = amount.isNotBlank()), so the click does not fire. +// +// This is the difference between a bound and a useless one. The window is an +// upper bound on the transactions the interval could hold, and a bound inflated +// by taps that commit nothing is a bound the app can never exceed: the iOS run +// in #78 read a rise of 15 transactions against a window of 37 submits and had +// nothing to say. Measured over four recorded android runs, 19, 11, 25 and 25 +// of 35, 26, 42 and 42 submit taps landed with the amount field empty. +// +// An amount too large for a Kotlin Long is refused by the app too, and still +// counts here: over-counting can only cost a detection, and the reading that +// would have to prove the overflow is a float that cannot hold the number. +export function submitCouldCommit(amountText: string | undefined): boolean { + if (amountText === undefined) return true; + const trimmed = amountText.trim().replace(/,/g, ""); + if (!/^\d+(\.\d{1,2})?$/.test(trimmed)) return false; + return /[1-9]/.test(trimmed); +} + // Parses formatCents output like "$5.00", "-$1,234.56", "+$0.50" back to // integer cents. Anything that is not a complete amount is null, not 0: a // balance we could not read is unknown, and reading it as zero silently moves @@ -240,6 +337,15 @@ export function cardBalanceText(args: { return match ? match[0].trim() : undefined; } +// One account's own balance, off the node whose whole text it is: bare on the +// ledger ("$196.00"), labelled in the add-transaction header +// ("Balance: $196.00"). Anchored at the end for the same reason as above, so +// the label cannot be read as part of the amount. +export function parseAccountBalance(text: string | undefined): number | null { + const match = text?.match(TRAILING_BALANCE); + return match ? parseDollarCents(match[0].trim()) : null; +} + // The result is an identity key, not a display name: off web it is the // AccountName text, on web it is whatever the merged card text leaves in front // of the count label, initials and all ("T2Travel" for "Travel 2024"). Its only @@ -259,6 +365,24 @@ export function cardAccountName(args: { return head.slice(0, label.index).trim(); } +// The avatar text that opens a merged card's identity key, mirroring Folio's +// initialsOf (app/shared/.../util/Format.kt). +// +// A mirror because the alternative is a suffix test, and a suffix test cannot +// say which card a name belongs to. Drift can only cost a detection: the result +// is compared whole against a card's key, so initials that stop matching the +// app match no card rather than the wrong one. +export function initialsOf(name: string): string { + // Java's \s, which is what Kotlin's Regex("\\s+") compiles to. JS's \s also + // matches the unicode spaces, and would split names the app keeps whole. + const parts = name.trim().split(/[ \t\n\v\f\r]+/).filter(part => part !== ""); + const first = parts[0]; + const last = parts[parts.length - 1]; + if (first === undefined || last === undefined) return "?"; + if (parts.length === 1) return first.slice(0, 2).toUpperCase(); + return (first.slice(0, 1) + last.slice(0, 1)).toUpperCase(); +} + // One card's transaction count, in the strongest form its SOURCE supports. The // two forms are the whole reason this is not just a number: // @@ -331,13 +455,26 @@ export function homeAccountsOf(cards: readonly CardReading[]): Account[] | null // // A name carried by more than one card is left out for the same reason, the // rule createdAccountHasNonZeroBalance applies with `matches.length === 1`: -// nothing here can say which of them a count came from. Folio accepts the same -// account name twice and Home lists whatever fits the viewport, so a reading -// that saw one Travel card and a later one that saw two would otherwise -// subtract two DIFFERENT accounts' counts and convict a healthy app of -// double-submitting. The twin does not have to be readable to spoil the -// identity, so duplicates are counted over every card, not just the usable -// ones. Dropping a card can only ever cost a detection. +// nothing here can say which of them a count came from. Two accounts never +// share a NAME (Accounts.name is UNIQUE and Repository.createAccount rejects +// one already taken, NOCASE), but they can share a KEY, because web's key is +// the card text in front of the digit run, which is the name with any trailing +// digits shaved off it: "Travel1" holding 25 transactions and "Travel12" +// holding 6 both key to "TRTravel" and both read a three-digit run, so +// subtracting one from the other subtracts two unrelated counting series. The +// twin does not have to be readable to spoil the identity, so duplicates are +// counted over every card, not just the usable ones. Dropping a card can only +// ever cost a detection. +// +// What this cannot see is a twin that never shares a reading with its pair, and +// Home lists only what fits the viewport. Nothing computed from the card text +// can: the two cards' text is identical character for character ("TRTravel1" +// followed by "25 transactions" and "TRTravel12" followed by "6 transactions" +// are one string), so no key derived from it separates them. Refusing every run +// that could hide a name's own digits would, at the price of the evidence web +// convicts on today, whose measured witness is a count of 12 rising to 14. +// Separating them needs something the tree does not carry: the account's id on +// the card, or a separator in front of the count. export function homeTxnCountsOf(cards: readonly CardReading[]): Record | null { const cardsPerName = new Map(); for (const card of cards) cardsPerName.set(card.name, (cardsPerName.get(card.name) ?? 0) + 1); @@ -381,15 +518,30 @@ export function createdAccountHasNonZeroBalance(args: { // card to: the card that turned up may be an older account of the same name // scrolling into view. if (!confirmedApplied(lastAction)) return false; + // A relaunch draws Home from the top again, so the card that carries the + // typed name may be an older account of that name laid out where the new one + // used to be, and the create may not have reached sqlite at all. + if (acrossRelaunch(lastAction)) return false; if (before === null || after === null) return false; const typed = (args.typedName ?? "").trim(); if (typed === "") return false; // Web merges the card into one node whose text opens with the avatar // initials, so the identity key is "INInvestments" where android and iOS give - // "Investments"; endsWith covers both. Two cards answering to the same typed - // name (a second "Travel", or a card the tree exposed twice) leave the - // appearance unattributable, so nothing is judged. - const matches = after.filter(account => account.name.endsWith(typed)); + // "Investments". Both forms are built from the name that was typed and + // compared whole. A suffix test covered both too, and it also let any OTHER + // account ending in those letters answer for the created one: type "Fund" + // next to an existing "Emergency Fund", have the new card clipped out of the + // reading the way Home clips any card, and the old account is convicted for + // money it has held all along. It cost detections as well, because a typed + // name that two cards end with is judged as unattributable rather than as the + // one card that carries it. + // + // Two cards answering to one key stay unattributable: Accounts.name is UNIQUE + // and Repository.createAccount rejects a name already taken, so that pair is + // a card the tree exposed twice, or two names the merged key cannot tell + // apart. + const mergedKey = initialsOf(typed) + typed; + const matches = after.filter(account => account.name === typed || account.name === mergedKey); const created = matches.length === 1 ? matches[0] : undefined; if (created === undefined) return false; if (before.some(account => account.name === created.name)) return false; @@ -431,6 +583,54 @@ export function committedTransactionsExceedSubmits(args: { return committed > submitsInWindow; } +// The same rule as above, measured in money over the account's own window: one +// submit action can commit one transaction, so the account's balance cannot +// move by more than the amount that submit typed. +// +// An UPPER BOUND, not the equality submitChangesBalanceByTypedAmount uses, and +// that is what makes a one-action window safe. A balance that has not moved is +// a commit still in flight (createTransaction runs in a coroutine), a submit +// the app rejected, or a tap that never landed, and none of those is evidence +// of anything; an equality would convict all three. Moving by MORE than one +// submit's worth is not something a correct app can do: only createTransaction +// moves this number, only a TxnSubmit tap reaches it, and the window holds +// exactly one such tap. A double tap is one action committing two transactions, +// so it moves the balance by twice what was typed and lands here. +// +// A submit the runner could not confirm needs no case of its own for the same +// reason: if it never landed the balance did not move, which is under the +// bound. countSubmitsInWindow counts it either way, so it cannot smuggle a +// second commit into a window that looks like one. +// +// typedAmount is the amount the app parsed for THIS submit (parseTypedAmount +// mirrors parseCents), so every rejected amount and every amount too large to +// hold exactly arrives here as 0 and is vacuous. +// +// The float guards are the ones submitChangesBalanceByTypedAmount explains: +// each balance and the typed amount lose precision on their own past +// Number.MAX_SAFE_INTEGER. Their difference needs none, because a difference +// that is really within a safe typedAmount is itself safe and comes out exact. +export function committedAmountExceedsOneSubmit(args: { + route: string | null; + lastAction: ObservedAction | null; + submitsInWindow: number; + typedAmount: number; + prevAccountBalance: number | null; + currAccountBalance: number | null; +}): boolean { + const { route, lastAction, submitsInWindow, typedAmount } = args; + const { prevAccountBalance, currAccountBalance } = args; + if (!showsOneAccount(route)) return false; + if (!isTxnSubmitTap(lastAction)) return false; + if (submitsInWindow !== 1) return false; + if (typedAmount <= 0) return false; + if (prevAccountBalance === null || currAccountBalance === null) return false; + if (!Number.isSafeInteger(prevAccountBalance)) return false; + if (!Number.isSafeInteger(currAccountBalance)) return false; + if (!Number.isSafeInteger(typedAmount)) return false; + return Math.abs(currAccountBalance - prevAccountBalance) > typedAmount; +} + // How far one account's count rose between two readings, or null when the pair // is not comparable. Not comparable is not zero: the account drops out of the // sum entirely, which can only cost a detection. @@ -510,6 +710,10 @@ export function submitChangesBalanceByTypedAmount(args: { // runner could not confirm may have committed nothing, and a balance that // did not move is then exactly what a healthy app looks like. if (!confirmedApplied(lastAction)) return true; + // The runner restarted the app after this tap, so the process may have died + // between the commit and the sqlite write. A balance that did not move is + // then a healthy app, exactly as it is for a submit that may not have landed. + if (acrossRelaunch(lastAction)) return true; if (submitsInWindow !== 1) return true; if (typedAmount === 0) return true; // An unknown total on either side is not evidence of anything. Comparing one diff --git a/examples/folio/sanderling/spec.ts b/examples/folio/sanderling/spec.ts index a403d51..260e291 100644 --- a/examples/folio/sanderling/spec.ts +++ b/examples/folio/sanderling/spec.ts @@ -17,6 +17,7 @@ import { cardAccountName, cardBalanceText, cardTxnCount, + committedAmountExceedsOneSubmit, committedTransactionsExceedSubmits, countSubmitsInWindow, createdAccountHasNonZeroBalance, @@ -25,6 +26,7 @@ import { oncePerFrame, parseDollarCents, parseTypedAmount, + readAccountBalance, readHomeCards, readHomeTotalBalance, routeOfFrame, @@ -102,6 +104,11 @@ const homeCards = oncePerFrame((s: State): CardReading[] => // the last-read Home total so `previous` and `current` stay on the same scale. const homeTotalText = (s: State) => on("home", "TotalBalance")(s)?.text; +// The amount field as the frame a submit landed on shows it, which is the form +// state that submit read. Every window below asks, because a submit the app +// must have refused raises no bound: see submitCouldCommit. +const txnAmountText = oncePerFrame((s: State) => on("add-transaction", "TxnAmountField")(s)?.text); + let lastHomeTotal: number | null = null; const totalBalance = extract("totalBalance", s => { const reading = readHomeTotalBalance({ @@ -127,6 +134,7 @@ const submitsInWindow = extract("submitsInWindow", s => { const window = countSubmitsInWindow({ previousCount: submitsSinceHomeTotal, lastAction: s.lastAction, + amountText: txnAmountText(s), fresh, }); submitsSinceHomeTotal = window.next; @@ -175,12 +183,52 @@ const submitsSinceCounts = extract("submitsSinceCounts", s => { const window = countSubmitsInWindow({ previousCount: submitsSinceHomeCards, lastAction: s.lastAction, + amountText: txnAmountText(s), fresh, }); submitsSinceHomeCards = window.next; return window.reported; }); +// The account's own balance, off whichever of its two screens is up. The routes +// are exclusive, so at most one of these resolves and the reading is always one +// account's number. Its carrier is dropped on every other route, which is what +// keeps two readings from spanning two accounts: see readAccountBalance. +const accountBalanceText = (s: State) => + on("ledger", "LedgerBalance")(s)?.text ?? on("add-transaction", "TxnCurrentBalance")(s)?.text; + +let lastAccountBalance: number | null = null; +const accountBalance = extract("accountBalance", s => { + const reading = readAccountBalance({ + route: routeOf(s), + balanceText: accountBalanceText(s), + previousCarrier: lastAccountBalance, + }); + lastAccountBalance = reading.carrier; + return reading.value; +}); + +// A third window, for the same reason the counting invariant has its own: it +// closes on this reading's freshness, which is a different event again. The +// transaction flow redraws this balance on nearly every frame, so this window +// is the narrow one, usually a single action wide. +let submitsSinceAccountBalance = 0; +const submitsSinceBalance = extract("submitsSinceAccountBalance", s => { + const fresh = readAccountBalance({ + route: routeOf(s), + balanceText: accountBalanceText(s), + previousCarrier: null, + }).fresh; + const window = countSubmitsInWindow({ + previousCount: submitsSinceAccountBalance, + lastAction: s.lastAction, + amountText: txnAmountText(s), + fresh, + }); + submitsSinceAccountBalance = window.next; + return window.reported; +}); + const lastAction = extract("lastAction", s => s.lastAction); const loginEmailField = extract("loginEmailField", on("login", "LoginEmail")); @@ -232,13 +280,29 @@ const submitMovesBalanceByTypedAmount = always( // stays sound however wide the window between two Home readings gets, because // both sides of the comparison accumulate over the same window. It is the // double-submit stated directly: one tap, two rows. +// +// One rule, two windows. The counting form can only compare two Home readings, +// and a walk that stays inside the transaction flow gives it a window hundreds +// of steps and dozens of submits wide, which is sound and says nothing. The +// second form says the same thing in money about the one account whose screen +// the walk is on, and that window is usually a single action, so it can still +// tell one commit from two: see committedAmountExceedsOneSubmit. const submitCommitsOneTransactionPerAction = always( - next(() => - !committedTransactionsExceedSubmits({ - countsBefore: homeTxnCounts.previous ?? null, - countsAfter: homeTxnCounts.current, - submitsInWindow: submitsSinceCounts.current, - }), + next( + () => + !committedTransactionsExceedSubmits({ + countsBefore: homeTxnCounts.previous ?? null, + countsAfter: homeTxnCounts.current, + submitsInWindow: submitsSinceCounts.current, + }) && + !committedAmountExceedsOneSubmit({ + route: route.current, + lastAction: lastAction.current, + submitsInWindow: submitsSinceBalance.current, + typedAmount: parseTypedAmount(txnAmountField.previous?.text), + prevAccountBalance: accountBalance.previous ?? null, + currAccountBalance: accountBalance.current, + }), ), ); diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 67536a7..52903ab 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -31,17 +31,22 @@ type DeviceOptions struct { BundleID string // AppPath is the .app bundle installed via devicectl for clear-state. AppPath string + // ClearState reinstalls the app while NewDevice runs, before the runner's + // test session exists. Clear state is a property of the driver rather than + // of a launch: see Launch. + ClearState bool // Output receives the runner session log path and driver warnings. Output io.Writer // DoubleTapGapMilliseconds overrides the synthesized double-tap gap. DoubleTapGapMilliseconds float64 // Test seams. Production leaves them nil and NewDevice wires the real - // build/spawn/tunnel/dial. - spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) - startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) - dialRunner func(address string) (transport.Companion, error) - pickAddress func() (string, error) + // build/spawn/tunnel/dial/devicectl. + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) + dialRunner func(address string) (transport.Companion, error) + pickAddress func() (string, error) + reinstallApp func(ctx context.Context) error } // deviceStartupTimeout bounds the runner's startup once its hosting test @@ -61,6 +66,9 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { if options.CoreDeviceID == "" { return nil, errors.New("ios device: CoreDeviceID is required") } + if options.ClearState && options.BundleID == "" { + return nil, errors.New("ios device: clear-state needs BundleID: there is nothing to uninstall without it") + } output := options.Output if output == nil { output = io.Discard @@ -75,6 +83,7 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { coreDeviceID: options.CoreDeviceID, bundleID: options.BundleID, appPath: options.AppPath, + clearStateAtStartup: options.ClearState, output: output, doubleTapGapMilliseconds: gap, deviceMode: true, @@ -103,7 +112,10 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { // Device seams: clear-state reinstalls via devicectl; the container reset and // paste grant are simulator-only and become no-ops. The runner types // natively, so no paste prompt is ever hit. - d.reinstallApp = d.devicectlReinstall + d.reinstallApp = options.reinstallApp + if d.reinstallApp == nil { + d.reinstallApp = d.devicectlReinstall + } d.resetContainer = d.deviceResetContainerUnsupported d.grantPaste = func(context.Context) error { return nil } d.restart = d.respawnDevice @@ -116,6 +128,13 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { } d.deviceLock = lock + if options.ClearState { + if err := d.clearAppState(ctx); err != nil { + d.Close() + return nil, err + } + } + if err := d.bringUpDevice(ctx); err != nil { d.Close() return nil, err diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index 5cacfc0..1ffd11d 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -6,6 +6,7 @@ import ( "io" "net" "os/exec" + "slices" "testing" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/transport" @@ -152,6 +153,35 @@ func TestDeviceEraseAndPressKeyRouteThroughEditor(t *testing.T) { } } +func TestNewDeviceReinstallsOnceBeforeTheRunnerSession(t *testing.T) { + address := startLoopbackListener(t) + probe := &clearStateProbe{} + options := testDeviceOptions(address, newDeviceCompanion()) + options.HardwareUDID = "00008140-CLEAR" + options.AppPath = "/tmp/Sample.app" + options.ClearState = true + options.reinstallApp = func(context.Context) error { probe.record("reinstall"); return nil } + spawn := options.spawnRunner + options.spawnRunner = func(ctx context.Context, runnerAddress string) (*exec.Cmd, error) { + probe.record("runner session") + return spawn(ctx, runnerAddress) + } + + d, err := NewDevice(context.Background(), options) + if err != nil { + t.Fatalf("NewDevice: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", true, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"reinstall", "runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: devicectl must reinstall once, before the runner's test session attaches", got, want) + } +} + func TestDeviceClearStateWithoutAppPathWarnsOnce(t *testing.T) { output := &bytes.Buffer{} d := &Driver{output: output, deviceMode: true} diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 78a2ada..04b7d7c 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -53,6 +53,14 @@ var shutdownGrace = 15 * time.Second // A variable so the timeout test can shrink it. var launchTimeout = 90 * time.Second +// launchRecoveryTimeout bounds the whole recovery a blown launch bound +// triggers, the session restart and the second attempt together. It keeps the +// launch path inside the three minutes testrun allows it, so what a user sees +// when the app really cannot be launched stays the driver's error rather than +// that backstop firing over the top of it. A variable so the bound test can +// shrink it. +var launchRecoveryTimeout = 60 * time.Second + // longPressHoldMilliseconds is how long LongPress holds the finger down. const longPressHoldMilliseconds = 600 @@ -65,16 +73,24 @@ type Options struct { // AppPath is the .app bundle directory. Required for clear-state reinstall; // when empty, clear state falls back to resetting the data container. AppPath string + // ClearState resets the app to first-launch state while New runs, before + // any automation session attaches. Clear state is a property of the driver + // rather than of a launch: see Launch. + ClearState bool // Output receives companion stdout and stderr plus driver warnings. Output io.Writer // DoubleTapGapMilliseconds overrides the synthesized double-tap gap. DoubleTapGapMilliseconds float64 - // spawnChild, dialCompanion, and pickAddress are test seams. Production - // leaves them nil and New wires the real extraction, spawn, and dial. - spawnChild func(ctx context.Context, address string) (*exec.Cmd, error) - dialCompanion func(address string) (transport.Companion, error) - pickAddress func() (string, error) + // These are test seams. Production leaves them nil and New wires the real + // extraction, spawn, dial and simctl calls. + spawnChild func(ctx context.Context, address string) (*exec.Cmd, error) + dialCompanion func(address string) (transport.Companion, error) + pickAddress func() (string, error) + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + dialRunner func(address string) (transport.Companion, error) + reinstallApp func(ctx context.Context) error + resetContainer func(ctx context.Context) error } // Driver implements driver.DeviceDriver against an iOS simulator companion. @@ -85,6 +101,12 @@ type Driver struct { appPath string output io.Writer + // clearStateAtStartup records that New (or NewDevice) reset the app to + // first-launch state before attaching, which is the only point in a run + // where clearing is safe. Launch refuses a clear-state request the driver + // was not built for rather than reinstalling under a live session. + clearStateAtStartup bool + screenWidth int screenHeight int @@ -129,12 +151,13 @@ type Driver struct { // lifecycle, screenshot) with an in-simulator runner that serves // collapse-free accessibility snapshots and native unicode typing. // runnerClient is nil on the legacy-only path. - runnerClient transport.Companion - runnerChild *exec.Cmd - runnerAddress string - spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) - dialRunner func(address string) (transport.Companion, error) - hybrid bool + runnerClient transport.Companion + runnerChild *exec.Cmd + runnerAddress string + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + dialRunner func(address string) (transport.Companion, error) + pickRunnerAddress func() (string, error) + hybrid bool // Device-mode fields. On the physical-device path d.companion is the runner // dialed over a usbmux tunnel, hybrid is false, and runnerClient is nil. @@ -189,6 +212,9 @@ func New(ctx context.Context, options Options) (*Driver, error) { if options.UniqueDeviceIdentifier == "" { return nil, errors.New("ios companion: UniqueDeviceIdentifier is required") } + if options.ClearState && options.BundleID == "" { + return nil, errors.New("ios companion: clear-state needs BundleID: there is nothing to uninstall or wipe without it") + } output := options.Output if output == nil { output = io.Discard @@ -202,10 +228,15 @@ func New(ctx context.Context, options Options) (*Driver, error) { udid: options.UniqueDeviceIdentifier, bundleID: options.BundleID, appPath: options.AppPath, + clearStateAtStartup: options.ClearState, output: output, doubleTapGapMilliseconds: gap, spawnChild: options.spawnChild, dial: options.dialCompanion, + spawnRunner: options.spawnRunner, + dialRunner: options.dialRunner, + reinstallApp: options.reinstallApp, + resetContainer: options.resetContainer, hybrid: hybridCompanionEnabled(), } if driverInstance.spawnChild == nil { @@ -232,9 +263,14 @@ func New(ctx context.Context, options Options) (*Driver, error) { return nil, err } driverInstance.address = address + driverInstance.pickRunnerAddress = pickAddress driverInstance.restart = driverInstance.respawnAndRedial - driverInstance.resetContainer = driverInstance.resetDataContainer - driverInstance.reinstallApp = driverInstance.simctlReinstall + if driverInstance.resetContainer == nil { + driverInstance.resetContainer = driverInstance.resetDataContainer + } + if driverInstance.reinstallApp == nil { + driverInstance.reinstallApp = driverInstance.simctlReinstall + } driverInstance.grantPaste = driverInstance.grantPasteboardAccess driverInstance.processContext, driverInstance.processCancel = context.WithCancel(ctx) @@ -245,6 +281,13 @@ func New(ctx context.Context, options Options) (*Driver, error) { } driverInstance.deviceLock = lock + if options.ClearState { + if err := driverInstance.clearAppState(ctx); err != nil { + driverInstance.Close() + return nil, err + } + } + if err := driverInstance.bringUp(ctx); err != nil { driverInstance.Close() return nil, err @@ -358,7 +401,7 @@ func (d *Driver) bringUpRunner(ctx context.Context) error { // A fresh port every bring-up: after a restart the dying session's // listener may still answer on the old port and would satisfy the wait // below with a dead server. - address, err := pickLoopbackAddress() + address, err := d.pickRunnerAddress() if err != nil { return err } @@ -452,6 +495,13 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e // loudly rather than silently dropping the request. return errors.New("ios companion: launch with environment variables is unsupported on this backend") } + if clearState && !d.clearStateAtStartup { + // Clearing here would uninstall and reinstall the app underneath a live + // automation session, which is what races FrontBoard's registration and + // leaves the session launching a bundle FrontBoard has not registered. + return errors.New("ios companion: clear-state must be requested when the driver is created (Options.ClearState); " + + "this backend clears the app before its automation session exists") + } // Terminate first so the launch is a clean cold start regardless of the // app's prior state. A not-running app is not an error here. @@ -459,12 +509,6 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e return companion.Terminate(callCtx, d.bundleID) }) - if clearState { - if err := d.clearAppState(ctx); err != nil { - return err - } - } - // Grant the app pasteboard access before it runs so unicode input (which // must go through the pasteboard, since HID cannot express it) never trips // the iOS paste-permission prompt. clearState reinstall resets the grant, @@ -477,14 +521,55 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e } } - if err := d.lifecycleCall(ctx, func(callCtx context.Context, companion transport.Companion) error { - return companion.Launch(callCtx, d.bundleID, true) - }); err != nil { + if err := d.launchWithSessionRecovery(ctx); err != nil { return fmt.Errorf("launch %s: %w", d.bundleID, err) } return nil } +// launchWithSessionRecovery runs the launch RPC and, when it blows its own +// bound, replaces the session and launches again. +// +// A launch the simulator refuses, which is what a clear-state reinstall racing +// FrontBoard's registration produces, never comes back as an error: XCTest +// records the refusal as a test failure the runner cannot observe, then holds +// the session's main thread for about four minutes walking a diagnostic chain +// (a 120s accessibility wait, a spindump, an idle wait). So there is no error +// text to key a retry on, only the expired bound, and every later call queues +// behind the same wedge. Only a session that never served the refused launch +// can serve the retry, which is why this restarts rather than calls again. +func (d *Driver) launchWithSessionRecovery(ctx context.Context) error { + launch := func(callCtx context.Context, companion transport.Companion) error { + return companion.Launch(callCtx, d.bundleID, true) + } + err := d.lifecycleCall(ctx, launch) + // A caller whose own budget ran out gets no restart: the bound that expired + // was the caller's to spend, and the second attempt would inherit it dead. + if err == nil || !errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil || d.restart == nil { + return err + } + fmt.Fprintf(d.output, "launch %s blew its %v bound (%v); restarting the session and launching once more\n", + d.bundleID, launchTimeout, err) + + // The restart runs under the driver's own lifetime context for the same + // reason withRecovery's does, while the second attempt stays on the + // caller's. Both end at one deadline, so a launch that already spent + // launchTimeout cannot then wait out a session cold start on top of it. + recoveryDeadline := time.Now().Add(launchRecoveryTimeout) + restartCtx := d.processContext + if restartCtx == nil { + restartCtx = ctx + } + restartCtx, cancelRestart := context.WithDeadline(restartCtx, recoveryDeadline) + defer cancelRestart() + if restartErr := d.restart(restartCtx); restartErr != nil { + return fmt.Errorf("session restart failed: %w (original: %v)", restartErr, err) + } + relaunchCtx, cancelRelaunch := context.WithDeadline(ctx, recoveryDeadline) + defer cancelRelaunch() + return d.lifecycleCall(relaunchCtx, launch) +} + // lifecycleCall runs an app lifecycle RPC against lifecycleCompanion under a // launchTimeout-bounded context, with the usual one-restart recovery. The // companion is resolved inside the retry so a restart's replacement client @@ -511,7 +596,8 @@ func (d *Driver) lifecycleCompanion() transport.Companion { // clearAppState resets the app to a first-launch state. With an app path it // uninstalls and reinstalls; without one it falls back to wiping the app's data -// container and warns once that a full reinstall needs the app path. +// container and warns once that a full reinstall needs the app path. Called +// only from construction, before any automation session is attached to the app. func (d *Driver) clearAppState(ctx context.Context) error { if d.appPath != "" { if err := d.reinstallApp(ctx); err != nil { diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index a70ae6a..db3b2f4 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "sync" "testing" @@ -182,47 +183,149 @@ func TestLaunchContinuesWhenGrantFails(t *testing.T) { } } -func TestLaunchClearStateReinstallsWithAppPath(t *testing.T) { +// clearStateProbe records, in order, the calls a run makes to reset the app and +// to bring the runner's automation session up. A reinstall recorded after the +// session is the ordering that races FrontBoard. +type clearStateProbe struct { + mutex sync.Mutex + events []string +} + +func (p *clearStateProbe) record(event string) { + p.mutex.Lock() + defer p.mutex.Unlock() + p.events = append(p.events, event) +} + +func (p *clearStateProbe) recorded() []string { + p.mutex.Lock() + defer p.mutex.Unlock() + out := make([]string, len(p.events)) + copy(out, p.events) + return out +} + +// clearStateOptions wires every seam a hybrid bring-up needs, so New runs its +// real sequence against fakes: no simulator, no simctl, no XCTest session. +func clearStateOptions(t *testing.T, probe *clearStateProbe, udid string, clearState bool) Options { + t.Helper() + t.Setenv("SANDERLING_SIMULATOR_COMPANION", "") + address := startLoopbackListener(t) + return Options{ + UniqueDeviceIdentifier: udid, + BundleID: "com.example.app", + ClearState: clearState, + Output: &bytes.Buffer{}, + pickAddress: func() (string, error) { return address, nil }, + spawnChild: func(context.Context, string) (*exec.Cmd, error) { return &exec.Cmd{}, nil }, + dialCompanion: func(string) (transport.Companion, error) { + return &fakeCompanion{accessibilityJSON: "[]"}, nil + }, + spawnRunner: func(context.Context, string) (*exec.Cmd, error) { + probe.record("runner session") + return &exec.Cmd{}, nil + }, + dialRunner: func(string) (transport.Companion, error) { + return &fakeCompanion{accessibilityJSON: "[]"}, nil + }, + reinstallApp: func(context.Context) error { probe.record("reinstall"); return nil }, + resetContainer: func(context.Context) error { probe.record("reset container"); return nil }, + } +} + +func TestClearStateReinstallsOnceBeforeTheRunnerSession(t *testing.T) { + probe := &clearStateProbe{} + options := clearStateOptions(t, probe, "CLEAR-REINSTALL-UDID", true) + options.AppPath = "/tmp/Sample.app" + + d, err := New(context.Background(), options) + if err != nil { + t.Fatalf("New: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", true, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"reinstall", "runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: the reinstall must run once, before the automation session attaches", got, want) + } +} + +func TestClearStateWithoutAppPathWipesContainerBeforeTheRunnerSession(t *testing.T) { + probe := &clearStateProbe{} + output := &bytes.Buffer{} + options := clearStateOptions(t, probe, "CLEAR-CONTAINER-UDID", true) + options.Output = output + + d, err := New(context.Background(), options) + if err != nil { + t.Fatalf("New: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", true, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"reset container", "runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: the fallback must wipe the container once, before the session, and never reinstall", got, want) + } + if warnings := strings.Count(output.String(), "resetting the data container only"); warnings != 1 { + t.Fatalf("warning emitted %d times, want once", warnings) + } +} + +func TestWithoutClearStateTheAppIsLeftAlone(t *testing.T) { + probe := &clearStateProbe{} + options := clearStateOptions(t, probe, "NO-CLEAR-UDID", false) + options.AppPath = "/tmp/Sample.app" + + d, err := New(context.Background(), options) + if err != nil { + t.Fatalf("New: %v", err) + } + defer d.Close() + if err := d.Launch(context.Background(), "", false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + + want := []string{"runner session"} + if got := probe.recorded(); !slices.Equal(got, want) { + t.Fatalf("calls = %v, want %v: a run that did not ask for clear state must not touch the install", got, want) + } +} + +func TestLaunchRefusesClearStateTheDriverWasNotBuiltFor(t *testing.T) { companion := &fakeCompanion{accessibilityJSON: "[]"} d := newTestDriver(companion) d.appPath = "/tmp/Sample.app" reinstalls := 0 d.reinstallApp = func(context.Context) error { reinstalls++; return nil } - if err := d.Launch(context.Background(), "", true, nil); err != nil { - t.Fatalf("Launch: %v", err) + + err := d.Launch(context.Background(), "", true, nil) + if err == nil || !strings.Contains(err.Error(), "clear-state") { + t.Fatalf("Launch err = %v, want a refusal naming clear-state", err) } - if reinstalls != 1 { - t.Fatalf("clear-state with app path must reinstall exactly once; got %d", reinstalls) + if reinstalls != 0 { + t.Fatalf("reinstalls = %d, want 0: a live session must never have the app reinstalled under it", reinstalls) } - if indexOf(companion.calls, "launch") < indexOf(companion.calls, "terminate") { - t.Fatalf("launch must still follow terminate; got %v", companion.calls) + if indexOf(companion.recorded(), "launch") >= 0 { + t.Fatalf("a refused launch must not reach the companion; got %v", companion.recorded()) } } -func TestLaunchClearStateFallbackWarnsOnce(t *testing.T) { - companion := &fakeCompanion{accessibilityJSON: "[]"} - output := &bytes.Buffer{} - d := newTestDriver(companion) - d.output = output - resets := 0 - d.resetContainer = func(context.Context) error { resets++; return nil } +func TestNewRejectsClearStateWithoutBundleID(t *testing.T) { + probe := &clearStateProbe{} + options := clearStateOptions(t, probe, "NO-BUNDLE-UDID", true) + options.BundleID = "" - for i := 0; i < 2; i++ { - if err := d.Launch(context.Background(), "", true, nil); err != nil { - t.Fatalf("Launch %d: %v", i, err) - } + if _, err := New(context.Background(), options); err == nil || !strings.Contains(err.Error(), "BundleID") { + t.Fatalf("New err = %v, want a refusal naming BundleID", err) } - if resets != 2 { - t.Fatalf("resetContainer called %d times, want 2", resets) - } - warnings := strings.Count(output.String(), "resetting the data container only") - if warnings != 1 { - t.Fatalf("warning emitted %d times, want once", warnings) - } - for _, call := range companion.calls { - if call == "install" || call == "uninstall" { - t.Fatalf("fallback path must not install/uninstall; got %v", companion.calls) - } + if got := probe.recorded(); len(got) != 0 { + t.Fatalf("calls = %v, want none: clearing an unnamed bundle would reinstall without resetting anything", got) } } @@ -951,6 +1054,175 @@ func TestLaunchLeavesATighterCallerDeadlineAlone(t *testing.T) { } } +// wedgedUntilRestartCompanion models the session a refused launch leaves +// behind: the refusal is never reported, and no later launch is answered until +// the session itself is replaced. +type wedgedUntilRestartCompanion struct { + fakeCompanion + mutex sync.Mutex + replaced bool + attempted int +} + +func (w *wedgedUntilRestartCompanion) replaceSession() { + w.mutex.Lock() + defer w.mutex.Unlock() + w.replaced = true +} + +func (w *wedgedUntilRestartCompanion) launchAttempts() int { + w.mutex.Lock() + defer w.mutex.Unlock() + return w.attempted +} + +func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bool) error { + w.mutex.Lock() + w.attempted++ + replaced := w.replaced + w.mutex.Unlock() + if replaced { + return nil + } + <-ctx.Done() + return ctx.Err() +} + +// TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound covers the FrontBoard +// race: a clear-state reinstall the simulator has not finished registering +// makes the session refuse the launch, and XCTest answers that refusal with +// minutes of diagnostics instead of an error, so the bound expires and every +// later call queues behind the same wedge. Calling launch again on that session +// cannot work; the run only recovers if the session is replaced first. +func TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound(t *testing.T) { + previous := launchTimeout + launchTimeout = 100 * time.Millisecond + defer func() { launchTimeout = previous }() + + companion := &wedgedUntilRestartCompanion{} + output := &bytes.Buffer{} + d := newTestDriver(companion) + d.output = output + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + companion.replaceSession() + return nil + } + + if err := d.Launch(context.Background(), "", false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + if restarts != 1 { + t.Fatalf("session restarts = %d, want exactly 1", restarts) + } + if attempts := companion.launchAttempts(); attempts != 2 { + t.Fatalf("launch attempts = %d, want 2: one that wedged and one on the replaced session", attempts) + } + if !strings.Contains(output.String(), "restarting the session") { + t.Fatalf("the recovery was silent, so a run that needed it never says so; output was %q", output.String()) + } +} + +// TestLaunchBoundsTheSessionRestartItTriggers keeps the recovery inside a +// budget of its own. The restart deliberately runs on the driver's lifetime +// context rather than the caller's, so without a deadline a session that never +// comes back would hang the launch path exactly the way #73 stopped it hanging. +func TestLaunchBoundsTheSessionRestartItTriggers(t *testing.T) { + previousLaunch, previousRecovery := launchTimeout, launchRecoveryTimeout + launchTimeout = 100 * time.Millisecond + launchRecoveryTimeout = 200 * time.Millisecond + defer func() { launchTimeout, launchRecoveryTimeout = previousLaunch, previousRecovery }() + + d := newTestDriver(&wedgedUntilRestartCompanion{}) + d.restart = func(restartCtx context.Context) error { + <-restartCtx.Done() + return restartCtx.Err() + } + + done := make(chan error, 1) + go func() { done <- d.Launch(context.Background(), "", false, nil) }() + select { + case err := <-done: + if err == nil || !strings.Contains(err.Error(), "session restart failed") { + t.Fatalf("err = %v, want the failed restart named", err) + } + case <-time.After(10 * time.Second): + t.Fatal("Launch never returned: a session that never comes back hangs the launch path") + } +} + +// TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires holds the recovery +// to the driver's own bound. Spending a session restart on a caller that has +// already run out of budget cannot produce a launch, only a later failure. +func TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires(t *testing.T) { + previous := launchTimeout + launchTimeout = 30 * time.Second + defer func() { launchTimeout = previous }() + + companion := &wedgedUntilRestartCompanion{} + d := newTestDriver(companion) + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + companion.replaceSession() + return nil + } + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + if err := d.Launch(ctx, "", false, nil); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err = %v, want a deadline-exceeded error", err) + } + if restarts != 0 { + t.Fatalf("session restarts = %d, want 0", restarts) + } +} + +// refusedLaunchCompanion answers a launch the way the runner does once it +// checks the app's state after activating it: promptly, naming the app and the +// state it reached, over a session that is still serving. +type refusedLaunchCompanion struct { + fakeCompanion + attempts int +} + +func (r *refusedLaunchCompanion) Launch(context.Context, string, bool) error { + r.attempts++ + return errors.New(`runner launch: failed("com.example.app is not running after launch")`) +} + +// TestLaunchKeepsTheSessionWhenTheRunnerNamesTheRefusal separates a launch that +// answers from a launch that never does. The session restart is the only +// recovery from a wedged session, and it costs a cold start; a runner that +// reports the app's state has already said what a fresh session would say, so +// restarting to hear it again only delays the error and hides the app under it. +func TestLaunchKeepsTheSessionWhenTheRunnerNamesTheRefusal(t *testing.T) { + companion := &refusedLaunchCompanion{} + output := &bytes.Buffer{} + d := newTestDriver(companion) + d.output = output + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + return nil + } + + err := d.Launch(context.Background(), "", false, nil) + if err == nil || !strings.Contains(err.Error(), "com.example.app is not running after launch") { + t.Fatalf("err = %v, want the runner's refusal reaching the caller intact", err) + } + if restarts != 0 { + t.Fatalf("session restarts = %d, want 0: a refusal the runner reported is not a wedged session", restarts) + } + if companion.attempts != 1 { + t.Fatalf("launch attempts = %d, want 1: relaunching an app the runner just refused cannot launch it", companion.attempts) + } + if strings.Contains(output.String(), "restarting the session") { + t.Fatalf("the driver announced a recovery it must not spend here; output was %q", output.String()) + } +} + // newLockTestOptions builds New options that dial a seamed companion, so the // device-lock tests exercise New without spawning anything. func newLockTestOptions(t *testing.T, udid string) Options { diff --git a/internal/hierarchy/hierarchy.go b/internal/hierarchy/hierarchy.go index c54a052..1f1e9b4 100644 --- a/internal/hierarchy/hierarchy.go +++ b/internal/hierarchy/hierarchy.go @@ -11,7 +11,8 @@ // descPrefix: - starts-with on content-desc / accessibilityText // // Object selectors (multi-attribute AND, element-scoped or global): -// { attr: value, ... } - all key/value pairs must match; substring / boolean semantics +// { attr: value, ... } - all key/value pairs must match, each key resolved by +// the same rule its string form above uses // // Path queries (global scan only, string form): // > > ... - each segment matched within subtree of previous match @@ -156,10 +157,17 @@ func matchAttr(element *Element, attr, value string) bool { return false } -// matchSelector returns true when all filters in sel match the element (AND semantics). +// matchSelector returns true when all filters in sel match the element (AND +// semantics). Each filter goes through match, the same rule the string form +// resolves a "kind:value" segment by, so {id: "Submit"} and "id:Submit" can +// never resolve to different elements. Applying matchAttr directly here made +// the object form skip the kind arms entirely: id, desc and descPrefix name no +// attribute any producer writes, so those keys matched NOTHING through an +// object selector while the string form matched, and every property over the +// missing element passed vacuously. func matchSelector(element *Element, sel Selector) bool { for _, f := range sel.Filters { - if !matchAttr(element, f.Attr, f.Value) { + if !match(element, f.Attr, f.Value) { return false } } diff --git a/internal/hierarchy/hierarchy_test.go b/internal/hierarchy/hierarchy_test.go index 91abd4c..395065f 100644 --- a/internal/hierarchy/hierarchy_test.go +++ b/internal/hierarchy/hierarchy_test.go @@ -1035,3 +1035,83 @@ func TestTreeTransitional(t *testing.T) { t.Error("nil tree must not be flagged as transitional") } } + +// selectorFormsDump carries one node per id shape a real dump produces, plus +// nodes carrying a description in the ", " form the desc rule knows about and a +// text the text rule matches on a substring. +const selectorFormsDump = `{ + "attributes": {"resource-id": "root", "bounds": "[0,0,400,800]"}, + "children": [ + {"attributes": {"resource-id": "BareThing", "bounds": "[0,0,100,50]"}, "children": []}, + {"attributes": {"resource-id": "com.example.app:id/AndroidThing", "bounds": "[0,50,100,100]"}, + "children": []}, + {"attributes": {"accessibilityIdentifier": "IosThing", "bounds": "[0,100,100,150]"}, + "children": []}, + {"attributes": {"resource-id": "Described", "content-desc": "Save, button", "bounds": "[0,150,100,200]"}, + "children": []}, + {"attributes": {"resource-id": "Labelled", "text": "Total balance", "bounds": "[0,200,100,250]"}, + "children": []} + ] +}` + +// TestSelectorFormsResolveTheSameElement holds the two selector forms a spec can +// write to ONE rule per key. A spec reaches these through state.ax.find: a +// string goes to FindNode, an object to FindBySelector, and the two ran +// different matchers. `id` has a kind arm that knows an Android resource id is +// package-qualified (com.example.app:id/Thing) and that a spec names the bare +// tail; the object form had no such arm and looked for a literal `id` attribute +// no producer writes, so {id: "Thing"} silently matched nothing on every +// platform while "id:Thing" matched. `desc` and `descPrefix` had the same +// split. A selector that resolves nothing makes every property over it +// vacuously true, which is the failure that reports a green run while checking +// nothing. +func TestSelectorFormsResolveTheSameElement(t *testing.T) { + tree, err := Parse(selectorFormsDump) + if err != nil { + t.Fatal(err) + } + for _, test := range []struct { + key string + value string + want string + }{ + {"id", "BareThing", "BareThing"}, + // A spec names the tail; an Android dump carries the package prefix. + {"id", "AndroidThing", "com.example.app:id/AndroidThing"}, + {"id", "com.example.app:id/AndroidThing", "com.example.app:id/AndroidThing"}, + {"id", "IosThing", "IosThing"}, + {"desc", "Save, button", "Described"}, + // The ", " form an accessibility label takes when a role is appended. + {"desc", "Save", "Described"}, + {"descPrefix", "Sav", "Described"}, + {"text", "Total", "Labelled"}, + {"resource-id", "BareThing", "BareThing"}, + {"testTag", "IosThing", "IosThing"}, + } { + t.Run(test.key+":"+test.value, func(t *testing.T) { + stringForm := test.key + ":" + test.value + fromString := tree.FindNode(stringForm) + if fromString == nil { + t.Fatalf("the string form %q resolved nothing", stringForm) + } + if fromString.ResourceID != test.want { + t.Fatalf("the string form resolved %q, want %q", fromString.ResourceID, test.want) + } + fromObject := tree.Root.FindBySelector( + Selector{Filters: []AttrFilter{{Attr: test.key, Value: test.value}}}, + ) + if fromObject == nil { + t.Fatalf( + "the object form {%s: %q} resolved nothing while %q resolved %q", + test.key, test.value, stringForm, fromString.ResourceID, + ) + } + if fromObject != fromString { + t.Errorf( + "one selector, two answers: {%s: %q} resolved %q and %q resolved %q", + test.key, test.value, fromObject.ResourceID, stringForm, fromString.ResourceID, + ) + } + }) + } +} diff --git a/internal/runner/composition_reread_test.go b/internal/runner/composition_reread_test.go new file mode 100644 index 0000000..5847d8e --- /dev/null +++ b/internal/runner/composition_reread_test.go @@ -0,0 +1,297 @@ +package runner + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/priyanshujain/sanderling/internal/driver" + mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock" +) + +// homeWithRows is one settled route whose list holds rows. A row arriving +// between two reads is what a Compose lazy list mounting over several frames +// looks like from the runner's side. +func homeWithRows(rows int) string { + var children strings.Builder + for row := range rows { + fmt.Fprintf(&children, + `,{"attributes":{"resource-id":"TxnRow%d","class":"android.view.View"},"children":[]}`, row) + } + return fmt.Sprintf( + `{"attributes":{"resource-id":"HomeScreen","class":"android.view.View"},"children":[ + {"attributes":{"resource-id":"TxnList","class":"android.view.View"},"children":[]}%s + ]}`, children.String()) +} + +// composesLateDriver answers the paired Snapshot with the frame the step +// records and the hierarchy read that follows with a tree that has grown a row, +// for the first composingReads reads of the run. After that both reads describe +// the same screen. +type composesLateDriver struct { + *mockdriver.Driver + composingReads int64 + reads atomic.Int64 +} + +func (d *composesLateDriver) Snapshot(context.Context) (string, driver.Image, error) { + return homeWithRows(1), driver.Image{PNG: []byte("png"), Width: 1, Height: 1}, nil +} + +func (d *composesLateDriver) Hierarchy(context.Context) (string, error) { + if d.reads.Add(1) <= d.composingReads { + return homeWithRows(2), nil + } + return homeWithRows(1), nil +} + +// A route can settle before its content composes, so a tree read the moment the +// route arrives can describe a screen that is still filling in. Verifying that +// step compares a half-composed frame against a settled one and convicts an app +// that did nothing wrong. Two reads a read apart see it happening, and the step +// they disagree on is one the verifier must never be handed. +// +// The always-false property is the witness: it fires on the first step the +// verifier evaluates, so the step index of its violation says exactly which +// step reached the verifier. +func TestRunner_AStepWhoseTreeChangedBetweenReadsIsNotVerified(t *testing.T) { + run := func(t *testing.T, composingReads int64) (Summary, string) { + t.Helper() + state := newHarnessWithSpec(t, violationSpec) + device := &composesLateDriver{Driver: state.mock, composingReads: composingReads} + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 3, + Driver: device, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.Steps != 3 { + t.Fatalf("steps = %d, want 3", summary.Steps) + } + return summary, state.writer.Directory() + } + + t.Run("the step it changed on is skipped, the next one is judged", func(t *testing.T) { + summary, directory := run(t, 1) + if len(summary.Violations) != 1 { + t.Fatalf("violations = %v, want exactly one", summary.Violations) + } + violation := summary.Violations[0] + if violation.Properties[0] != "balanceNonNegative" { + t.Fatalf("violated %v, want balanceNonNegative", violation.Properties) + } + if violation.StepIndex != 2 { + t.Errorf("the property first judged step %d, want 2; the verifier was handed "+ + "a screen that grew a row while the runner was reading it", + violation.StepIndex) + } + if summary.SkippedVerification != 1 { + t.Errorf("the run reports %d step(s) judged by nothing, want 1", + summary.SkippedVerification) + } + // Skipped is not lost: the step is still recorded, screenshot and all, + // so the run can be replayed over the frame nothing judged. + steps := traceSteps(t, directory) + if len(steps) != 3 { + t.Fatalf("trace holds %d step(s), want 3", len(steps)) + } + if len(steps[0].Violations) != 0 { + t.Errorf("step 1 recorded violations %v; it was never verified", steps[0].Violations) + } + screenshot := filepath.Join(directory, "screenshots", "step-00001.png") + if _, err := os.Stat(screenshot); err != nil { + t.Errorf("expected the skipped step's screenshot at %s: %v", screenshot, err) + } + }) + + // The control. Two reads that agree must verify as they always did, + // otherwise the case above is just a runner that verifies nothing. + t.Run("two reads that agree verify the step", func(t *testing.T) { + summary, _ := run(t, 0) + if len(summary.Violations) != 1 { + t.Fatalf("violations = %v, want exactly one", summary.Violations) + } + if got := summary.Violations[0].StepIndex; got != 1 { + t.Errorf("the property first judged step %d, want 1; a settled screen must be "+ + "verified on the step it was read", got) + } + }) +} + +// submitsOnTapDriver commits a transaction on every tap, shows the running +// total in the tree, and grows a row under the hierarchy read that follows the +// paired Snapshot: on one chosen step, or on every one of them. +type submitsOnTapDriver struct { + *mockdriver.Driver + composingRead int64 + everyRead bool + reads atomic.Int64 + committed atomic.Int64 +} + +func (d *submitsOnTapDriver) Tap(context.Context, int, int) error { return d.commit() } +func (d *submitsOnTapDriver) TapSelector(context.Context, string) error { return d.commit() } + +func (d *submitsOnTapDriver) commit() error { + d.committed.Add(1) + return nil +} + +func (d *submitsOnTapDriver) Snapshot(context.Context) (string, driver.Image, error) { + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), driver.Image{}, nil +} + +func (d *submitsOnTapDriver) Hierarchy(context.Context) (string, error) { + if read := d.reads.Add(1); d.everyRead || read == d.composingRead { + return fmt.Sprintf(homeWithTxnCountComposing, d.committed.Load()), nil + } + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), nil +} + +// The same tree with one more row in it, which is what the reread sees while +// the screen is still filling in. +const homeWithTxnCountComposing = `{"attributes":{"resource-id":"HomeScreen"},"children":[ + {"attributes":{"resource-id":"TxnCount","text":"%d"},"children":[]}, + {"attributes":{"resource-id":"TxnSubmit","bounds":"[40,80,240,160]"},"children":[],"clickable":true,"enabled":true}, + {"attributes":{"resource-id":"TxnRowLate"},"children":[]} +]}` + +// Skipping a step is only free if nothing the spec needs goes missing with it. +// The action a step applies is reported to the spec on the NEXT step the +// verifier accepts, so a skipped step in between swallows the action before it: +// the transaction it committed still turns up in the next reading, and +// submitCommitsOneTransactionPerAction sees a rise nothing in its window +// accounts for. That is the conviction #77 and #78 are about, arriving through +// the skip rather than through the runner's report. +// +// So a frame the verifier will not look at is not one to act on either, which +// is also what #75 asked for: the fuzzer must not tap into a screen that is +// still filling in. +func TestRunner_ASkippedStepDoesNotSwallowTheActionBeforeIt(t *testing.T) { + spec := specWithFolioPredicates(t) + + run := func(t *testing.T, composingRead int64) (Summary, int64) { + t.Helper() + state := newHarnessWithSpec(t, spec) + device := &submitsOnTapDriver{Driver: state.mock, composingRead: composingRead} + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 3, + Driver: device, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.Steps != 3 { + t.Fatalf("steps = %d, want 3", summary.Steps) + } + return summary, device.committed.Load() + } + + t.Run("a submit is not lost to the step that follows it", func(t *testing.T) { + summary, committed := run(t, 2) + if summary.SkippedVerification != 1 { + t.Fatalf("the run skipped %d step(s), want 1; the reread never fired, so this "+ + "proves nothing", summary.SkippedVerification) + } + if committed == 0 { + t.Fatal("the device committed nothing; a runner that never acts passes this " + + "test without meaning anything") + } + if len(summary.Violations) != 0 { + t.Errorf("the counting property convicted a healthy app: %v\n"+ + "one transaction per submit rose, and a submit went unreported because "+ + "the step after it was skipped", summary.Violations) + } + }) + + // The control: with nothing composing, every step is verified and the same + // app is judged clean, so the case above is not just a runner that stopped + // judging. + t.Run("every step verified, same app, no violation", func(t *testing.T) { + summary, committed := run(t, 0) + if summary.SkippedVerification != 0 { + t.Fatalf("the run skipped %d step(s), want 0", summary.SkippedVerification) + } + if committed != 3 { + t.Fatalf("the device committed %d transaction(s), want 3", committed) + } + if len(summary.Violations) != 0 { + t.Errorf("the counting property convicted a healthy app: %v", summary.Violations) + } + }) +} + +// Holding an action back is bounded. A screen that changes shape under every +// pair of reads (a live list, a spinner mounting and unmounting) would +// otherwise take the whole run: nothing verified, nothing tapped, and a green +// summary at the end of it. +func TestRunner_AScreenThatNeverSettlesDoesNotStallTheRun(t *testing.T) { + state := newHarnessWithSpec(t, specWithFolioPredicates(t)) + device := &submitsOnTapDriver{Driver: state.mock, everyRead: true} + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 5, + Driver: device, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.SkippedVerification != 5 { + t.Fatalf("the run verified some step of a screen that never settled: skipped %d of 5", + summary.SkippedVerification) + } + if device.committed.Load() == 0 { + t.Error("the fuzzer never acted across 5 steps; a screen that keeps moving must " + + "cost the run a step or two, not all of them") + } +} + +type traceLine struct { + Step int `json:"step"` + Violations []string `json:"violations"` +} + +func traceSteps(t *testing.T, directory string) []traceLine { + t.Helper() + body, err := os.ReadFile(filepath.Join(directory, "trace.jsonl")) + if err != nil { + t.Fatal(err) + } + var steps []traceLine + for _, raw := range bytes.Split(bytes.TrimSpace(body), []byte("\n")) { + var line traceLine + if err := json.Unmarshal(raw, &line); err != nil { + t.Fatalf("decode trace line: %v", err) + } + steps = append(steps, line) + } + return steps +} diff --git a/internal/runner/foreground_guard_last_action_test.go b/internal/runner/foreground_guard_last_action_test.go new file mode 100644 index 0000000..8932f1b --- /dev/null +++ b/internal/runner/foreground_guard_last_action_test.go @@ -0,0 +1,287 @@ +package runner + +import ( + "context" + "fmt" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/priyanshujain/sanderling/internal/driver" + mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock" +) + +const guardedBundleID = "app.folio" + +// committingDevice is a device whose submit taps commit transactions the next +// hierarchy read shows, and which can say how many it has committed so a test +// can prove the taps landed before reading anything into a verdict. +type committingDevice interface { + driver.DeviceDriver + commits() int64 +} + +// leavesForegroundAfterSubmitDriver is the condition the app-scope guard exists +// for: the submit tap lands and commits, and the app is no longer the +// foreground app by the time the next step looks. Folio's transactions are in +// sqlite, so the commit survives the relaunch and the next reading shows it. +type leavesForegroundAfterSubmitDriver struct { + *mockdriver.Driver + commitsPerTap int64 + committed atomic.Int64 + away atomic.Bool +} + +func (d *leavesForegroundAfterSubmitDriver) Tap(context.Context, int, int) error { + return d.commitThenLeave() +} + +func (d *leavesForegroundAfterSubmitDriver) TapSelector(context.Context, string) error { + return d.commitThenLeave() +} + +func (d *leavesForegroundAfterSubmitDriver) commitThenLeave() error { + d.committed.Add(d.commitsPerTap) + d.away.Store(true) + return nil +} + +func (d *leavesForegroundAfterSubmitDriver) commits() int64 { return d.committed.Load() } + +func (d *leavesForegroundAfterSubmitDriver) Launch( + ctx context.Context, + bundleID string, + clearState bool, + env map[string]string, +) error { + d.away.Store(false) + return d.Driver.Launch(ctx, bundleID, clearState, env) +} + +func (d *leavesForegroundAfterSubmitDriver) ForegroundApp(context.Context) (string, error) { + if d.away.Load() { + return "com.android.launcher", nil + } + return guardedBundleID, nil +} + +func (d *leavesForegroundAfterSubmitDriver) FocusedWindowApp(ctx context.Context) (string, error) { + return d.ForegroundApp(ctx) +} + +func (d *leavesForegroundAfterSubmitDriver) Snapshot(context.Context) (string, driver.Image, error) { + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), driver.Image{}, nil +} + +// No device answers Snapshot and Hierarchy off different trees, and the runner +// reads both per step, so this one answers them off the same commit count. +func (d *leavesForegroundAfterSubmitDriver) Hierarchy(context.Context) (string, error) { + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), nil +} + +// obscuredAfterSubmitDriver is the other half of the same guard: the app stays +// the resumed activity, but a system window (the notification shade) owns the +// focused window when the next step looks, and the guard presses back to +// collapse it. +type obscuredAfterSubmitDriver struct { + *mockdriver.Driver + commitsPerTap int64 + committed atomic.Int64 + obscured atomic.Bool +} + +func (d *obscuredAfterSubmitDriver) Tap(context.Context, int, int) error { + return d.commitThenObscure() +} + +func (d *obscuredAfterSubmitDriver) TapSelector(context.Context, string) error { + return d.commitThenObscure() +} + +func (d *obscuredAfterSubmitDriver) commitThenObscure() error { + d.committed.Add(d.commitsPerTap) + d.obscured.Store(true) + return nil +} + +func (d *obscuredAfterSubmitDriver) commits() int64 { return d.committed.Load() } + +func (d *obscuredAfterSubmitDriver) PressKey(ctx context.Context, key string) error { + if key == "back" { + d.obscured.Store(false) + } + return d.Driver.PressKey(ctx, key) +} + +func (d *obscuredAfterSubmitDriver) ForegroundApp(context.Context) (string, error) { + return guardedBundleID, nil +} + +func (d *obscuredAfterSubmitDriver) FocusedWindowApp(context.Context) (string, error) { + if d.obscured.Load() { + return "com.android.systemui", nil + } + return guardedBundleID, nil +} + +func (d *obscuredAfterSubmitDriver) Snapshot(context.Context) (string, driver.Image, error) { + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), driver.Image{}, nil +} + +func (d *obscuredAfterSubmitDriver) Hierarchy(context.Context) (string, error) { + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), nil +} + +// runTwoSubmitSteps drives two steps of the shipped folio counting property +// against a device that commits on every tap, and hands back what the property +// decided. Both steps have to run: the first arms the comparison, the second is +// where the guard fires and the pair is judged. +func runTwoSubmitSteps( + t *testing.T, + state *harness, + device committingDevice, + commitsPerTap int64, +) []ViolationRecord { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: time.Hour, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 2, + BundleID: guardedBundleID, + Driver: device, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if summary.Steps != 2 { + t.Fatalf("steps = %d, want 2; the run never reached the step that judges the pair", + summary.Steps) + } + if got := device.commits(); got != commitsPerTap*2 { + t.Fatalf("the device committed %d transaction(s), want %d; the taps never reached it", + got, commitsPerTap*2) + } + return summary.Violations +} + +func countMockActions(state *harness, kind mockdriver.ActionKind, key string) int { + count := 0 + for _, action := range state.mock.Actions() { + if action.Kind != kind { + continue + } + if key != "" && action.Key != key { + continue + } + count++ + } + return count +} + +func specWithFolioPredicates(t *testing.T) string { + t.Helper() + predicates, err := filepath.Abs("../../examples/folio/sanderling/predicates.ts") + if err != nil { + t.Fatal(err) + } + return fmt.Sprintf(submitCountingSpecTemplate, predicates) +} + +// A relaunch is not proof that nothing ran before it. The submit was dispatched +// and confirmed; what the relaunch changed is that the app restarted between +// the two readings the property compares. Reporting "no action" for it hands +// submitCommitsOneTransactionPerAction a transaction rise of one against a +// window of zero submits, which is the conviction #77 fixed for the apply-error +// path, manufactured here out of the scope guard instead. +func TestRunner_ARelaunchDoesNotConvictTheSubmitCountingProperty(t *testing.T) { + spec := specWithFolioPredicates(t) + + run := func(t *testing.T, commitsPerTap int64) []ViolationRecord { + t.Helper() + state := newHarnessWithSpec(t, spec) + device := &leavesForegroundAfterSubmitDriver{ + Driver: state.mock, + commitsPerTap: commitsPerTap, + } + violations := runTwoSubmitSteps(t, state, device, commitsPerTap) + if countMockActions(state, mockdriver.ActionLaunch, "") == 0 { + t.Fatal("the app was never relaunched, so the guard this test is about never ran") + } + return violations + } + + t.Run("one transaction per tap is not a double submit", func(t *testing.T) { + if violations := run(t, 1); len(violations) != 0 { + t.Errorf("the counting property convicted a healthy app: %v\n"+ + "one transaction rose against a submit the runner confirmed, and the "+ + "spec was told no action happened because the app was relaunched", + violations) + } + }) + + // The control. Without it a green above proves nothing: a property that + // never sees a comparable pair is silently vacuous and reports the same + // empty violation list. + t.Run("two transactions per tap still convicts", func(t *testing.T) { + violations := run(t, 2) + if len(violations) == 0 { + t.Fatal("the counting property missed a double submit; the harness never " + + "put the property in a position to fire, so the case above proves nothing") + } + if violations[0].Properties[0] != "submitCommitsOneTransactionPerAction" { + t.Errorf("violated %v, want submitCommitsOneTransactionPerAction", violations[0].Properties) + } + }) +} + +// The same hole through the guard's other branch. A system window holding the +// focus says nothing about whether the tap under it ran: it was dispatched, and +// what nobody can say afterwards is whether the app received it. That is the +// unknown `applied` already carries, and it counts toward the submits a window +// could hold. Reporting no action instead convicts the app of a transaction +// with no cause. +func TestRunner_AnOverlayDoesNotConvictTheSubmitCountingProperty(t *testing.T) { + spec := specWithFolioPredicates(t) + + run := func(t *testing.T, commitsPerTap int64) []ViolationRecord { + t.Helper() + state := newHarnessWithSpec(t, spec) + device := &obscuredAfterSubmitDriver{ + Driver: state.mock, + commitsPerTap: commitsPerTap, + } + violations := runTwoSubmitSteps(t, state, device, commitsPerTap) + if countMockActions(state, mockdriver.ActionPressKey, "back") == 0 { + t.Fatal("the overlay was never dismissed, so the guard this test is about never ran") + } + if countMockActions(state, mockdriver.ActionLaunch, "") != 0 { + t.Fatal("a resumed-but-obscured app must not be relaunched") + } + return violations + } + + t.Run("one transaction per tap is not a double submit", func(t *testing.T) { + if violations := run(t, 1); len(violations) != 0 { + t.Errorf("the counting property convicted a healthy app: %v\n"+ + "one transaction rose against a submit the runner dispatched, and the "+ + "spec was told no action happened because a system window took the focus", + violations) + } + }) + + t.Run("two transactions per tap still convicts", func(t *testing.T) { + violations := run(t, 2) + if len(violations) == 0 { + t.Fatal("the counting property missed a double submit; the harness never " + + "put the property in a position to fire, so the case above proves nothing") + } + if violations[0].Properties[0] != "submitCommitsOneTransactionPerAction" { + t.Errorf("violated %v, want submitCommitsOneTransactionPerAction", violations[0].Properties) + } + }) +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 1ef8d5e..a7aac31 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -52,6 +52,11 @@ type Summary struct { EndTime time.Time Steps int Violations []ViolationRecord + // SkippedVerification counts the steps whose tree was still moving when it + // was read, so no property judged them. A green run that skipped most of + // its steps checked almost nothing, and nothing else in the output would + // say so. + SkippedVerification int // UnsupportedVerbs lists verbs the picker requested that the platform // could not dispatch, deduped, so the report can flag a spec exercising // gestures this target does not support. @@ -89,11 +94,13 @@ func Run(ctx context.Context, options Options) (Summary, error) { return Summary{}, err } _, pageExtractors := extractorSource.(webSource) + rereadHierarchy := driverIsAndroid(ctx, options, logger) summary := Summary{StartTime: time.Now()} deadline := summary.StartTime.Add(options.Duration) stepIndex := 0 consecutiveApplyFailures := 0 + heldSteps := 0 var lastAction *verifier.Action var lastLogTime time.Time for time.Now().Before(deadline) { @@ -110,8 +117,23 @@ func Run(ctx context.Context, options Options) (Summary, error) { // backed out of (or otherwise left) the app, relaunch it before we // observe or act, so properties never evaluate against a foreign app // and actions never land outside the app. - if ensureForeground(ctx, options, logger, stepIndex) { - lastAction = nil + // + // What the guard did is reported to the spec on the action it followed, + // because dropping that action says "nothing ran between these two + // readings" and the runner has no business saying that: the action ran, + // and a property told otherwise convicts the app of an effect with no + // cause. See foreground_guard_last_action_test.go. + guard := ensureForeground(ctx, options, logger, stepIndex) + if lastAction != nil { + switch guard { + case foregroundRelaunched: + lastAction.Relaunched = true + case foregroundOverlayDismissed: + // A system window owned the focused window, so whether the app + // itself ever received this action is exactly the unknown + // Applied already has a state for. + lastAction.Applied = false + } } // Hierarchy, metrics, and logs are independent device reads. Run @@ -132,7 +154,8 @@ func Run(ctx context.Context, options Options) (Summary, error) { // screenshot describe the same frame, then re-fetches the pair // while the tree still looks transitional. g.Go(func() error { - tree, screenshotPNG, transitional, hierarchyErr = fetchSyncedState(gctx, options, logger, si) + tree, screenshotPNG, transitional, hierarchyErr = fetchSyncedState( + gctx, options, logger, si, rereadHierarchy) return nil }) g.Go(func() error { @@ -173,8 +196,10 @@ func Run(ctx context.Context, options Options) (Summary, error) { screen = tree.Elements[0].Screen } - // Transitional trees describe a NavHost mid cross-fade. Pushing - // one would poison the verifier's previous/current extractor + // A transitional tree is one nothing can vouch for: a NavHost mid + // cross-fade, a screen that changed shape between two reads, or a + // hierarchy that came back empty. Pushing one would poison the + // verifier's previous/current extractor // advance, so the next clean step would compare against this // transient state and emit false-positive violations. We still // record the step (hierarchy + screenshot) for replay-side @@ -247,18 +272,44 @@ func Run(ctx context.Context, options Options) (Summary, error) { extractorChanges = encodeExtractorChanges(options.Verifier.ChangedExtractors()) } else { skippedVerification = true - logger.Warn("transitional tree after retry budget; skipping verifier", + summary.SkippedVerification++ + logger.Warn("unsettled tree; skipping verifier", "step", stepIndex, "screen", screen, "nodes", treeSize) } logger.Info("step", "index", stepIndex, "screen", screen, "nodes", treeSize) - nextAction, nextErr := actionSource.NextAction(ctx) + // A frame the verifier would not look at is not one to act on either. + // #75 is the fuzzer tapping into a screen that is still filling in, and + // holding the action back is also what keeps the spec's view of the run + // continuous: the action a step applies is reported on the NEXT step the + // verifier accepts, so acting here would leave the action applied last + // step unreported for good, and a property counting actions against + // their effects would then see an effect whose cause the runner + // swallowed. See TestRunner_ASkippedStepDoesNotSwallowTheActionBeforeIt. + // + // Bounded, because a screen that never settles must not stall the whole + // run: past the bound the runner acts anyway, which is where it was + // before this held anything back. + held := skippedVerification && heldSteps < maxHeldSteps + if held { + heldSteps++ + logger.Warn("screen still moving; holding this step's action back", + "step", stepIndex, "held", heldSteps) + } else { + heldSteps = 0 + } + + var nextAction verifier.Action + nextErr := verifier.ErrNoAction var traceAction *trace.Action - if nextErr == nil { - traceAction = traceActionFor(nextAction, tree) - stampActionSource(traceAction, actionSource) - } else if !errors.Is(nextErr, verifier.ErrNoAction) { - return summary, fmt.Errorf("step %d next action: %w", stepIndex, nextErr) + if !held { + nextAction, nextErr = actionSource.NextAction(ctx) + if nextErr == nil { + traceAction = traceActionFor(nextAction, tree) + stampActionSource(traceAction, actionSource) + } else if !errors.Is(nextErr, verifier.ErrNoAction) { + return summary, fmt.Errorf("step %d next action: %w", stepIndex, nextErr) + } } residuals, residualErr := encodeResiduals(options.Verifier.Residuals()) @@ -266,7 +317,7 @@ func Run(ctx context.Context, options Options) (Summary, error) { logger.Warn("residual encode failed", "step", stepIndex, "err", residualErr) } - applySkipped := false + applySkipped := held if nextErr == nil && !appIsForeground(ctx, options) { // The app left the foreground between observe and apply (a prior // action's gesture settling late, or an async navigation). The @@ -311,9 +362,12 @@ func Run(ctx context.Context, options Options) (Summary, error) { applied.Applied = true lastAction = &applied } - } else { + } else if !held { lastAction = nil } + // A held step leaves lastAction alone on purpose: nothing ran here, and + // the action it points at is still the one the next verified step has to + // be told about. step := trace.Step{ Index: stepIndex, @@ -396,6 +450,10 @@ func RenderSummary(w io.Writer, summary Summary, platform string) { fmt.Fprintf(w, " step %d: %v\n", violation.StepIndex, violation.Properties) } } + if summary.SkippedVerification > 0 { + fmt.Fprintf(w, "%d step(s) judged by nothing: the screen was still moving when it was read\n", + summary.SkippedVerification) + } if len(summary.UnsupportedVerbs) > 0 { fmt.Fprintf(w, "unsupported on %s: %s\n", platform, strings.Join(summary.UnsupportedVerbs, ", ")) @@ -445,20 +503,38 @@ func resolveIdleTimeout(options Options) time.Duration { return timeout } +// foregroundGuard is what ensureForeground had to do to put the app back in +// front. The two interventions are separate values because they are separate +// facts about the action they follow: a relaunch leaves it confirmed but +// straddling a restart, while a system window holding the focus leaves it +// dispatched with no way to tell whether the app received it. +type foregroundGuard int + +const ( + foregroundIntact foregroundGuard = iota + foregroundOverlayDismissed + foregroundRelaunched +) + // ensureForeground keeps the app under test in the foreground. When the driver // can report the foreground app and it no longer matches the bundle under test, -// the app is relaunched. Returns true when a relaunch happened so the caller -// can drop the now-stale lastAction. Drivers without ForegroundChecker (web, +// the app is relaunched. Reports what it did so the caller can pass that on to +// the spec through the previous action. Drivers without ForegroundChecker (web, // iOS) are a no-op. -func ensureForeground(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) bool { +func ensureForeground( + ctx context.Context, + options Options, + logger *slog.Logger, + stepIndex int, +) foregroundGuard { checker, ok := options.Driver.(driver.ForegroundChecker) if !ok || options.BundleID == "" { - return false + return foregroundIntact } foreground, err := checker.ForegroundApp(ctx) if err != nil { logger.Warn("foreground check failed", "step", stepIndex, "err", err) - return false + return foregroundIntact } if foreground != "" && foreground != options.BundleID { logger.Warn("app left foreground; relaunching", @@ -471,7 +547,7 @@ func ensureForeground(ctx context.Context, options Options, logger *slog.Logger, // window, so it never acts outside the app no matter how slow the // relaunch settles. awaitForeground(ctx, options, logger, stepIndex) - return true + return foregroundRelaunched } // The app is the resumed activity, but a system overlay can still own the // focused window while the app stays resumed: a fuzzer swipe starting in the @@ -481,15 +557,15 @@ func ensureForeground(ctx context.Context, options Options, logger *slog.Logger, // the app again. focusChecker, hasFocus := options.Driver.(driver.FocusedWindowChecker) if !hasFocus { - return false + return foregroundIntact } focused, err := focusChecker.FocusedWindowApp(ctx) if err != nil { logger.Warn("focus check failed", "step", stepIndex, "err", err) - return false + return foregroundIntact } if focused == "" || focused == options.BundleID { - return false + return foregroundIntact } logger.Warn("system window obscuring app; dismissing", "step", stepIndex, "focused", focused, "want", options.BundleID) @@ -497,7 +573,7 @@ func ensureForeground(ctx context.Context, options Options, logger *slog.Logger, logger.Warn("dismiss overlay failed", "step", stepIndex, "err", err) } settleForForeground(ctx, options) - return true + return foregroundOverlayDismissed } // appIsForeground reports whether the app under test currently owns the @@ -931,10 +1007,17 @@ const ( // orthogonal case where the frame itself is transitional. // // The transitional return reports whether the retry budget was exhausted -// on a still-transitional tree. Callers use it to skip the verifier for -// that step so the previous/current extractor advance does not absorb +// on a still-transitional tree, or (when reread is set) whether a second +// hierarchy read disagreed with the first. Callers use it to skip the verifier +// for that step so the previous/current extractor advance does not absorb // transient state. -func fetchSyncedState(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) (tree *hierarchy.Tree, png []byte, transitional bool, err error) { +func fetchSyncedState( + ctx context.Context, + options Options, + logger *slog.Logger, + stepIndex int, + reread bool, +) (tree *hierarchy.Tree, png []byte, transitional bool, err error) { var pngBytes []byte var previousJSON string retryLoop: @@ -970,6 +1053,9 @@ retryLoop: case <-timer.C: } } + if reread && err == nil && !transitional && changedOnReread(ctx, options, logger, stepIndex, tree) { + transitional = true + } if len(pngBytes) > 0 { if writeErr := options.TraceWriter.WriteScreenshot(stepIndex, pngBytes); writeErr != nil { logger.Warn("screenshot write failed", "step", stepIndex, "err", writeErr) @@ -978,6 +1064,88 @@ retryLoop: return tree, pngBytes, transitional, err } +// changedOnReread reads the hierarchy once more and reports whether the screen +// changed shape while we were looking at it. A Compose route can settle before +// its content composes (a lazy list mounts over several frames, a query lands a +// frame late), and a tree read in that window describes a screen that is still +// filling in. Two reads a read apart are the cheapest thing that can see it +// happening: the round trip IS the interval, so there is no sleep here. +// +// Waiting for the change to stop was measured on an API 34 device and refused: +// a 750ms-quiet poll capped at 2s cost a median 1434ms against 76ms for one +// read, hit its cap on every frame it fired for, and still handed back a frame +// that might be filling. Detecting is what the runner can act on, because a +// step it declines to verify is at worst a missed conviction, never a false +// one. +// +// A read that fails reports no change. Nothing about a dropped RPC says the +// screen was moving, and skipping verification on it would quietly spend the +// run's evidence on a flaky link. +func changedOnReread( + ctx context.Context, + options Options, + logger *slog.Logger, + stepIndex int, + first *hierarchy.Tree, +) bool { + // An empty tree is skipped by the caller anyway, so the read buys nothing. + if first == nil || len(first.Elements) == 0 { + return false + } + hierarchyJSON, err := options.Driver.Hierarchy(ctx) + if err != nil { + logger.Warn("second hierarchy read failed", "step", stepIndex, "err", err) + return false + } + second, err := hierarchy.Parse(hierarchyJSON) + if err != nil || second == nil { + logger.Warn("second hierarchy parse failed", "step", stepIndex, "err", err) + return false + } + if structuralShape(first) == structuralShape(second) { + return false + } + logger.Warn("screen changed between two reads; skipping verifier", + "step", stepIndex, "nodes", len(first.Elements), "then", len(second.Elements)) + return true +} + +// structuralShape renders what is on screen as its nodes' identities in tree +// order: how many there are, and which ids and classes they carry. +// +// Text and bounds are deliberately absent. A measure pass that moves pixels is +// not a screen still composing, and neither is a value arriving into a node +// that already exists, which this cannot tell apart from a clock ticking. This +// decides whether a property gets to judge at all, so it reads only what a +// change in what is on screen can move: a detector that fires on every step of +// a screen with a timer on it would leave the run green and vacuous, which is +// worse than the composition it set out to catch. The trade is measured rather +// than assumed: over 100 folio steps on an API 35 emulator, text moved under +// an unchanged shape on 1 step, and the shape itself moved on 1 other. +func structuralShape(tree *hierarchy.Tree) string { + var shape strings.Builder + for _, element := range tree.Elements { + shape.WriteString(element.ResourceID) + shape.WriteByte(0x1f) + shape.WriteString(element.Class) + shape.WriteByte(0x1e) + } + return shape.String() +} + +// driverIsAndroid asks the driver what it is, once per run, so the step loop +// never repeats the RPC. It gates the reread: #75 is about Compose composition, +// and web and iOS have their own settle paths and no measurement saying an +// extra hierarchy read there is cheap. An unreadable answer is not android. +func driverIsAndroid(ctx context.Context, options Options, logger *slog.Logger) bool { + health, err := options.Driver.Health(ctx) + if err != nil { + logger.Warn("health read failed; not rereading the hierarchy", "err", err) + return false + } + return health.Platform == "android" +} + func traceActionFor(action verifier.Action, tree *hierarchy.Tree) *trace.Action { traceAction := &trace.Action{Kind: string(action.Kind), X: action.X, Y: action.Y} switch action.Kind { @@ -1148,6 +1316,14 @@ func encodeResiduals(residuals map[string]ltl.Formula) (map[string]json.RawMessa // would be spent doing nothing. const maxConsecutiveApplyFailures = 3 +// maxHeldSteps bounds how many steps in a row the runner will decline to act on +// because their screen was still moving. It is a livelock bound, not a settle +// budget: a screen that changes shape under every pair of reads (a live list, a +// spinner that mounts and unmounts) would otherwise take the whole run without +// the fuzzer ever touching it. Two is what the measured cases need, which came +// one step at a time and never twice in a row. +const maxHeldSteps = 2 + // isWDADrop reports that the sidecar could not restart the iOS XCTest // runner: the channel is gone for good and the run must abort. Transient // drops are classified by the sidecar itself (it reconnects and surfaces diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 749289d..fa7cf94 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -196,6 +196,23 @@ func TestRenderSummary_OmitsUnsupportedLineWhenNone(t *testing.T) { } } +// A step nothing judged is not a step that passed. The run prints its count so +// a green summary cannot hide a run that skipped most of its steps, which is +// what a screen that keeps moving under the reads would produce. +func TestRenderSummary_CountsTheStepsNothingJudged(t *testing.T) { + var out bytes.Buffer + RenderSummary(&out, Summary{Steps: 10, SkippedVerification: 4}, "android") + if !strings.Contains(out.String(), "4 step(s) judged by nothing") { + t.Errorf("expected the unjudged-step count, got:\n%s", out.String()) + } + + out.Reset() + RenderSummary(&out, Summary{Steps: 10}, "android") + if strings.Contains(out.String(), "judged by nothing") { + t.Errorf("a run that judged every step must not print the line, got:\n%s", out.String()) + } +} + func TestRunner_ViolationSurfacesInSummary(t *testing.T) { state := newHarnessWithSpec(t, violationSpec) @@ -1073,8 +1090,15 @@ func TestRunner_UsesAtomicSnapshot(t *testing.T) { if snapshotCalls == 0 { t.Errorf("expected at least one Snapshot call, got %d", snapshotCalls) } - if hierarchyCalls != 0 { - t.Errorf("expected zero standalone Hierarchy calls (runner must use Snapshot), got %d", hierarchyCalls) + // The recorded pair still comes from Snapshot. The standalone hierarchy + // reads are the composition detector (changedOnReread), one per step at + // most, and they are never the source of what the step records. + if hierarchyCalls > summary.Steps { + t.Errorf("expected at most one standalone Hierarchy call per step (runner must observe through Snapshot), got %d over %d steps", + hierarchyCalls, summary.Steps) + } + if snapshotCalls < summary.Steps { + t.Errorf("expected a Snapshot per step, got %d over %d steps", snapshotCalls, summary.Steps) } if screenshotCalls != 0 { t.Errorf("expected zero standalone Screenshot calls (runner must use Snapshot), got %d", screenshotCalls) @@ -1894,8 +1918,10 @@ func TestEnsureForeground_DismissesSystemOverlay(t *testing.T) { logger := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelWarn})) options := Options{BundleID: "app.folio", Driver: m, IdleTimeout: 10 * time.Millisecond} - if !ensureForeground(context.Background(), options, logger, 5) { - t.Fatal("expected the guard to act on the focus-stealing overlay") + got := ensureForeground(context.Background(), options, logger, 5) + if got != foregroundOverlayDismissed { + t.Fatalf("the guard reported %v, want foregroundOverlayDismissed; "+ + "an obscured app is not a relaunched one", got) } backs, relaunches := 0, 0 for _, a := range m.Actions() { diff --git a/internal/runner/uncertain_last_action_test.go b/internal/runner/uncertain_last_action_test.go index f687e99..cd122f5 100644 --- a/internal/runner/uncertain_last_action_test.go +++ b/internal/runner/uncertain_last_action_test.go @@ -22,7 +22,7 @@ import ( // // The spec below is the real folio predicate pair, imported from the example, // so what this asserts is the verdict the shipped property reaches. -const uncertainApplySpecTemplate = ` +const submitCountingSpecTemplate = ` import { actions, always, extract, next, Tap } from "@sanderling/spec"; import { committedTransactionsExceedSubmits, @@ -90,12 +90,16 @@ func (d *dispatchThenFailDriver) Snapshot(context.Context) (string, driver.Image return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), driver.Image{}, nil } +func (d *dispatchThenFailDriver) Hierarchy(context.Context) (string, error) { + return fmt.Sprintf(homeWithTxnCount, d.committed.Load()), nil +} + func TestRunner_ApplyErrorAfterDispatchDoesNotConvictTheSubmitCountingProperty(t *testing.T) { predicates, err := filepath.Abs("../../examples/folio/sanderling/predicates.ts") if err != nil { t.Fatal(err) } - spec := fmt.Sprintf(uncertainApplySpecTemplate, predicates) + spec := fmt.Sprintf(submitCountingSpecTemplate, predicates) run := func(t *testing.T, commitsPerTap int64) []ViolationRecord { t.Helper() diff --git a/internal/runner/web_carrier_test.go b/internal/runner/web_carrier_test.go index d4776a5..c5cb169 100644 --- a/internal/runner/web_carrier_test.go +++ b/internal/runner/web_carrier_test.go @@ -55,6 +55,12 @@ func (d *carrierWebDriver) Snapshot(ctx context.Context) (string, driver.Image, func (d *carrierWebDriver) InstallBundle(context.Context, []byte) error { return nil } +// A web target says so. The runner's per-step hierarchy reread is android-only, +// and a fake claiming android would take a path no chrome run takes. +func (d *carrierWebDriver) Health(context.Context) (driver.Health, error) { + return driver.Health{Ready: true, Version: "fake", Platform: "web"}, nil +} + func (d *carrierWebDriver) EvaluateExtractors(context.Context) (map[int]json.RawMessage, error) { d.reads++ return map[int]json.RawMessage{0: json.RawMessage(strconv.Itoa(d.reads))}, nil diff --git a/internal/runner/web_last_action_test.go b/internal/runner/web_last_action_test.go index e3a7ecc..10ce20e 100644 --- a/internal/runner/web_last_action_test.go +++ b/internal/runner/web_last_action_test.go @@ -72,7 +72,7 @@ func TestRunner_WebInstallsLastActionInThePage(t *testing.T) { // Every later step carries what the runner actually applied. The shape is // the goja host's (internal/verifier/marshal.go lastActionFields), pinned // against it by TestLastAction_WebJSONMatchesTheGojaObject. - const want = `{"kind":"Tap","applied":true,"on":"id:TxnSubmit"}` + const want = `{"kind":"Tap","applied":true,"relaunched":null,"on":"id:TxnSubmit"}` if web.installed[1] != want { t.Errorf("step 2 installed %s, want %s", web.installed[1], want) } @@ -112,7 +112,7 @@ func TestRunner_WebInstallsAnUnconfirmedActionWithItsFateUnknown(t *testing.T) { t.Fatalf("the page was handed lastAction %d time(s); the web path never installed it", len(web.installed)) } - const want = `{"kind":"Tap","applied":null,"on":"id:TxnSubmit"}` + const want = `{"kind":"Tap","applied":null,"relaunched":null,"on":"id:TxnSubmit"}` if web.installed[1] != want { t.Errorf("step 2 installed %s, want %s", web.installed[1], want) } diff --git a/internal/testrun/driver.go b/internal/testrun/driver.go index a46a857..2bf3b5c 100644 --- a/internal/testrun/driver.go +++ b/internal/testrun/driver.go @@ -84,6 +84,17 @@ var newDeviceDriver = func(ctx context.Context, options ioscompanion.DeviceOptio return d, d.Close, nil } +// newSimulatorDriver constructs the iOS simulator driver and its cleanup. A +// seam so routing tests assert the run's options reach ioscompanion.Options +// without spawning a companion. +var newSimulatorDriver = func(ctx context.Context, options ioscompanion.Options) (driver.DeviceDriver, func(), error) { + d, err := ioscompanion.New(ctx, options) + if err != nil { + return nil, nil, err + } + return d, d.Close, nil +} + // buildDriver creates the appropriate DeviceDriver for the platform and returns // a cleanup function. For web, ChromeDriver is used directly. An iOS simulator // is driven by the native simulator companion (no JVM). A physical iOS device @@ -99,16 +110,17 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver } if options.Platform == "ios" && options.iosIsSimulator { - d, err := ioscompanion.New(ctx, ioscompanion.Options{ + d, cleanup, err := newSimulatorDriver(ctx, ioscompanion.Options{ UniqueDeviceIdentifier: options.iosUDID, BundleID: options.BundleID, AppPath: options.IosAppPath, + ClearState: options.ClearData, Output: stdout, }) if err != nil { return nil, nil, fmt.Errorf("ios simulator driver: %w", err) } - return d, d.Close, nil + return d, cleanup, nil } if options.Platform == "ios" { @@ -117,6 +129,7 @@ func buildDriver(ctx context.Context, options Options, stdout io.Writer) (driver CoreDeviceID: options.iosCoreDeviceID, BundleID: options.BundleID, AppPath: options.IosAppPath, + ClearState: options.ClearData, Output: stdout, }) if err != nil { diff --git a/internal/testrun/driver_test.go b/internal/testrun/driver_test.go index 5e461b3..653be9c 100644 --- a/internal/testrun/driver_test.go +++ b/internal/testrun/driver_test.go @@ -27,7 +27,7 @@ func TestBuildDriverRoutesPhysicalIOSToDeviceDriver(t *testing.T) { return stubDeviceDriver{}, func() { closed = true }, nil } - options := Options{Platform: "ios", BundleID: "app.folio", IosAppPath: "/tmp/iosApp.app"} + options := Options{Platform: "ios", BundleID: "app.folio", IosAppPath: "/tmp/iosApp.app", ClearData: true} options.iosIsSimulator = false options.iosUDID = "00008140-HW" options.iosCoreDeviceID = "CORE-1" @@ -45,12 +45,41 @@ func TestBuildDriverRoutesPhysicalIOSToDeviceDriver(t *testing.T) { if got.BundleID != "app.folio" || got.AppPath != "/tmp/iosApp.app" { t.Fatalf("DeviceOptions = %+v, want bundle and app path threaded through", got) } + if !got.ClearState { + t.Fatalf("DeviceOptions = %+v, want clear-data threaded through: the driver clears before its session, so a launch cannot", got) + } cleanup() if !closed { t.Fatal("cleanup must close the device driver") } } +func TestBuildDriverThreadsClearStateToTheSimulatorDriver(t *testing.T) { + stubPreflight(t) + original := newSimulatorDriver + t.Cleanup(func() { newSimulatorDriver = original }) + + var got ioscompanion.Options + newSimulatorDriver = func(_ context.Context, options ioscompanion.Options) (driver.DeviceDriver, func(), error) { + got = options + return stubDeviceDriver{}, func() {}, nil + } + + options := Options{Platform: "ios", BundleID: "app.folio", IosAppPath: "/tmp/iosApp.app", ClearData: true} + options.iosIsSimulator = true + options.iosUDID = "SIM-UDID" + + if _, _, err := buildDriver(context.Background(), options, io.Discard); err != nil { + t.Fatalf("buildDriver: %v", err) + } + if got.UniqueDeviceIdentifier != "SIM-UDID" || got.BundleID != "app.folio" || got.AppPath != "/tmp/iosApp.app" { + t.Fatalf("Options = %+v, want the resolved target, bundle and app path", got) + } + if !got.ClearState { + t.Fatalf("Options = %+v, want clear-data threaded through: the driver clears before its session, so a launch cannot", got) + } +} + func TestBuildDriverSurfacesDeviceConstructionError(t *testing.T) { stubPreflight(t) original := newDeviceDriver diff --git a/internal/verifier/ax_integration_test.go b/internal/verifier/ax_integration_test.go index 97fd596..6a3589b 100644 --- a/internal/verifier/ax_integration_test.go +++ b/internal/verifier/ax_integration_test.go @@ -2,6 +2,7 @@ package verifier import ( "os" + "strconv" "testing" "github.com/priyanshujain/sanderling/internal/hierarchy" @@ -99,3 +100,77 @@ func TestStateAxFindWorks(t *testing.T) { t.Fatalf("findAll count = %d, want 1", count) } } + +// axSelectorFormsTree carries one node per id shape a dump produces: the bare +// tag Compose and the web driver emit, the package-qualified resource id +// Android emits, and the iOS accessibility identifier. +const axSelectorFormsTree = `{ + "attributes": {"resource-id": "root", "bounds": "[0,0,400,800]"}, + "children": [ + {"attributes": {"resource-id": "BareThing", "text": "bare", "bounds": "[0,0,100,50]"}, + "children": []}, + {"attributes": {"resource-id": "com.example.app:id/AndroidThing", "text": "android", + "bounds": "[0,50,100,100]"}, "children": []}, + {"attributes": {"accessibilityIdentifier": "IosThing", "text": "ios", + "bounds": "[0,100,100,150]"}, "children": []} + ] +}` + +// TestStateAxSelectorFormsAgree drives both selector forms a spec can write +// through state.ax.find and holds them to the same element. The two forms +// dispatch to different lookups (findNodeFromJS sends a string to FindNode and +// an object to FindBySelector), and the object one used to skip the id rule +// that knows an Android resource id is package-qualified, so a spec that wrote +// ax.find({id: "AddAccountSubmit"}) got undefined on Android and every property +// reading it passed while checking nothing. +func TestStateAxSelectorFormsAgree(t *testing.T) { + tree, err := hierarchy.Parse(axSelectorFormsTree) + if err != nil { + t.Fatal(err) + } + for _, test := range []struct { + value string + want string + }{ + {"BareThing", "bare"}, + {"AndroidThing", "android"}, + {"com.example.app:id/AndroidThing", "android"}, + {"IosThing", "ios"}, + } { + t.Run(test.value, func(t *testing.T) { + verifier := newVerifier(t) + mustLoad(t, verifier, ` + globalThis.fromObject = __sanderling__.extract( + state => state.ax.find({ id: `+strconv.Quote(test.value)+` })?.text, "fromObject"); + globalThis.fromString = __sanderling__.extract( + state => state.ax.find("id:" + `+strconv.Quote(test.value)+`)?.text, "fromString"); + globalThis.properties = {}; + `) + if err := verifier.PushSnapshot(SnapshotInput{Tree: tree}); err != nil { + t.Fatal(err) + } + fromObject := readCurrent(t, verifier, "fromObject") + fromString := readCurrent(t, verifier, "fromString") + if fromString != test.want { + t.Fatalf(`ax.find("id:%s") read %v, want %q`, test.value, fromString, test.want) + } + if fromObject != fromString { + t.Errorf( + `one selector, two answers: ax.find({id: %q}) read %v and ax.find("id:%s") read %v`, + test.value, fromObject, test.value, fromString, + ) + } + }) + } +} + +// readCurrent returns a named extractor's current value, or nil when the getter +// returned undefined, which is what an unresolved selector produces. +func readCurrent(t *testing.T, verifier *Verifier, name string) any { + t.Helper() + handle := verifier.runtime.GlobalObject().Get(name) + if handle == nil { + t.Fatalf("%s is not defined", name) + } + return handle.ToObject(verifier.runtime).Get("current").Export() +} diff --git a/internal/verifier/extractor_encoding_test.go b/internal/verifier/extractor_encoding_test.go index a693dbf..40b9a32 100644 --- a/internal/verifier/extractor_encoding_test.go +++ b/internal/verifier/extractor_encoding_test.go @@ -177,3 +177,79 @@ func compactJSON(t *testing.T, source string) string { } return compact.String() } + +// TestExtractorEncoding_NestedUndefinedIsNotOnTheWire pins the one reading +// shape the two hosts do NOT encode alike, rather than hiding it. +// +// JSON has no undefined, so the page loses the whole key (asserted in +// pkg/spec/test/web-runtime.test.ts) while goja writes null. goja cannot mirror +// the drop: Export reports an undefined member and a null member identically as +// nil, so dropping those keys here would drop the genuine nulls the page keeps. +// Mirroring the other way, by writing null on the page, would break the one +// thing that does agree. Carrying the member across takes a wire format that +// can express undefined, which is a change to every layer that parses a reading +// and to the replay UI that renders one. +// +// So the guarantee is narrower than "the same object": both hosts answer +// undefined when a property READS the member. Key presence (`in`, Object.keys) +// is not part of it, and this test says so out loud, so closing the gap has to +// be a deliberate change to both hosts at once. +func TestExtractorEncoding_NestedUndefinedIsNotOnTheWire(t *testing.T) { + const reading = `({ absent: undefined, empty: null, present: 1 })` + const fromGoja = `{"absent":null,"empty":null,"present":1}` + // What the page sends for the same getter, with the key gone. + const fromWeb = `{"empty":null,"present":1}` + + if got := encodeSpecValue(t, reading); got != fromGoja { + t.Errorf("goja encoded the reading as %s, want %s", got, fromGoja) + } + + native := newVerifier(t) + mustLoad(t, native, "__sanderling__.extract(state => "+reading+", \"value\");\nglobalThis.properties = {};") + if err := native.PushSnapshot(SnapshotInput{}); err != nil { + t.Fatal(err) + } + + web := newVerifier(t) + mustLoad(t, web, "__sanderling__.extract(state => null, \"value\");\nglobalThis.properties = {};") + if err := web.PushSnapshot(SnapshotInput{}); err != nil { + t.Fatal(err) + } + if _, err := web.OverrideExtractorValues(map[int]json.RawMessage{0: json.RawMessage(fromWeb)}); err != nil { + t.Fatal(err) + } + + for _, probe := range []struct { + expression string + native bool + web bool + }{ + {"reading.absent === undefined", true, true}, + {"reading.empty === null", true, true}, + {"reading.present === 1", true, true}, + // The half that does not survive the wire. + {`"absent" in reading`, true, false}, + } { + if got := evaluateAgainstReading(t, native, probe.expression); got != probe.native { + t.Errorf("goja host: %s is %v, want %v", probe.expression, got, probe.native) + } + if got := evaluateAgainstReading(t, web, probe.expression); got != probe.web { + t.Errorf("web host: %s is %v, want %v", probe.expression, got, probe.web) + } + } +} + +// evaluateAgainstReading answers a boolean expression over the value a property +// would read out of the first extractor, which is where the two hosts have to +// agree. +func evaluateAgainstReading(t *testing.T, verifier *Verifier, expression string) bool { + t.Helper() + if err := verifier.runtime.GlobalObject().Set("reading", verifier.extractors[0].currentValue); err != nil { + t.Fatal(err) + } + value, err := verifier.runtime.RunString(expression) + if err != nil { + t.Fatalf("evaluate %s: %v", expression, err) + } + return value.ToBoolean() +} diff --git a/internal/verifier/marshal.go b/internal/verifier/marshal.go index d738e84..18b2ac6 100644 --- a/internal/verifier/marshal.go +++ b/internal/verifier/marshal.go @@ -353,9 +353,20 @@ func lastActionFields(action *Action) []actionField { if action.Applied { applied = true } + // A relaunch between two readings is not "no action happened", which is + // what dropping the action reported instead: the app restarted after an + // action that did run. Only the positive report is a fact the runner can + // vouch for, so the other side is null rather than false: a target whose + // foreground the runner cannot read (web, iOS) never relaunches the app and + // still cannot promise it never restarted. + var relaunched any + if action.Relaunched { + relaunched = true + } fields := []actionField{ {key: "kind", value: string(action.Kind)}, {key: "applied", value: applied}, + {key: "relaunched", value: relaunched}, } if action.On != "" { fields = append(fields, actionField{key: "on", value: action.On}) diff --git a/internal/verifier/marshal_test.go b/internal/verifier/marshal_test.go index b76e480..b3a1a56 100644 --- a/internal/verifier/marshal_test.go +++ b/internal/verifier/marshal_test.go @@ -169,6 +169,7 @@ func TestLastAction_WebJSONMatchesTheGojaObject(t *testing.T) { {"nil", nil}, {"Tap", &Action{Kind: ActionKindTap, On: "id:TxnSubmit", X: 12, Y: 34}}, {"TapApplied", &Action{Kind: ActionKindTap, On: "id:TxnSubmit", Applied: true}}, + {"TapRelaunched", &Action{Kind: ActionKindTap, On: "id:TxnSubmit", Applied: true, Relaunched: true}}, {"TapWithoutSelector", &Action{Kind: ActionKindTap, X: 12, Y: 34}}, {"DoubleTap", &Action{Kind: ActionKindDoubleTap, On: `desc:say "hi" `}}, {"InputText", &Action{Kind: ActionKindInputText, On: "id:field", Text: "50"}}, @@ -233,3 +234,51 @@ func TestLastAction_SeparatesNoActionFromAnActionOfUnknownFate(t *testing.T) { }) } } + +// The runner relaunches the app when it leaves the foreground, which used to be +// reported to the spec as "no action ran between these two readings". The +// action did run; what a property cannot assume across it is that app state was +// continuous, so the relaunch is its own fact on an action that keeps its +// confirmed dispatch. +func TestLastAction_ReportsARelaunchSeparatelyFromTheDispatch(t *testing.T) { + verifier := newVerifier(t) + mustLoad(t, verifier, ` + globalThis.continuity = __sanderling__.extract(state => + state.lastAction === null ? "no action" + : state.lastAction.applied !== true ? "unconfirmed" + : state.lastAction.relaunched === true ? "applied across a relaunch" + : state.lastAction.relaunched === null ? "applied, no relaunch reported" + : "unreadable"); + `) + + for _, testCase := range []struct { + name string + action *Action + want string + }{ + {"nothing ran", nil, "no action"}, + { + "confirmed, app stayed", + &Action{Kind: ActionKindTap, On: "id:TxnSubmit", Applied: true}, + "applied, no relaunch reported", + }, + { + "confirmed, app relaunched after it", + &Action{Kind: ActionKindTap, On: "id:TxnSubmit", Applied: true, Relaunched: true}, + "applied across a relaunch", + }, + } { + t.Run(testCase.name, func(t *testing.T) { + if err := verifier.PushSnapshot(SnapshotInput{ + Snapshots: Snapshots{}, + LastAction: testCase.action, + }); err != nil { + t.Fatal(err) + } + handle := verifier.runtime.GlobalObject().Get("continuity").ToObject(verifier.runtime) + if got := handle.Get("current").String(); got != testCase.want { + t.Errorf("the spec read %q, want %q", got, testCase.want) + } + }) + } +} diff --git a/internal/verifier/types.go b/internal/verifier/types.go index 125703b..09d2c5c 100644 --- a/internal/verifier/types.go +++ b/internal/verifier/types.go @@ -38,6 +38,12 @@ type Action struct { // when the apply call failed and nothing can say whether the action // reached the app. The spec is told which of the two it is. Applied bool + // Relaunched, like Applied, is meaningful only on state.lastAction: the + // runner brought the app back to the foreground after this action, so the + // two readings the spec compares straddle a restart. The action still + // happened; what a property cannot assume across it is that app state ran + // continuously from one reading to the next. + Relaunched bool } // LogEntry mirrors a logcat line captured between steps. diff --git a/pkg/spec/LICENSE b/pkg/spec/LICENSE new file mode 100644 index 0000000..8755b39 --- /dev/null +++ b/pkg/spec/LICENSE @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright 2026 Priyanshu Jain + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/pkg/spec/src/types.ts b/pkg/spec/src/types.ts index c28fd05..9d3cdf8 100644 --- a/pkg/spec/src/types.ts +++ b/pkg/spec/src/types.ts @@ -109,8 +109,17 @@ export interface ExceptionRecord { * tap landed. Null is unknown, not "it did not happen" (`state.lastAction` is * itself null for that), so a property attributing an effect to this action * has to decline unless `applied` is true. + * + * `relaunched` is true when the runner had to bring the app back to the + * foreground after this action, so the previous reading and the current one + * straddle a restart. The action itself still happened; what a property cannot + * assume across it is that app state ran continuously between the two readings, + * and one demanding an effect of this action has to decline. Null is "not + * reported", which is weaker than "the app never restarted": a target whose + * foreground the runner cannot read never relaunches the app and cannot promise + * that either. */ -export type LastAction = Action & { applied: true | null }; +export type LastAction = Action & { applied: true | null; relaunched: true | null }; export interface State { snapshots: Snapshots; diff --git a/pkg/spec/src/web-runtime.ts b/pkg/spec/src/web-runtime.ts index 24deca8..a607758 100644 --- a/pkg/spec/src/web-runtime.ts +++ b/pkg/spec/src/web-runtime.ts @@ -193,13 +193,18 @@ function selectorFromString(selector: string): { css?: string; xpath?: string } // (Compose for Web mounts its canvas and its whole accessibility tree inside a // shadow root on the mount element) keeps its entire UI on the far side of one: // without this a spec sees four nodes and can neither enumerate a target nor -// resolve a testTag. Light-DOM matches come first, then shadow content in walk -// order. XPath has no equivalent, so `text:` selectors stop at the boundary. +// resolve a testTag. Matches come back in the order expandShadowContent walks +// and buildTree (internal/driver/chrome/driver.go) emits: a host, then that +// host's shadow content, then the host's light children. Sweeping the light DOM +// first and descending afterwards put a shadow-hosted match behind a later +// light-DOM one, so find() answered with a different element on each host. +// XPath has no equivalent, so `text:` selectors stop at the boundary. function deepQueryAll(selector: string, root: ParentNode): Element[] { const found: Element[] = []; const visit = (scope: ParentNode): void => { - for (const element of Array.from(scope.querySelectorAll(selector))) found.push(element); + const matched = new Set(Array.from(scope.querySelectorAll(selector))); for (const element of Array.from(scope.querySelectorAll("*"))) { + if (matched.has(element)) found.push(element); if (element.shadowRoot) visit(element.shadowRoot); } }; diff --git a/pkg/spec/test/folio-account-card-parse.test.ts b/pkg/spec/test/folio-account-card-parse.test.ts index 6b046b3..18230f3 100644 --- a/pkg/spec/test/folio-account-card-parse.test.ts +++ b/pkg/spec/test/folio-account-card-parse.test.ts @@ -65,6 +65,18 @@ test("name ending in digits does not leak into the balance", () => { assert.equal(balanceOf(card("20", "2024", 3, "-$1,234.56")), -123456); }); +// The limit of a key read off merged text, and the reason homeTxnCountsOf +// guards the ambiguity rather than resolving it: two DIFFERENT accounts render +// the same card, character for character. Names are unique (Accounts.name is +// UNIQUE, checked NOCASE) but the count runs straight into a name that ends in +// digits, so nothing computed from this string can say which account it is. +test("two accounts can render one card, so no key off it can be injective", () => { + const travel1 = card("TR", "Travel1", 25, "$120.00"); + const travel12 = card("TR", "Travel12", 5, "$120.00"); + assert.equal(travel1, travel12); + assert.equal(cardAccountName({ childText: undefined, cardText: travel1 }), "TRTravel"); +}); + // The account key only has to be stable and per-account. newAccountBalanceIsZero // reads it as a set member: a key that drifted as an account's transaction // count grew would make an existing account look brand new, and the property diff --git a/pkg/spec/test/folio-ledger-window.test.ts b/pkg/spec/test/folio-ledger-window.test.ts new file mode 100644 index 0000000..49bf8e5 --- /dev/null +++ b/pkg/spec/test/folio-ledger-window.test.ts @@ -0,0 +1,441 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + committedAmountExceedsOneSubmit, + committedTransactionsExceedSubmits, + countSubmitsInWindow, + homeTxnCountsOf, + parseAccountBalance, + parseTypedAmount, + readAccountBalance, + readHomeCards, +} from "../../../examples/folio/sanderling/predicates.ts"; +import type { + ObservedAction, + TxnCount, +} from "../../../examples/folio/sanderling/predicates.ts"; + +// The per-account balance the ledger and the add-transaction screen both show. +// Its window closes on every frame of the transaction flow, where the Home +// total's closes only when the walk goes back to Home: the iOS run in #78 went +// 117 steps between two Home readings and accumulated 37 submits against a rise +// of 15, so the double tap at step 32 sat in a window far too wide to judge. + +const submit: ObservedAction = { + kind: "Tap", + on: "testTag:AddTransactionScreen > testTag:TxnSubmit", + applied: true, +}; +const doubleSubmit: ObservedAction = { ...submit, kind: "DoubleTap" }; +const openLedger: ObservedAction = { + kind: "Tap", + on: "testTag:HomeScreen > testTag:AccountCard", + applied: true, +}; +const openAddTxn: ObservedAction = { + kind: "Tap", + on: "testTag:LedgerScreen > testTag:AddTransactionButton", + applied: true, +}; +const typeAmount: ObservedAction = { + kind: "InputText", + on: "testTag:AddTransactionScreen > testTag:TxnAmountField", + applied: true, +}; +const goBack: ObservedAction = { kind: "Tap", on: "testTag:BackButton", applied: true }; + +test("the ledger writes the balance bare and the add-transaction header labels it", () => { + assert.equal(parseAccountBalance("$196.00"), 19600); + assert.equal(parseAccountBalance("Balance: $196.00"), 19600); + assert.equal(parseAccountBalance("-$1,234.56"), -123456); + assert.equal(parseAccountBalance("Balance: -$1,234.56"), -123456); + assert.equal(parseAccountBalance("$0.00"), 0); +}); + +test("a balance that is not a complete amount is unknown, not zero", () => { + assert.equal(parseAccountBalance(undefined), null); + assert.equal(parseAccountBalance(""), null); + assert.equal(parseAccountBalance("Balance:"), null); + assert.equal(parseAccountBalance("$1,23.00"), null); +}); + +// Which account these numbers belong to is never asked, because inside a run of +// these two routes it cannot change: Route.Ledger is pushed only by tapping a +// card on Home, Route.AddTransaction only by the ledger's own button for its +// own account, and an accepted submit pops back to that same ledger. Reaching +// another account means passing through Home, so every frame that is not one of +// the two routes drops the carrier. +test("a frame off the account's own screens drops the carrier", () => { + for (const route of ["home", "login", "add-account", null]) { + assert.deepEqual( + readAccountBalance({ route, balanceText: "$196.00", previousCarrier: 10000 }), + { value: null, carrier: null, fresh: false }, + `route ${route} kept a carrier that may belong to another account`, + ); + } +}); + +test("a readable balance on either of the two screens closes the window", () => { + assert.deepEqual( + readAccountBalance({ route: "ledger", balanceText: "$196.00", previousCarrier: 10000 }), + { value: 19600, carrier: 19600, fresh: true }, + ); + assert.deepEqual( + readAccountBalance({ + route: "add-transaction", + balanceText: "Balance: $196.00", + previousCarrier: 10000, + }), + { value: 19600, carrier: 19600, fresh: true }, + ); +}); + +// The balance node scrolled out of the viewport is unknown, not a new value. +// The account still cannot have changed, so the carrier survives and the window +// stays open across the frame. +test("an unreadable balance keeps the carrier and does not close the window", () => { + assert.deepEqual( + readAccountBalance({ route: "ledger", balanceText: undefined, previousCarrier: 10000 }), + { value: 10000, carrier: 10000, fresh: false }, + ); +}); + +test("a double submit moves the account balance by twice what was typed", () => { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: doubleSubmit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: 49200, + }), + true, + ); +}); + +test("a double-submitted debit is caught by the same bound", () => { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: doubleSubmit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: -29200, + }), + true, + ); +}); + +test("one submit moving the balance by exactly the typed amount is the app working", () => { + for (const after of [29600, -9600]) { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: after, + }), + false, + ); + } +}); + +// A balance that has not moved is a commit still in flight (createTransaction +// runs in a coroutine), a submit the app rejected, or a tap that never landed. +// None of those is evidence, and an equality would convict all three. +test("a balance that has not moved yet is not evidence", () => { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: 10000, + }), + false, + ); +}); + +test("a window holding anything other than one submit is not attributable", () => { + for (const submitsInWindow of [0, 2, 37]) { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: 49200, + }), + false, + ); + } +}); + +test("an amount this reading cannot represent is vacuous, not a violation", () => { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: parseTypedAmount("not an amount"), + prevAccountBalance: 10000, + currAccountBalance: 49200, + }), + false, + ); + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: Number.MAX_SAFE_INTEGER + 2, + prevAccountBalance: 10000, + currAccountBalance: 49200, + }), + false, + ); +}); + +test("a balance too large to hold exactly is not compared", () => { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: Number.MAX_SAFE_INTEGER + 2, + currAccountBalance: 0, + }), + false, + ); + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 0, + currAccountBalance: Number.MAX_SAFE_INTEGER + 2, + }), + false, + ); +}); + +test("an unknown balance on either side is not evidence", () => { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: null, + currAccountBalance: 49200, + }), + false, + ); + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction: submit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: null, + }), + false, + ); +}); + +test("a step whose action was not a submit attributes nothing", () => { + for (const lastAction of [openLedger, openAddTxn, typeAmount, goBack, null]) { + assert.equal( + committedAmountExceedsOneSubmit({ + route: "ledger", + lastAction, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: 49200, + }), + false, + ); + } +}); + +test("Home shows every account's money, so it is not this comparison's scale", () => { + for (const route of ["home", "login", "add-account", null]) { + assert.equal( + committedAmountExceedsOneSubmit({ + route, + lastAction: doubleSubmit, + submitsInWindow: 1, + typedAmount: 19600, + prevAccountBalance: 10000, + currAccountBalance: 49200, + }), + false, + ); + } +}); + +// A step of the walk: the frame it landed on, the balance node that frame +// carried, what was in the amount field the step before, and the action that +// got there. Driven through the same carrier and window the spec holds. +interface Frame { + route: string | null; + balanceText?: string; + typed?: string; + lastAction: ObservedAction | null; +} + +function walk(frames: readonly Frame[]) { + let carrier: number | null = null; + let submits = 0; + const verdicts: { violated: boolean; balance: number | null; submits: number }[] = []; + let previous: number | null = null; + let typedBefore = ""; + for (const frame of frames) { + const reading = readAccountBalance({ + route: frame.route, + balanceText: frame.balanceText, + previousCarrier: carrier, + }); + carrier = reading.carrier; + const window = countSubmitsInWindow({ + previousCount: submits, + lastAction: frame.lastAction, + fresh: reading.fresh, + }); + submits = window.next; + verdicts.push({ + violated: committedAmountExceedsOneSubmit({ + route: frame.route, + lastAction: frame.lastAction, + submitsInWindow: window.reported, + typedAmount: parseTypedAmount(typedBefore), + prevAccountBalance: previous, + currAccountBalance: reading.value, + }), + balance: reading.value, + submits: window.reported, + }); + previous = reading.value; + typedBefore = frame.typed ?? ""; + } + return verdicts; +} + +// The trajectory of #78: open an account, open the transaction form, type, +// double tap. Not one frame of it is Home, so the Home readings the counting +// invariant compares never advance and it has nothing to say about any of it. +// This is what a 117-step stretch of that run looked like, and it is why the +// double tap at step 32 went unconvicted. +test("the Home window cannot judge a walk that never goes Home", () => { + const cards = [{ name: "Checking", balance: 10000, count: 3 as TxnCount }]; + let carrier: Record | null = homeTxnCountsOf(cards); + let submits = 0; + for (const lastAction of [openLedger, openAddTxn, typeAmount, doubleSubmit]) { + const reading = readHomeCards({ route: "ledger", reading: null, previousCarrier: carrier }); + const previous = carrier; + carrier = reading.carrier; + const window = countSubmitsInWindow({ previousCount: submits, lastAction, fresh: reading.fresh }); + submits = window.next; + assert.equal( + committedTransactionsExceedSubmits({ + countsBefore: previous, + countsAfter: reading.value, + submitsInWindow: window.reported, + }), + false, + ); + } +}); + +// The same trajectory, judged where the app actually is. The frame the double +// tap lands on is the account's own ledger, so the window that closes there +// holds exactly the one action. +test("the double tap is convicted on the frame it lands on", () => { + const verdicts = walk([ + { route: "ledger", balanceText: "$100.00", lastAction: openLedger }, + { route: "add-transaction", balanceText: "Balance: $100.00", lastAction: openAddTxn }, + { route: "add-transaction", balanceText: "Balance: $100.00", typed: "196", lastAction: typeAmount }, + { route: "ledger", balanceText: "$492.00", lastAction: doubleSubmit }, + ]); + assert.deepEqual( + verdicts.map(v => v.violated), + [false, false, false, true], + ); + assert.equal(verdicts[3]?.submits, 1); +}); + +test("the same walk with one transaction committed is silent throughout", () => { + const verdicts = walk([ + { route: "ledger", balanceText: "$100.00", lastAction: openLedger }, + { route: "add-transaction", balanceText: "Balance: $100.00", lastAction: openAddTxn }, + { route: "add-transaction", balanceText: "Balance: $100.00", typed: "196", lastAction: typeAmount }, + { route: "ledger", balanceText: "$296.00", lastAction: submit }, + { route: "add-transaction", balanceText: "Balance: $296.00", lastAction: openAddTxn }, + { route: "add-transaction", balanceText: "Balance: $296.00", typed: "50", lastAction: typeAmount }, + { route: "ledger", balanceText: "$346.00", lastAction: submit }, + ]); + assert.deepEqual( + verdicts.map(v => v.violated), + [false, false, false, false, false, false, false], + ); +}); + +// The reading a healthy app must survive: transactions arriving between two +// readings that the window can no longer attribute to one action. The balance +// node is off the viewport for a stretch, so the two numbers the property would +// compare straddle two commits, and the balance moves by 296.00 against a typed +// 50.00. Two submits in the window is not one, so there is nothing to judge. +test("transactions arriving between two readings do not convict a healthy app", () => { + const verdicts = walk([ + { route: "ledger", balanceText: "$100.00", lastAction: openLedger }, + { route: "add-transaction", lastAction: openAddTxn }, + { route: "add-transaction", typed: "196", lastAction: typeAmount }, + { route: "ledger", lastAction: submit }, + { route: "add-transaction", lastAction: openAddTxn }, + { route: "add-transaction", typed: "100", lastAction: typeAmount }, + { route: "ledger", balanceText: "$396.00", lastAction: submit }, + ]); + assert.deepEqual( + verdicts.map(v => v.violated), + [false, false, false, false, false, false, false], + ); + assert.equal(verdicts[6]?.submits, 2); + assert.equal(verdicts[6]?.balance, 39600); +}); + +// Attribution across accounts, which is the whole reason the carrier is dropped +// rather than carried. A $500.00 account is left behind for an empty one whose +// screens have not drawn their balance yet, and the submit into the new account +// lands with exactly one submit in the window: every gate this property has is +// open, and only the dropped carrier keeps it quiet. Carrying $500.00 across +// that frame reads as 30400 committed against 19600 typed, on an app that did +// nothing wrong. +test("a ledger opened for another account never inherits the old balance", () => { + for (const between of ["home", null]) { + const verdicts = walk([ + { route: "ledger", balanceText: "$500.00", lastAction: openAddTxn }, + { route: between, lastAction: goBack }, + { route: "ledger", lastAction: openLedger }, + { route: "add-transaction", typed: "196", lastAction: openAddTxn }, + { route: "ledger", balanceText: "$196.00", lastAction: submit }, + ]); + assert.deepEqual( + verdicts.map(v => v.violated), + [false, false, false, false, false], + `an account switch through ${between} was compared across accounts`, + ); + assert.equal(verdicts[4]?.submits, 1); + assert.equal(verdicts[4]?.balance, 19600); + } +}); diff --git a/pkg/spec/test/folio-new-account.test.ts b/pkg/spec/test/folio-new-account.test.ts index d5ee3ab..98845f8 100644 --- a/pkg/spec/test/folio-new-account.test.ts +++ b/pkg/spec/test/folio-new-account.test.ts @@ -1,7 +1,10 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { createdAccountHasNonZeroBalance } from "../../../examples/folio/sanderling/predicates.ts"; +import { + createdAccountHasNonZeroBalance, + initialsOf, +} from "../../../examples/folio/sanderling/predicates.ts"; const created = { kind: "Tap", @@ -209,9 +212,90 @@ test("the merged web key still matches the name that was typed", () => { ); }); -// Two cards answering to one typed name leave the appearance unattributable: -// the fuzzer creates duplicates from a five-name list, and the tree has been -// seen exposing the same card twice on a transition frame. +// The avatar the merged web key opens with, hand-computed off Format.kt rather +// than off the mirror, because a mirror checked against itself checks nothing. +// A single word gives its first two characters, several give the first letter +// of the first and of the last, and an empty name gives "?". +test("the initials a merged key opens with are the app's", () => { + const named: [string, string][] = [ + ["CH", "Checking"], + ["SA", "Savings"], + ["TR", "Travel"], + ["EF", "Emergency Fund"], + ["IN", "Investments"], + ["FU", "Fund"], + ["T2", "Travel 2024"], + ["A", "a"], + ["X9", "x9"], + ["-1", "-1"], + ["?", ""], + ["?", " "], + ]; + for (const [initials, name] of named) { + assert.equal(initialsOf(name), initials, `initials for ${JSON.stringify(name)}`); + } +}); + +// The attribution used to be a suffix test, and a suffix test hands the verdict +// to whichever OTHER account happens to end with the typed name. Home lists +// what fits the viewport, so the card that was just created is clipped out of +// the reading exactly as easily as any other, and the older account left in it +// is then judged for money it has held all along. +test("an older account whose name ends with the typed one is not the created one", () => { + assert.equal( + createdAccountHasNonZeroBalance({ + route: "home", + lastAction: created, + typedName: "Fund", + before: [account("Checking", 0)], + after: [account("Checking", 0), account("Emergency Fund", 461012300)], + }), + false, + ); +}); + +test("the merged web key is matched whole too, not by its ending", () => { + assert.equal( + createdAccountHasNonZeroBalance({ + route: "home", + lastAction: created, + typedName: "Fund", + before: [account("CHChecking", 0)], + after: [account("CHChecking", 0), account("EFEmergency Fund", 461012300)], + }), + false, + ); +}); + +// The card that was actually asked for is still judged, standing next to the +// account that merely ends with its name. +test("the created card is judged beside an account whose name ends with it", () => { + assert.equal( + createdAccountHasNonZeroBalance({ + route: "home", + lastAction: created, + typedName: "Fund", + before: [account("Emergency Fund", 461012300)], + after: [account("Emergency Fund", 461012300), account("Fund", 5000)], + }), + true, + ); + assert.equal( + createdAccountHasNonZeroBalance({ + route: "home", + lastAction: created, + typedName: "Fund", + before: [account("EFEmergency Fund", 461012300)], + after: [account("EFEmergency Fund", 461012300), account("FUFund", 5000)], + }), + true, + ); +}); + +// Two cards answering to one typed name leave the appearance unattributable. +// Accounts.name is UNIQUE and Repository.createAccount rejects a name already +// taken, so the pair is one card the tree exposed twice on a transition frame, +// or two names the merged web key cannot tell apart. test("two cards matching the typed name are not attributable to the creation", () => { assert.equal( createdAccountHasNonZeroBalance({ @@ -219,7 +303,7 @@ test("two cards matching the typed name are not attributable to the creation", ( lastAction: created, typedName: "Travel", before: [account("Checking", 0)], - after: [account("Checking", 0), account("Travel", 5000), account("MyTravel", 900)], + after: [account("Checking", 0), account("Travel", 5000), account("Travel", 900)], }), false, ); @@ -238,6 +322,40 @@ test("a card that was already there is not a card that was just created", () => ); }); +// The runner's foreground guard restarted the app after the create. A fresh +// launch draws Home from the top, so the visible set is whatever the new layout +// fits rather than what was there a step ago, and "appeared in the reading" is +// even less like "was created" than usual. The process may also have died +// before the write landed, which makes the card that carries the typed name an +// older account of that name coming into view. +test("a create the runner relaunched across attributes nothing", () => { + assert.equal( + createdAccountHasNonZeroBalance({ + route: "home", + lastAction: { ...created, relaunched: true }, + typedName: "Travel", + before: [account("Checking", 0)], + after: [account("Checking", 0), account("Travel", 5000)], + }), + false, + ); +}); + +test("no relaunch reported still judges the account that was created", () => { + for (const relaunched of [null, undefined]) { + assert.equal( + createdAccountHasNonZeroBalance({ + route: "home", + lastAction: { ...created, relaunched }, + typedName: "Travel", + before: [account("Checking", 0)], + after: [account("Checking", 0), account("Travel", 5000)], + }), + true, + ); + } +}); + // The apply call failed with the gesture possibly already delivered, so nobody // knows whether that account was created. The card carrying the typed name may // be an older one that scrolled into view, and attributing it to a creation diff --git a/pkg/spec/test/folio-submit-balance-predicate.test.ts b/pkg/spec/test/folio-submit-balance-predicate.test.ts index 672fb5b..280f18a 100644 --- a/pkg/spec/test/folio-submit-balance-predicate.test.ts +++ b/pkg/spec/test/folio-submit-balance-predicate.test.ts @@ -520,3 +520,41 @@ test("a submit the runner could not confirm demands no balance move", () => { true, ); }); + +// relaunched: true is the runner saying its foreground guard restarted the app +// after this action. The tap landed, so the window still counts it, but nobody +// can promise the process lived long enough for the write to reach sqlite. A +// balance still sitting where it was is exactly what a healthy app looks like +// across a relaunch, and demanding the typed amount of movement convicts it for +// the runner's own restart. +test("a submit the runner relaunched across demands no balance move", () => { + assert.equal( + submitChangesBalanceByTypedAmount({ + route: "home", + lastAction: { kind: "Tap", on: submitOn, applied: true, relaunched: true }, + submitsInWindow: 1, + typedAmount: 500, + prevTotalBalance: 1000, + currTotalBalance: 1000, + }), + true, + ); +}); + +// The guard must not become a way of switching the property off. No relaunch +// reported is the ordinary case, and web and iOS cannot report one at all. +test("no relaunch reported still convicts a double submit", () => { + for (const relaunched of [null, undefined]) { + assert.equal( + submitChangesBalanceByTypedAmount({ + route: "home", + lastAction: { kind: "DoubleTap", on: submitOn, applied: true, relaunched }, + submitsInWindow: 1, + typedAmount: 500, + prevTotalBalance: 1000, + currTotalBalance: 2000, + }), + false, + ); + } +}); diff --git a/pkg/spec/test/folio-submit-window.test.ts b/pkg/spec/test/folio-submit-window.test.ts index 6e19281..4774165 100644 --- a/pkg/spec/test/folio-submit-window.test.ts +++ b/pkg/spec/test/folio-submit-window.test.ts @@ -67,6 +67,96 @@ test("a second submit with no Home reading between them counts two", () => { ); }); +// The window is a budget: an upper bound on the transactions the interval could +// hold. A tap the app's own parser must have refused spends none of it, and on +// android it does not even reach the parser, because TxnSubmit is +// clickable(enabled = amount.isNotBlank()). Measured over four recorded android +// runs, 19, 11, 25 and 25 of 35, 26, 42 and 42 submit taps landed on the +// transaction screen with the amount field empty, so more than half the budget +// was being spent on taps that cannot commit anything. +test("a submit the app must have refused does not spend the window's budget", () => { + for (const amountText of ["", " ", "0", "0.00", "00", "5.", "abc"]) { + assert.deepEqual( + countSubmitsInWindow({ + previousCount: 0, + lastAction: { kind: "Tap", on: submitOn }, + amountText, + fresh: false, + }), + { reported: 0, next: 0 }, + `amount ${JSON.stringify(amountText)} was counted as a possible commit`, + ); + } +}); + +// The field as the landing frame shows it, which is the form state the tap read: +// nothing between the two changes it. Anywhere but the transaction screen there +// is no field to read, and unknown has to count. +test("an amount that could commit, or that nobody could read, spends the budget", () => { + for (const amountText of ["5", "0.01", "1,000", "999999999999999999999", undefined]) { + assert.deepEqual( + countSubmitsInWindow({ + previousCount: 0, + lastAction: { kind: "Tap", on: submitOn }, + amountText, + fresh: false, + }), + { reported: 1, next: 1 }, + `amount ${JSON.stringify(amountText)} was dropped from the budget`, + ); + } +}); + +// The one thing that can put a different form state on screen than the one the +// tap read: the runner restarting the app, which the tap survives and the typed +// amount does not. The field a fresh process draws is empty whatever was +// submitted, so it proves nothing and the submit keeps its place in the budget. +test("a submit across a relaunch spends the budget whatever the field shows", () => { + assert.deepEqual( + countSubmitsInWindow({ + previousCount: 0, + lastAction: { kind: "Tap", on: submitOn, applied: true, relaunched: true }, + amountText: "", + fresh: false, + }), + { reported: 1, next: 1 }, + ); +}); + +// What the budget costs the counting invariant, in the shape of the iOS run in +// #78: a stretch of the walk that never went Home, most of it taps on a submit +// button with nothing typed into the form, and one double tap that committed +// twice. Counting the refused taps hands the app five transactions of slack it +// never used, and two rows against six actions is no violation. +test("refused submits used to hide a double submit behind their own budget", () => { + const frames = [ + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: "", lastAction: { kind: "Tap", on: submitOn } }, + { amountText: undefined, lastAction: { kind: "DoubleTap", on: submitOn } }, + ]; + let budget = 0; + for (const frame of frames) { + budget = countSubmitsInWindow({ + previousCount: budget, + lastAction: frame.lastAction, + amountText: frame.amountText, + fresh: false, + }).next; + } + assert.equal(budget, 1); + assert.equal( + committedTransactionsExceedSubmits({ + countsBefore: { Checking: 3 }, + countsAfter: { Checking: 5 }, + submitsInWindow: budget, + }), + true, + ); +}); + // The two traces the freshness rule exists to tell apart, driven step by step // through the same pair of carriers the spec holds. function run(steps: { route: string | null; totalText?: string; lastAction: unknown }[]) { diff --git a/pkg/spec/test/web-dom-harness.ts b/pkg/spec/test/web-dom-harness.ts index cc0e671..f35a094 100644 --- a/pkg/spec/test/web-dom-harness.ts +++ b/pkg/spec/test/web-dom-harness.ts @@ -1,8 +1,21 @@ -// A minimal stand-in for the DOM surface the web host reads, shared by the web -// runtime's own tests and the cross-host eligibility test. The host asks the -// document for three things -- every element, the tappable set, the editable set -// -- and reads geometry, `disabled` and the scroll extents off each element, so -// that is all a fake has to answer. +// A small DOM the web runtime can be driven over, shared by the web runtime's +// own tests and the cross-host eligibility test. +// +// It is a fake, but the structure is real: elements nest, a host owns a shadow +// root, and querySelectorAll WALKS the tree and stops at a shadow boundary +// exactly as the browser's does. That is what makes the shadow descent in +// deepQueryAll and expandShadowContent (src/web-runtime.ts) observable here at +// all; the previous harness answered three fixed selectors from a flat list, so +// deleting either descent changed no test result. +// +// What it fabricates is layout: getBoundingClientRect, scrollHeight and +// clientHeight are handed over from the spec. No headless DOM computes those, +// and they are precisely the facts collectTargets reads, so a real DOM +// implementation would have to be stubbed for them anyway. +// +// An unsupported selector throws rather than matching nothing, so a test whose +// selector this cannot parse fails loudly instead of quietly asserting over an +// empty list. import { __testing__ } from "../src/web-runtime.ts"; @@ -23,23 +36,45 @@ export interface FakeElementSpec { label?: string; alt?: string; title?: string; - // clickable/editable place the element in the selector sets the host queries; - // the fake answers those queries directly rather than matching CSS. + // text is what an ax element handle reports as `text`, the same field the + // goja host reads off a hierarchy node, so a test can name WHICH of two + // same-id elements a lookup resolved to. + text?: string; + attrs?: Record; + // clickable/editable place the element in the two fact sets the host queries + // by selector. They are answered from these flags rather than by matching + // their CSS: the cross-host golden (fixtures/host-parity-golden.json, built + // row for row in internal/verifier/host_parity_test.go) pins fact + // combinations no CSS can produce, such as an that is editable and + // not clickable. A test states the facts there; this harness reports them. clickable?: boolean; editable?: boolean; disabled?: boolean; // overflows makes the element's content taller than its box, which is how the // host decides an element is scrollable. overflows?: boolean; + children?: FakeElementSpec[]; + shadow?: FakeElementSpec[]; } -export interface FakeElement extends FakeElementSpec { +export interface FakeRoot { + children: FakeElement[]; + querySelectorAll(selector: string): FakeElement[]; +} + +export interface FakeElement extends Omit { tagName: string; type: string; isContentEditable: boolean; id: string; + className: string; + textContent: string; dataset: Record; + parentElement: FakeElement | null; + children: FakeElement[]; + shadowRoot: FakeRoot | null; getAttribute(name: string): string | null; + querySelectorAll(selector: string): FakeElement[]; scrollHeight: number; clientHeight: number; scrollWidth: number; @@ -56,15 +91,26 @@ export interface FakeElement extends FakeElementSpec { export function fakeElement(spec: FakeElementSpec): FakeElement { const editable = spec.editable ?? false; - return { + const attributes: Record = { ...spec.attrs }; + if (spec.id !== undefined) attributes.id = spec.id; + if (spec.testid !== undefined) attributes["data-testid"] = spec.testid; + if (spec.label !== undefined) attributes["aria-label"] = spec.label; + if (spec.alt !== undefined) attributes.alt = spec.alt; + if (spec.title !== undefined) attributes.title = spec.title; + const element: FakeElement = { ...spec, tagName: spec.tag.toUpperCase(), type: spec.tag === "input" ? "text" : "", isContentEditable: editable && spec.tag !== "input" && spec.tag !== "textarea", id: spec.id ?? "", + className: attributes.class ?? "", + textContent: spec.text ?? "", dataset: { testid: spec.testid }, - getAttribute: (name: string) => - ({ "aria-label": spec.label, alt: spec.alt, title: spec.title })[name] ?? null, + parentElement: null, + children: (spec.children ?? []).map(fakeElement), + shadowRoot: null, + getAttribute: (name: string) => attributes[name] ?? null, + querySelectorAll: (selector: string) => queryScope(element, selector), scrollHeight: spec.overflows ? spec.height * 2 : spec.height, clientHeight: spec.height, scrollWidth: spec.width, @@ -78,27 +124,170 @@ export function fakeElement(spec: FakeElementSpec): FakeElement { bottom: spec.y + spec.height, }), }; + for (const child of element.children) child.parentElement = element; + if (spec.shadow) element.shadowRoot = fakeRoot(spec.shadow.map(fakeElement)); + return element; } -// withFakeDocument installs a document answering the host's three queries over -// `elements`, resets the host's per-tick cache, and restores the real document -// afterwards. +// A shadow root's children have no parentElement, as in a real DOM, so a +// descendant selector cannot reach across the boundary from either side. +function fakeRoot(children: FakeElement[]): FakeRoot { + const root: FakeRoot = { + children, + querySelectorAll: (selector: string) => queryScope(root, selector), + }; + return root; +} + +function queryScope(scope: { children: FakeElement[] }, selector: string): FakeElement[] { + const found: FakeElement[] = []; + const walk = (nodes: FakeElement[]): void => { + for (const node of nodes) { + if (matchesQuery(node, selector)) found.push(node); + walk(node.children); + } + }; + walk(scope.children); + return found; +} + +function matchesQuery(element: FakeElement, selector: string): boolean { + if (selector === TAPPABLE_SELECTOR) return element.clickable === true; + if (selector === EDITABLE_SELECTOR) return element.editable === true; + return matchesSelectorList(element, selector); +} + +function matchesSelectorList(element: FakeElement, selector: string): boolean { + return splitTopLevel(selector, ",").some((complex) => matchesComplex(element, complex)); +} + +function matchesComplex(element: FakeElement, complex: string): boolean { + const compounds = splitTopLevel(complex, " "); + const subject = compounds.pop(); + if (subject === undefined) return false; + if (!matchesCompound(element, subject)) return false; + let ancestor = element.parentElement; + for (const compound of compounds.reverse()) { + while (ancestor && !matchesCompound(ancestor, compound)) ancestor = ancestor.parentElement; + if (!ancestor) return false; + ancestor = ancestor.parentElement; + } + return true; +} + +const TAG_NAME = /^[a-zA-Z][a-zA-Z0-9-]*/; +const ATTRIBUTE = /^([a-zA-Z][\w-]*)(?:([~^]?)=(.+))?$/; + +function matchesCompound(element: FakeElement, compound: string): boolean { + let rest = compound; + while (rest.length > 0) { + if (rest.startsWith("*")) { + rest = rest.slice(1); + continue; + } + if (rest.startsWith("[")) { + const end = closingIndex(rest, "[", "]"); + if (!matchesAttribute(element, rest.slice(1, end))) return false; + rest = rest.slice(end + 1); + continue; + } + if (rest.startsWith(":is(") || rest.startsWith(":not(")) { + const end = closingIndex(rest, "(", ")"); + const inner = rest.slice(rest.indexOf("(") + 1, end); + const anyMatched = splitTopLevel(inner, ",").some((part) => + matchesSelectorList(element, part), + ); + if (rest.startsWith(":is(") ? !anyMatched : anyMatched) return false; + rest = rest.slice(end + 1); + continue; + } + const tag = TAG_NAME.exec(rest); + if (!tag) throw new Error(`web-dom-harness cannot parse selector ${JSON.stringify(compound)}`); + if (element.tagName !== tag[0].toUpperCase()) return false; + rest = rest.slice(tag[0].length); + } + return true; +} + +function matchesAttribute(element: FakeElement, body: string): boolean { + const parsed = ATTRIBUTE.exec(body); + if (!parsed) throw new Error(`web-dom-harness cannot parse attribute [${body}]`); + const [, name, operator, quoted] = parsed; + const actual = element.getAttribute(name!); + if (actual === null) return false; + if (quoted === undefined) return true; + const value = unescapeCss(quoted.replace(/^"(.*)"$/, "$1").replace(/^'(.*)'$/, "$1")); + if (operator === "~") return actual.split(/\s+/).includes(value); + if (operator === "^") return actual.startsWith(value); + return actual === value; +} + +// Selector values reach the harness escaped by CSS.escape, so `[id="1a"]` +// arrives as `[id="\31 a"]` and comparing it raw would never match. +function unescapeCss(value: string): string { + return value.replace(/\\([0-9a-fA-F]{1,6}) ?|\\(.)/g, (_, hex: string, literal: string) => + hex ? String.fromCodePoint(parseInt(hex, 16)) : literal, + ); +} + +function closingIndex(input: string, open: string, close: string): number { + let depth = 0; + let quote = ""; + for (let index = input.indexOf(open); index < input.length; index++) { + const character = input[index]!; + if (quote) { + if (character === quote) quote = ""; + continue; + } + if (character === '"' || character === "'") quote = character; + else if (character === open) depth++; + else if (character === close && --depth === 0) return index; + } + throw new Error(`web-dom-harness cannot parse selector ${JSON.stringify(input)}`); +} + +function splitTopLevel(input: string, separator: string): string[] { + const parts: string[] = []; + let current = ""; + let depth = 0; + let quote = ""; + for (const character of input) { + if (quote) { + current += character; + if (character === quote) quote = ""; + continue; + } + if (character === '"' || character === "'") quote = character; + else if (character === "(" || character === "[") depth++; + else if (character === ")" || character === "]") depth--; + else if (depth === 0 && (character === separator || (separator === " " && /\s/.test(character)))) { + parts.push(current); + current = ""; + continue; + } + current += character; + } + parts.push(current); + return parts.map((part) => part.trim()).filter((part) => part.length > 0); +} + +// withFakeDocument installs a document whose top-level children are `elements`, +// resets the host's per-tick cache, and restores the real globals afterwards. +// window goes in alongside document because buildState reads both, so an +// extractor reaching state.ax needs it. export function withFakeDocument(elements: FakeElement[], run: () => void): void { const global = globalThis as Record; - const original = global.document; - const answers: Record = { - "*": elements, - [TAPPABLE_SELECTOR]: elements.filter((element) => element.clickable), - [EDITABLE_SELECTOR]: elements.filter((element) => element.editable), - }; - global.document = { - querySelectorAll: (selector: string) => answers[selector] ?? [], - }; + const originalDocument = global.document; + const originalWindow = global.window; + const document: FakeRoot = fakeRoot(elements); + global.document = document; + global.window = {}; __testing__.resetTargetCache(); try { run(); } finally { __testing__.resetTargetCache(); - global.document = original; + global.document = originalDocument; + global.window = originalWindow; } } diff --git a/pkg/spec/test/web-runtime.test.ts b/pkg/spec/test/web-runtime.test.ts index a5ac7e6..105975b 100644 --- a/pkg/spec/test/web-runtime.test.ts +++ b/pkg/spec/test/web-runtime.test.ts @@ -84,6 +84,7 @@ test("installRuntime defined the host-invoked globals", () => { }); const { fakeElement, withFakeDocument } = await import("./web-dom-harness.ts"); +type FakeElementSpec = Parameters[0]; // The host reports facts and never routes verbs: which of these a verb may act // on is decided by the shared rule in src/targets.ts, exercised across both @@ -175,6 +176,55 @@ test("queryTargets leaves duplicated identities unnamed", () => { }); }); +// The enumeration ORDER is the parity contract. buildTree in +// internal/driver/chrome/driver.go emits a host's shadow children before its +// light ones, and TestHierarchy_DerivesTheSameFactsAsTheWebRuntime compares the +// two enumerations position by position. +test("queryTargets splices shadow content in before the host's light children", () => { + const page = fakeElement({ + tag: "div", x: 0, y: 0, width: 400, height: 800, id: "page", + children: [ + { + tag: "div", x: 0, y: 0, width: 400, height: 100, id: "mount", + shadow: [ + { tag: "button", x: 0, y: 0, width: 40, height: 20, id: "shadow-save", clickable: true }, + ], + children: [{ tag: "div", x: 0, y: 20, width: 40, height: 20, id: "mount-light-child" }], + }, + { tag: "div", x: 0, y: 100, width: 400, height: 100, id: "after" }, + ], + }); + withFakeDocument([page], () => { + assert.deepEqual( + host.queryTargets().map((target) => target.selector), + ["id:page", "id:mount", "id:shadow-save", "id:mount-light-child", "id:after"], + ); + }); +}); + +// The tappable set is resolved by selector, and querySelectorAll stops dead at +// a shadow boundary, so a control inside a shadow root carries the clickable +// fact only if the selector sweep descends. A Compose for Web app keeps every +// control it has on the far side of one boundary. +test("queryTargets reports a shadow-hosted control as clickable", () => { + const mount = fakeElement({ + tag: "div", x: 0, y: 0, width: 400, height: 100, id: "mount", + shadow: [ + { tag: "button", x: 0, y: 0, width: 40, height: 20, id: "shadow-save", clickable: true }, + { tag: "input", x: 0, y: 20, width: 40, height: 20, id: "shadow-amount", editable: true }, + ], + }); + withFakeDocument([mount], () => { + const targets = host.queryTargets(); + assert.deepEqual( + targets.map((target) => target.selector), + ["id:mount", "id:shadow-save", "id:shadow-amount"], + ); + assert.equal(targets[1]!.clickable, true); + assert.equal(targets[2]!.editable, true); + }); +}); + test("queryTargets caches within a tick until reset", () => { const button = fakeElement({ tag: "button", x: 0, y: 0, width: 10, height: 10, clickable: true }); withFakeDocument([button], () => { @@ -473,28 +523,23 @@ test("selectorTag renders the selector shapes the goja host renders", () => { // accounts/totalBalance extractors (findAll([{HomeScreen}, {AccountCard}])) // were empty on every web step and the properties over them checked nothing. test("ax.findAll resolves a selector path segment by segment", () => { - const rect = { left: 0, top: 0, right: 10, bottom: 10, width: 10, height: 10 }; - const node = (id: string, answers: Record = {}) => ({ - id, - tagName: "DIV", - className: "", - textContent: id, - dataset: {}, - getAttribute: () => null, - getBoundingClientRect: () => rect, - querySelectorAll: (selector: string) => answers[selector] ?? [], + const card = (id: string, y: number): FakeElementSpec => ({ + tag: "div", x: 0, y, width: 10, height: 10, testid: "AccountCard", text: id, + }); + // The stray card is outside HomeScreen, so a document-wide sweep for the + // second segment picks it up and the scoping assertion below fails. + const page = fakeElement({ + tag: "div", x: 0, y: 0, width: 100, height: 100, + children: [ + { + tag: "div", x: 0, y: 0, width: 100, height: 50, testid: "HomeScreen", + children: [card("first", 0), card("second", 10)], + }, + card("stray", 60), + ], }); - const cardCss = `:is([data-testid="AccountCard"], [id="AccountCard"])`; - const screenCss = `:is([data-testid="HomeScreen"], [id="HomeScreen"])`; - const cards = [node("first"), node("second")]; - const home = node("HomeScreen", { [cardCss]: cards }); - const g = globalThis as Record; - const originalDocument = g.document; - const originalWindow = g.window; - g.document = { querySelectorAll: (selector: string) => (selector === screenCss ? [home] : []) }; - g.window = {}; - try { + withFakeDocument([page], () => { __testing__.extractors.length = 0; __testing__.runtime.extract((state) => { const ax = (state as { ax: { findAll(s: unknown): Record[] } }).ax; @@ -506,30 +551,14 @@ test("ax.findAll resolves a selector path segment by segment", () => { // Scoped to the head match: the cards come from the HomeScreen node, not // from a document-wide sweep for AccountCard. assert.deepEqual(readingOf(values, 0), ["first", "second"]); - } finally { - g.document = originalDocument; - g.window = originalWindow; - } + }); }); test("ax.find and ax.findAll label the element with its selector", () => { - const rect = { left: 0, top: 0, right: 10, bottom: 10, width: 10, height: 10 }; - const submit = { - id: "TxnSubmit", - tagName: "DIV", - className: "", - textContent: "Submit", - dataset: {}, - getAttribute: () => null, - getBoundingClientRect: () => rect, - }; - const matches = `:is([data-testid="TxnSubmit"], [id="TxnSubmit"])`; - const g = globalThis as Record; - const originalDocument = g.document; - const originalWindow = g.window; - g.document = { querySelectorAll: (selector: string) => (selector === matches ? [submit] : []) }; - g.window = {}; - try { + const submit = fakeElement({ + tag: "div", x: 0, y: 0, width: 10, height: 10, id: "TxnSubmit", text: "Submit", + }); + withFakeDocument([submit], () => { __testing__.extractors.length = 0; __testing__.runtime.extract((state) => { const ax = (state as { ax: { find(s: unknown): Record | undefined } }).ax; @@ -546,8 +575,63 @@ test("ax.find and ax.findAll label the element with its selector", () => { // reference would hand the array INDEX to the runtime as the selector. const all = readingOf(values, 1) as Record[]; assert.equal(all[0]!.__sanderlingSelector, "testTag:TxnSubmit"); - } finally { - g.document = originalDocument; - g.window = originalWindow; - } + }); +}); + +// One page, one selector, two hosts. The goja host resolves a selector against +// the hierarchy dump, whose buildTree (internal/driver/chrome/driver.go) emits +// a host's shadow children BEFORE its light ones, so a pre-order search there +// reaches a shadow-hosted match first. deepQueryAll swept the whole light DOM +// first and only then descended, so this page answered find({id:"x"}) with the +// light node in V8 and the shadow node in goja, and on web V8's answer is the +// one that reaches the properties. +test("ax.find resolves the shadow-hosted match the hierarchy dump reaches first", () => { + const page = fakeElement({ + tag: "div", x: 0, y: 0, width: 400, height: 800, id: "page", + children: [ + { + tag: "div", x: 0, y: 0, width: 400, height: 100, id: "mount", + shadow: [{ tag: "span", x: 0, y: 0, width: 40, height: 20, id: "x", text: "shadow" }], + }, + { tag: "span", x: 0, y: 100, width: 40, height: 20, id: "x", text: "light" }, + ], + }); + withFakeDocument([page], () => { + __testing__.extractors.length = 0; + __testing__.runtime.extract((state) => { + const ax = (state as { ax: { find(s: unknown): Record | undefined } }).ax; + return ax.find("id:x")?.text; + }); + __testing__.runtime.extract((state) => { + const ax = (state as { ax: { findAll(s: unknown): Record[] } }).ax; + return ax.findAll("id:x").map((element) => element.text); + }); + const values = __testing__.evaluateExtractors(); + assert.equal(readingOf(values, 0), "shadow"); + assert.deepEqual(readingOf(values, 1), ["shadow", "light"]); + }); +}); + +// A nested undefined is the one reading shape the two hosts do NOT encode +// alike, and this pins the split instead of hiding it. JSON has no undefined, +// so the key goes with the value here; goja marshals the same member as null, +// and it cannot do otherwise, because an exported goja object reports undefined +// and null identically, so dropping those keys there would drop the genuine +// nulls this host keeps. Carrying the member across would take a wire format +// that can express undefined. +// +// What both hosts DO agree on is the member's value: reading it answers +// undefined either way, and that is the guarantee a property may rely on. Key +// presence (`in`, Object.keys) is not. +// TestExtractorEncoding_NestedUndefinedIsNotOnTheWire in +// internal/verifier/extractor_encoding_test.go pins the other half. +test("a nested undefined leaves the page as a dropped key, a nested null does not", () => { + __testing__.extractors.length = 0; + __testing__.runtime.extract(() => ({ absent: undefined, empty: null, present: 1 })); + let wire = ""; + withState(() => { + // Exactly what extractorScript in internal/driver/chrome/driver.go sends. + wire = JSON.stringify(__testing__.evaluateExtractors()); + }); + assert.equal(wire, `{"0":{"value":{"empty":null,"present":1}}}`); }); diff --git a/replay-ui/sanderling/spec.ts b/replay-ui/sanderling/spec.ts index 0044eaa..75d19d6 100644 --- a/replay-ui/sanderling/spec.ts +++ b/replay-ui/sanderling/spec.ts @@ -170,11 +170,44 @@ const switchATab = actions(() => { return tabs.length === 0 ? [] : [Tap({ on: from(tabs).generate() })]; }); +// badgeCountMatchesThePanel needs two readings on ONE step: the badge, which a +// tab strip renders only for a step that HAS a violation, and a violations +// panel to compare it against, which exists while the properties or violations +// tab is selected. Undirected actions put both on the same step 0 times in the +// 80 of the first dogfood run: the property was reachable in principle and +// judged nothing in practice. +// +// Both halves have to be aimed at. Aiming at the step alone just moved the +// misses to the other side, 0 judged either way. So this selects a step the +// list marks as violating, and once standing on one, opens a panel if none is +// up. It opens the AFTER panel's, because the before panel's screenshot is what +// screenshotShowsTheSelectedStep reads and covering that up trades one +// property's evidence for another's. +const violatingRows = extract("violatingRows", (s) => + s.ax.findAll({ "data-testid": "step-row" }).filter((row) => dataOf(row, "violations") === "true"), +); +const afterPropertiesTabs = extract("afterPropertiesTabs", (s) => + s.ax + .findAll([{ "data-testid": "state-after" }, { "data-testid": "tab" }]) + .filter((tab) => dataOf(tab, "tabId") === "properties"), +); + +const showAViolatingStepWithItsPanel = actions(() => { + const rows = violatingRows.current; + if (!rows.some((row) => dataOf(row, "active") === "true")) { + return rows.length === 0 ? [] : [Tap({ on: from(rows).generate() })]; + } + if (violationPanelCounts.current.length > 0) return []; + const tabs = afterPropertiesTabs.current; + return tabs.length === 0 ? [] : [Tap({ on: from(tabs).generate() })]; +}); + // defaultActions carries the rest: the jump-to-violation button, the theme // toggle, the link back to the run list, and the scrolling. export const actionsRoot = weighted( [30, selectAStep], [20, navigateByKeyboard], [25, switchATab], + [20, showAViolatingStepWithItsPanel], [25, defaultActions], ); diff --git a/replay-ui/src/components/Tabs.css b/replay-ui/src/components/Tabs.css index fea0f76..744d9b9 100644 --- a/replay-ui/src/components/Tabs.css +++ b/replay-ui/src/components/Tabs.css @@ -6,8 +6,15 @@ font-family: var(--font-mono); } +/* Wrapping is what keeps the last tabs reachable. In a narrow column the five + tabs are wider than the panel, and the overflow scrolls .detail-panel-body, + which carries that tab's own panel out of the column with it. In a 756px + viewport (what a headless run gets) Properties and Violations then sit under + the neighbouring panel, where neither a person nor the fuzzer can click + them. */ .tabs-header { display: flex; + flex-wrap: wrap; gap: 0; border-bottom: 1px solid var(--border); flex: 0 0 auto; diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt index d987490..3d69e2f 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt @@ -185,7 +185,8 @@ private val ROUTE_TAG_KEYS = setOf( "accessibilityIdentifier", ) -private val jsonMapper = com.fasterxml.jackson.module.kotlin.jacksonObjectMapper() +private val jsonMapper = + com.fasterxml.jackson.module.kotlin.jacksonObjectMapper() // countRouteScreens counts DISTINCT route-level destination tags, not the nodes // carrying them. A screen that nests a node repeating its own route id puts two @@ -328,11 +329,12 @@ internal fun readLogcat( arguments.add(since) } return try { - val process = ProcessBuilder( - adbCmd(serial) + arguments, - ).redirectErrorStream(false).start() - val output = process.inputStream.bufferedReader().readText() - process.waitFor() + val command = adbCmd(serial) + arguments + val output = readProcessOutput( + ProcessBuilder(command).redirectErrorStream(false).start(), + ADB_OUTPUT_TIMEOUT_MILLIS, + describe = { command.joinToString(" ") }, + ) StubDriverBackend.parseLogcatOutput(output) } catch (cause: Exception) { println("adb logcat failed: $cause") @@ -358,13 +360,76 @@ internal fun readProcMetrics(serial: String?, bundleId: String): MetricsSample { private fun adbCmd(serial: String?): List = if (serial == null) listOf("adb") else listOf("adb", "-s", serial) +// ADB_OUTPUT_TIMEOUT_MILLIS bounds the diagnostic adb reads: dumpsys, logcat, +// `settings get`, /proc stats. None of them is the driver's data path, so the +// bound wants to be generous enough that it cannot fire on a link that works, +// and it is: a hierarchy fetch, far heavier than any of these, measures at a +// 76ms median and a 168ms p90 over the same remote adb link. What it caps is +// the other end, where a wedged adb once held a step ~100s. +internal const val ADB_OUTPUT_TIMEOUT_MILLIS = 10_000L + +private val adbReaders: java.util.concurrent.ExecutorService = + java.util.concurrent.Executors.newCachedThreadPool { runnable -> + Thread(runnable, "adb-output").apply { isDaemon = true } + } + +// readProcessOutput returns a process's stdout, or "" when it does not arrive +// inside timeoutMillis. +// +// The bound belongs on the READ, not on waitFor. readText ends at EOF, and a +// wedged adb neither writes nor exits, so EOF never comes and a waitFor with a +// timeout after it is a line that never runs. Waiting first and reading after +// is worse still: a process with more to say than a pipe buffer holds, which +// logcat and dumpsys both are, blocks writing while the waiter waits for it to +// finish, and neither ever moves. +// +// So the read runs on a daemon thread and killing the process is what releases +// it: destroy closes the pipe, the reader sees EOF, the thread ends. Returning +// "" hands every caller the answer it already treats as "adb said nothing", +// which is the safe direction for all of them. +internal fun readProcessOutput( + process: Process, + timeoutMillis: Long, + describe: () -> String, + log: (String) -> Unit = { System.err.println(it) }, +): String { + val reader = adbReaders.submit { + process.inputStream.bufferedReader().readText() + } + return try { + val output = reader.get( + timeoutMillis, + java.util.concurrent.TimeUnit.MILLISECONDS, + ) + if (!process.waitFor( + timeoutMillis, + java.util.concurrent.TimeUnit.MILLISECONDS, + ) + ) { + process.destroyForcibly() + } + output + } catch (cause: java.util.concurrent.TimeoutException) { + process.destroyForcibly() + reader.cancel(true) + log( + "warn: ${describe()} gave nothing in ${timeoutMillis}ms; " + + "killed it and read no answer", + ) + "" + } catch (cause: Exception) { + process.destroyForcibly() + "" + } +} + private fun adbOutput(serial: String?, arguments: List): String = try { - val process = ProcessBuilder( - adbCmd(serial) + arguments, - ).redirectErrorStream(false).start() - val output = process.inputStream.bufferedReader().readText() - process.waitFor() - output + val command = adbCmd(serial) + arguments + readProcessOutput( + ProcessBuilder(command).redirectErrorStream(false).start(), + ADB_OUTPUT_TIMEOUT_MILLIS, + describe = { command.joinToString(" ") }, + ) } catch (cause: Exception) { "" } @@ -473,8 +538,14 @@ class StubDriverBackend( companion object { private const val IDLE_POLL_INTERVAL_MILLIS = 50L + // A count we could not read is not a count of zero. Defaulting it to + // zero made an unreadable dumpsys mean "nothing is animating, go + // ahead", which is the one answer the caller cannot check: it breaks + // out of the settle and snapshots whatever frame is on screen. Unknown + // keeps it waiting instead, inside the deadline waitForIdle already + // holds, and it agrees with the probe's own exception path. internal fun isAnimationCountIdle(grepOutput: String): Boolean = - (grepOutput.trim().toIntOrNull() ?: 0) == 0 + grepOutput.trim().toIntOrNull() == 0 internal fun parseResolvedActivity( bundleId: String, @@ -496,8 +567,8 @@ class StubDriverBackend( when (ch) { ' ' -> sb.append("%s") - '\\', '"', '\'', '&', '|', ';', '<', '>', '(', ')', '*', '?', - '$', '`', '[', ']', '{', '}', '~', '#', + '\\', '"', '\'', '&', '|', ';', '<', '>', '(', ')', '*', + '?', '$', '`', '[', ']', '{', '}', '~', '#', -> sb.append( '\\', ).append(ch) @@ -779,6 +850,210 @@ internal fun typeChunks( return typed } +// dismissSoftKeyboard closes an open IME, and issues nothing when none is open. +// +// The keyboard is its own window over the bottom of the app, and the hierarchy +// carries only what is visible to the user, so every app node under it is +// absent from the tree the picker enumerates targets from. Typing raises it, so +// an IME left open hides a form's submit control for as long as the fuzzer +// keeps typing into that form, which is a state it cannot type its way out of. +// +// The mInputShown guard is load-bearing rather than an optimisation: BACK is +// what closes an open IME, and BACK with no IME open navigates out of the +// screen, so an unguarded dismissal would make every InputText a back press. +// +// The flag trails the BACK it answers for, by about 0.6s on API 36, and a +// second dismissal inside that window reads the stale true and back-presses an +// IME that has already gone. What keeps that unreachable is the caller: one +// dismissal per inputText, and the runner focuses the field with a tap before +// every InputText, which raises the IME again long before this probe runs. A +// caller that dismissed twice in a row, or typed without focusing first, would +// lose that margin. +// +// treeWithoutKeyboard closes a keyboard too, on the snapshot path, and does not +// cost this one its margin: it reads no flag, and the two are a waitForIdle and +// a hierarchy fetch apart, several times the window in which this one is stale. +internal fun dismissSoftKeyboard(shell: (String) -> String) { + if (!shell("dumpsys input_method").contains("mInputShown=true")) return + shell("input keyevent 4") +} + +// KEYBOARD_DISMISS_READS bounds the re-reads a snapshot spends waiting for the +// IME window to leave the tree after BACK. The window leaves over an +// animation, so the first read back can still carry it. A hierarchy read +// measures at a 76ms median and a 168ms p90 on the API 34 emulator, so with +// the interval these four reads watch most of a second: several retractions +// over, without turning a keyboard the app keeps re-raising into a wait with +// no end. +internal const val KEYBOARD_DISMISS_READS = 4 +internal const val KEYBOARD_DISMISS_INTERVAL_MILLIS = 100L + +// imePackageOf takes the package half of an input-method component id +// ("pkg/.Service"), the form both `settings get secure default_input_method` +// and dumpsys' mCurMethodId use. Anything that is not a package name reads as +// "no IME known", which disables the dismissal rather than guessing. +internal fun imePackageOf(component: String): String? = + component.trim().substringBefore('/') + .takeIf { it.isNotEmpty() && it.contains('.') } + +// treeShowsIme reports whether the keyboard window is in the tree, by the view +// ids the IME's own resources give it ("pkg:id/name"). +internal fun treeShowsIme(treeJson: String, imePackage: String): Boolean = + treeJson.contains("$imePackage:id/") + +// treeWithoutKeyboard closes a keyboard standing in the snapshot and returns a +// tree read after it has gone, or the tree it was given when none is open. +// +// It belongs here, before the read the picker chooses from, rather than after +// the tap that raised the keyboard. Two reasons. The picker only ever sees +// snapshots, so a dismissal anywhere later leaves this step choosing between +// the handful of targets an open keyboard left in the tree, which is the +// budget the fuzzer was losing. And the state it has to judge is settled here: +// the action landed a waitForIdle ago, where straight after the tap the +// keyboard is still on its way up and nothing it could read would say so yet. +// +// The tree is also a better guard than mInputShown. BACK closes an open +// keyboard and navigates when none is open, so pressing it is only safe on a +// true reading; mInputShown trails the keyboard by up to 0.6s, while a tree +// carrying the IME's own view ids is the keyboard being on screen, read a +// moment ago. Once dismissed, the re-reads confirm it went rather than pressing +// BACK again, so a keyboard the app puts straight back costs re-reads and never +// a second back press. +internal fun treeWithoutKeyboard( + tree: String, + imePackage: String?, + dismiss: () -> Unit, + reread: () -> String, + sleep: (Long) -> Unit = { Thread.sleep(it) }, +): String { + if (imePackage == null || !treeShowsIme(tree, imePackage)) return tree + dismiss() + var current = tree + repeat(KEYBOARD_DISMISS_READS) { + sleep(KEYBOARD_DISMISS_INTERVAL_MILLIS) + current = reread() + if (!treeShowsIme(current, imePackage)) return current + } + return current +} + +// SELECT_ALL_COMMAND selects the focused field's whole content with +// CTRL+A (keycodes 113 and 29) and DELETE_KEY_COMMAND then deletes the +// selection (keycode 67). Two key events, whatever the field holds. +internal const val SELECT_ALL_COMMAND = "input keycombination 113 29" +internal const val DELETE_KEY_COMMAND = "input keyevent 67" + +// DELETE_BATCH_KEYS bounds how many deletes ride in one `input keyevent` +// invocation on the fallback path. `input` takes a list of keycodes, so the +// round trip is paid per batch rather than per character: measured 2.3 ms/char +// against the 29.6 ms/char of one round trip each. +internal const val DELETE_BATCH_KEYS = 200 + +internal fun deleteKeyCommands(count: Int, batch: Int): List { + if (count <= 0) return emptyList() + val size = batch.coerceAtLeast(1) + return (0 until count).chunked(size).map { chunk -> + chunk.joinToString(" ", prefix = "input keyevent ") { "67" } + } +} + +// focusedEditableTextLength reports how much text the focused text field +// holds, or null when the tree names no focused text field. Null is "cannot +// tell", which is not the same as empty and must not be read as it. +// +// The field is found by class, not by an "editable" attribute: maestro's tree +// carries no such attribute. Class also settles the trap an open keyboard +// sets, which is that the IME contributes a focused node of its own. That node +// holds no text, so taking the first focused node would read a field still +// holding 4096 characters as empty, and empty is the answer that stops the +// erase. +internal fun focusedEditableTextLength(treeJson: String): Int? { + if (treeJson.isBlank()) return null + return try { + focusedFieldLength(jsonMapper.readTree(treeJson)) + } catch (_: Exception) { + null + } +} + +private fun focusedFieldLength( + node: com.fasterxml.jackson.databind.JsonNode, +): Int? { + val attributes = node.get("attributes") + if (attributes != null && attributes.isObject && + attributes.get("focused")?.asText() == "true" && + attributes.get("class")?.asText().orEmpty().endsWith("EditText") + ) { + return attributes.get("text")?.asText().orEmpty().length + } + val children = node.get("children") ?: return null + if (!children.isArray) return null + for (child in children) focusedFieldLength(child)?.let { return it } + return null +} + +// eraseFocusedField clears the field the runner just tapped. +// +// maestro's eraseText sends one delete per character through its own +// instrumentation, which measured 29.6 ms/char on the API 34 emulator: the +// 4096-character string the corpus types cost ~121s to clear, a fifth of a 20 +// minute budget spent on one step. Selecting the content and deleting the +// selection costs the same two key events at any length, measured 0.15s to +// 1.16s for 4096 characters across API 34, 35 and 36. +// +// A fast erase that leaves characters behind would be far worse than a slow +// one, because the next InputText appends to the residue and nothing +// downstream detects it. So the result is read back off the tree, and a field +// that is not empty, or that the tree cannot report on at all, is finished off +// per character. Those deletes ride in batches, so even that path costs one +// round trip per batch rather than the one per character this replaces. +internal fun eraseFocusedField( + characterCount: Int, + shell: (String) -> Unit, + focusedTextLength: () -> Int?, +) { + if (characterCount <= 0) return + shell(SELECT_ALL_COMMAND) + shell(DELETE_KEY_COMMAND) + if (focusedTextLength() == 0) return + for (command in deleteKeyCommands(characterCount, DELETE_BATCH_KEYS)) { + shell(command) + } +} + +// typingOwner picks what the mid-type foreground guard holds later reads +// against. A dumpsys it could read names the resumed package, and that is the +// answer. +// +// A read that failed is the interesting case, and neither obvious answer is +// right. Passing null hands typeChunks "no owner", which switches the guard off +// altogether and lets the rest of the string spray into whatever holds the +// foreground: an unreadable probe must never read as focus being fine. But +// refusing to type is worse in practice. The failure is a degraded link, which +// lasts, so every InputText in the run becomes a no-op, the budget goes on +// typing nothing, and the run ends green having tested nothing. +// +// So it falls back to the bundle the run launched, which leaves the guard armed +// against the app the keystrokes were meant for. That reference is better than +// the resumed package anyway: a foreground already stolen before typing began +// reads as its own owner, and the guard then matches it happily chunk after +// chunk. +internal fun typingOwner( + dumpsys: String, + launchedBundleId: String?, + warn: (String) -> Unit, +): String? { + parseResumedPackage(dumpsys)?.let { return it } + warn( + "warn: could not read the foreground app; guarding typing with " + + ( + launchedBundleId?.let { "the launched bundle $it" } + ?: "nothing, no launch was recorded" + ), + ) + return launchedBundleId +} + // resumedActivityPackage matches a "package/activity" component, mirroring the // Go scope guard's regex so both read the same dumpsys wording. private val resumedActivityPackage = @@ -831,6 +1106,14 @@ internal fun retryOpen( class MaestroDriverBackend(private val serial: String?) : DriverBackend { private val dadb: dadb.Dadb = buildDadb(serial) + private val imePackage: String? by lazy { + imePackageOf( + runCatching { + dadb.shell("settings get secure default_input_method").allOutput + }.getOrDefault(""), + ) + } + // A fresh AndroidDriver per open attempt. Its gRPC channel is built once in // the constructor and permanently shut down by close(), so reopening a // closed instance would reuse a dead channel; rebuild it each try instead. @@ -847,6 +1130,9 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { } } + @Volatile + private var launchedBundleId: String? = null + override fun launch( bundleId: String, clearState: Boolean, @@ -854,6 +1140,7 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { ) { if (clearState) driver.clearAppState(bundleId) driver.launchApp(bundleId, env) + launchedBundleId = bundleId } override fun terminate(bundleId: String) = driver.stopApp(bundleId) @@ -885,6 +1172,11 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { } else { driver.inputText(text) } + // A probe that fails reads as "no IME open", which is the safe way to be + // wrong: it skips the dismissal rather than sending a stray BACK. + dismissSoftKeyboard { + runCatching { dadb.shell(it).allOutput }.getOrDefault("") + } } // typeShellSafe types shell-safe ASCII through adb `input text` in chunks, @@ -892,8 +1184,14 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { // started in has lost the foreground, the remaining keystrokes would spray // into whatever window stole it (the launcher search box, in practice), so // typing stops instead of leaking out of the app under test. + // + // typingOwner decides what "the app the type started in" means when the + // read that would name it fails: the launched bundle, so a link that cannot + // answer degrades the guard rather than switching it off. private fun typeShellSafe(text: String) { - val owner = foregroundPackage() + val owner = typingOwner(foregroundDumpsys(), launchedBundleId) { + System.err.println(it) + } val typed = typeChunks(chunkForInput(text, INPUT_CHUNK_CHARS), owner, { foregroundPackage() @@ -907,17 +1205,21 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { } } - // foregroundPackage returns the package of the top resumed activity, or null - // if it cannot be read. Used to detect mid-type focus escapes. - private fun foregroundPackage(): String? = parseResumedPackage( - adbOutput( - serial, - listOf("shell", "dumpsys", "activity", "activities"), - ), + private fun foregroundDumpsys(): String = adbOutput( + serial, + listOf("shell", "dumpsys", "activity", "activities"), ) - override fun eraseText(characterCount: Int) = - driver.eraseText(characterCount) + // foregroundPackage returns the package of the top resumed activity, or null + // if it cannot be read. Used to detect mid-type focus escapes. + private fun foregroundPackage(): String? = + parseResumedPackage(foregroundDumpsys()) + + override fun eraseText(characterCount: Int) = eraseFocusedField( + characterCount, + shell = { dadb.shell(it) }, + focusedTextLength = { focusedEditableTextLength(hierarchy()) }, + ) override fun swipe( fromX: Int, @@ -961,8 +1263,15 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { // read the snapshot was going to do anyway. That is what makes this // affordable, where the structural poll that used to run in waitForIdle was // not: it fetched the hierarchy ~4 more times on every mutating step. - override fun snapshot(): SnapshotSample = - SnapshotSample(awaitSettledTree { hierarchy() }, screenshot()) + override fun snapshot(): SnapshotSample { + val tree = treeWithoutKeyboard( + awaitSettledTree { hierarchy() }, + imePackage, + dismiss = { runCatching { dadb.shell("input keyevent 4") } }, + reread = { awaitSettledTree { hierarchy() } }, + ) + return SnapshotSample(tree, screenshot()) + } override fun waitForIdle(durationMillis: Long) { // waitForAppToSettle blocks on the View-system animation and maestro's @@ -1014,15 +1323,75 @@ internal fun dadbTargetFor(serial: String?): DadbTarget { } } +internal data class AdbServerEndpoint(val host: String, val port: Int) + +private const val ADB_SERVER_HOST = "localhost" +private const val ADB_SERVER_PORT = 5037 + +// adbServerEndpoint reads where the adb server listens the way the adb CLI +// reads it: ADB_SERVER_SOCKET ("tcp:host:port", or "tcp:port" for a server on +// this machine) outranks the older ANDROID_ADB_SERVER_ADDRESS / +// ANDROID_ADB_SERVER_PORT pair, and unset means the loopback default. +// +// A value it cannot read throws instead of falling back to loopback. The +// fallback is the dangerous answer: emulator serials are numbered per server, +// so a run aimed at a remote emulator-5554 would quietly drive whatever this +// machine calls emulator-5554 and report the results as the remote device's. +internal fun adbServerEndpoint( + env: (String) -> String? = System::getenv, +): AdbServerEndpoint { + val socket = env("ADB_SERVER_SOCKET")?.trim().orEmpty() + if (socket.isNotEmpty()) return parseAdbServerSocket(socket) + val host = env("ANDROID_ADB_SERVER_ADDRESS")?.trim().orEmpty() + val port = env("ANDROID_ADB_SERVER_PORT")?.trim().orEmpty() + return AdbServerEndpoint( + host.ifEmpty { ADB_SERVER_HOST }, + if (port.isEmpty()) { + ADB_SERVER_PORT + } else { + adbServerPort(port, "ANDROID_ADB_SERVER_PORT=\"$port\"") + }, + ) +} + +private fun parseAdbServerSocket(value: String): AdbServerEndpoint { + val named = "ADB_SERVER_SOCKET=\"$value\"" + val address = value.removePrefix("tcp:") + if (address == value) rejectAdbServerSocket(named) + val colon = address.lastIndexOf(':') + if (colon < 0) { + return AdbServerEndpoint( + ADB_SERVER_HOST, + adbServerPort(address, named), + ) + } + val host = address.substring(0, colon) + if (host.isEmpty()) rejectAdbServerSocket(named) + return AdbServerEndpoint( + host, + adbServerPort(address.substring(colon + 1), named), + ) +} + +private fun rejectAdbServerSocket(named: String): Nothing = + throw IllegalArgumentException("$named is not tcp:host:port") + +private fun adbServerPort(text: String, named: String): Int = + text.toIntOrNull()?.takeIf { it in 1..65535 } + ?: throw IllegalArgumentException("$named has no usable port") + private fun buildDadb(serial: String?): dadb.Dadb = when (val target = dadbTargetFor(serial)) { is DadbTarget.Tcp -> dadb.Dadb.create(target.host, target.port) - is DadbTarget.Server -> dadb.adbserver.AdbServer.createDadb( - "localhost", - 5037, - "host:transport:${target.serial}", - ) + is DadbTarget.Server -> { + val server = adbServerEndpoint() + dadb.adbserver.AdbServer.createDadb( + server.host, + server.port, + "host:transport:${target.serial}", + ) + } } internal fun findBoundsBySelector( @@ -1086,7 +1455,8 @@ internal fun pngHeight(bytes: ByteArray): Int { (bytes[22].toInt() and 0xFF shl 8) or (bytes[23].toInt() and 0xFF) } -internal const val IOS_XCTEST_RUNNER_BUNDLE_ID = "dev.mobile.maestro-driver-iosUITests.xctrunner" +internal const val IOS_XCTEST_RUNNER_BUNDLE_ID = + "dev.mobile.maestro-driver-iosUITests.xctrunner" // reapOrphanIosRunners kills XCTest runner sessions left over from a prior // run. A sidecar that died without its shutdown hook leaves its xcodebuild diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt index cfe11d0..647655b 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverService.kt @@ -31,7 +31,10 @@ class DriverService( private val launchedBundleId = AtomicReference(null) private val snapshotLock = Any() - override fun launch(request: LaunchRequest, responseObserver: StreamObserver) { + override fun launch( + request: LaunchRequest, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.launch(request.bundleId, request.clearState, request.envMap) launchedBundleId.set(request.bundleId) @@ -39,7 +42,10 @@ class DriverService( } } - override fun terminate(request: Empty, responseObserver: StreamObserver) { + override fun terminate( + request: Empty, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { launchedBundleId.get()?.let { backend.terminate(it) } launchedBundleId.set(null) @@ -54,42 +60,60 @@ class DriverService( } } - override fun doubleTap(request: Point, responseObserver: StreamObserver) { + override fun doubleTap( + request: Point, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.doubleTap(request.x, request.y) Empty.getDefaultInstance() } } - override fun longPress(request: Point, responseObserver: StreamObserver) { + override fun longPress( + request: Point, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.longPress(request.x, request.y) Empty.getDefaultInstance() } } - override fun tapSelector(request: Selector, responseObserver: StreamObserver) { + override fun tapSelector( + request: Selector, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.tapSelector(request.value) Empty.getDefaultInstance() } } - override fun inputText(request: Text, responseObserver: StreamObserver) { + override fun inputText( + request: Text, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.inputText(request.value) Empty.getDefaultInstance() } } - override fun eraseText(request: EraseTextRequest, responseObserver: StreamObserver) { + override fun eraseText( + request: EraseTextRequest, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.eraseText(request.characterCount) Empty.getDefaultInstance() } } - override fun swipe(request: SwipeRequest, responseObserver: StreamObserver) { + override fun swipe( + request: SwipeRequest, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { val from = request.from val to = request.to @@ -98,16 +122,25 @@ class DriverService( } } - override fun pressKey(request: PressKeyRequest, responseObserver: StreamObserver) { + override fun pressKey( + request: PressKeyRequest, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.pressKey(request.key) Empty.getDefaultInstance() } } - override fun recentLogs(request: RecentLogsRequest, responseObserver: StreamObserver) { + override fun recentLogs( + request: RecentLogsRequest, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { - val entries = backend.recentLogs(request.sinceUnixMillis, request.levelAtLeast) + val entries = backend.recentLogs( + request.sinceUnixMillis, + request.levelAtLeast, + ) val builder = LogEntries.newBuilder() for (entry in entries) { builder.addEntries( @@ -123,7 +156,10 @@ class DriverService( } } - override fun screenshot(request: Empty, responseObserver: StreamObserver) { + override fun screenshot( + request: Empty, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { val (png, width, height) = backend.screenshot() Image.newBuilder() @@ -134,18 +170,28 @@ class DriverService( } } - override fun hierarchy(request: Empty, responseObserver: StreamObserver) { + override fun hierarchy( + request: Empty, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { HierarchyJSON.newBuilder().setJson(backend.hierarchy()).build() } } - override fun snapshot(request: Empty, responseObserver: StreamObserver) { + override fun snapshot( + request: Empty, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { val sample = synchronized(snapshotLock) { backend.snapshot() } val (png, width, height) = sample.screenshot SnapshotResponse.newBuilder() - .setHierarchy(HierarchyJSON.newBuilder().setJson(sample.hierarchyJson).build()) + .setHierarchy( + HierarchyJSON.newBuilder() + .setJson(sample.hierarchyJson) + .build(), + ) .setScreenshot( Image.newBuilder() .setPng(ByteString.copyFrom(png)) @@ -157,14 +203,20 @@ class DriverService( } } - override fun waitForIdle(request: Duration, responseObserver: StreamObserver) { + override fun waitForIdle( + request: Duration, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { backend.waitForIdle(request.millis) Empty.getDefaultInstance() } } - override fun health(request: Empty, responseObserver: StreamObserver) { + override fun health( + request: Empty, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { HealthStatus.newBuilder() .setReady(backend.healthy()) @@ -174,9 +226,16 @@ class DriverService( } } - override fun metrics(request: MetricsRequest, responseObserver: StreamObserver) { + override fun metrics( + request: MetricsRequest, + responseObserver: StreamObserver, + ) { runRpc(responseObserver) { - val bundleId = if (request.bundleId.isNotEmpty()) request.bundleId else launchedBundleId.get().orEmpty() + val bundleId = if (request.bundleId.isNotEmpty()) { + request.bundleId + } else { + launchedBundleId.get().orEmpty() + } val sample = backend.metrics(bundleId) MetricsResponse.newBuilder() .setCpuPercent(sample.cpuPercent) @@ -191,7 +250,9 @@ class DriverService( // stale session, then closes the backend so the iOS XCTest runner process // dies with us instead of being orphaned. fun shutdown() { - runCatching { launchedBundleId.getAndSet(null)?.let { backend.terminate(it) } } + runCatching { + launchedBundleId.getAndSet(null)?.let { backend.terminate(it) } + } runCatching { backend.close() } } @@ -209,8 +270,10 @@ class DriverService( // failures that do not extend Exception, and an uncaught one // kills the RPC as a channel-level Unknown instead of a status // the runner can classify. - observer.onError(io.grpc.Status.INTERNAL.withDescription(cause.toString()) - .withCause(cause).asRuntimeException()) + observer.onError( + io.grpc.Status.INTERNAL.withDescription(cause.toString()) + .withCause(cause).asRuntimeException(), + ) } } diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt index 39f9a5e..5692d29 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/Main.kt @@ -13,14 +13,17 @@ class SidecarServer( private val shutdownLatch = CountDownLatch(1) fun start(): Int { - val server = NettyServerBuilder.forAddress(InetSocketAddress("127.0.0.1", port)) + val server = NettyServerBuilder + .forAddress(InetSocketAddress("127.0.0.1", port)) .addService(service) .build() server.start() grpcServer = server - Runtime.getRuntime().addShutdownHook(Thread { - stop() - }) + Runtime.getRuntime().addShutdownHook( + Thread { + stop() + }, + ) return server.port } @@ -48,23 +51,41 @@ class SidecarServer( // lost from run output. private fun quietExpectedDriverNoise() { org.apache.logging.log4j.core.config.Configurator.setLevel( - "util.CommandLineUtils", org.apache.logging.log4j.Level.OFF) + "util.CommandLineUtils", + org.apache.logging.log4j.Level.OFF, + ) org.apache.logging.log4j.core.config.Configurator.setLevel( - "xcuitest.XCTestDriverClient", org.apache.logging.log4j.Level.OFF) + "xcuitest.XCTestDriverClient", + org.apache.logging.log4j.Level.OFF, + ) org.apache.logging.log4j.core.config.Configurator.setLevel( - "maestro.drivers.AndroidDriver", org.apache.logging.log4j.Level.OFF) + "maestro.drivers.AndroidDriver", + org.apache.logging.log4j.Level.OFF, + ) } fun main(arguments: Array) { quietExpectedDriverNoise() val port = arguments.indexOf("--port").let { index -> - if (index >= 0 && index + 1 < arguments.size) arguments[index + 1].toInt() else 0 + if (index >= 0 && index + 1 < arguments.size) { + arguments[index + 1].toInt() + } else { + 0 + } } val platform = arguments.indexOf("--platform").let { index -> - if (index >= 0 && index + 1 < arguments.size) arguments[index + 1] else "android" + if (index >= 0 && index + 1 < arguments.size) { + arguments[index + 1] + } else { + "android" + } } val serial = arguments.indexOf("--serial").let { index -> - if (index >= 0 && index + 1 < arguments.size) arguments[index + 1] else null + if (index >= 0 && index + 1 < arguments.size) { + arguments[index + 1] + } else { + null + } } val backend: DriverBackend = when (platform) { @@ -74,7 +95,9 @@ fun main(arguments: Array) { val service = DriverService(platform = platform, backend = backend) val server = SidecarServer(port, service) val boundPort = server.start() - println("sanderling-sidecar listening on 127.0.0.1:$boundPort platform=$platform") + println( + "sanderling-sidecar listening on 127.0.0.1:$boundPort platform=$platform", + ) System.out.flush() server.awaitTermination() } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/AdbOutputTimeoutTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/AdbOutputTimeoutTest.kt new file mode 100644 index 0000000..6a741b9 --- /dev/null +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/AdbOutputTimeoutTest.kt @@ -0,0 +1,120 @@ +package dev.sanderling.sidecar + +import org.junit.Test +import java.io.InputStream +import java.io.OutputStream +import java.util.concurrent.CountDownLatch +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class AdbOutputTimeoutTest { + + // An adb wedged on the link neither writes nor exits, so the read never + // reaches EOF. Without a bound on the READ the step waits for as long as + // adb feels like it: one such stall measured ~100s against a remote adb + // server. The bound has to release the reader as well as return, which is + // what killing the process does. + @Test(timeout = 20_000L) + fun aWedgedReadIsAbandonedAtTheBoundInsteadOfHangingForever() { + val process = FakeProcess(BlockingStream()) + val logged = mutableListOf() + + val started = System.currentTimeMillis() + val output = readProcessOutput(process, 200L, { "adb shell pidof" }) { + logged.add(it) + } + val elapsed = System.currentTimeMillis() - started + + assertEquals("", output, "a read that never lands is no answer") + assertTrue(process.destroyed, "the wedged adb must be killed, not left") + assertTrue( + elapsed < 10_000L, + "returned in ${elapsed}ms, not at a bound", + ) + assertEquals(1, logged.size, "a silent timeout hides a degrading link") + } + + @Test(timeout = 20_000L) + fun theAbandonedReadNamesTheCommandAndTheBound() { + val logged = mutableListOf() + readProcessOutput( + FakeProcess(BlockingStream()), + 200L, + { "adb -s emulator-5556 shell cat /proc/6103/stat" }, + ) { logged.add(it) } + + val line = logged.single() + assertTrue( + line.contains("adb -s emulator-5556 shell cat /proc/6103/stat"), + line, + ) + assertTrue(line.contains("200"), line) + } + + // The bound must cost the healthy path nothing: output that arrives comes + // back whole, and the process is left to exit on its own. + @Test(timeout = 20_000L) + fun outputThatArrivesComesBackWholeAndTheProcessSurvives() { + val text = "VmRSS:\t 123456 kB\nVmSize:\t 654321 kB\n" + val process = FakeProcess(text.byteInputStream()) + val logged = mutableListOf() + + val output = readProcessOutput(process, 5_000L, { "adb shell cat" }) { + logged.add(it) + } + + assertEquals(text, output) + assertTrue(!process.destroyed, "a process that answered is not killed") + assertTrue(logged.isEmpty(), "nothing to report on the healthy path") + } + + // Output larger than a pipe buffer is why the read cannot be deferred + // until after the process exits: a process with more to say than the + // buffer holds blocks writing while a waiter waits for it to finish. + @Test(timeout = 20_000L) + fun outputLargerThanAPipeBufferComesBackWhole() { + val text = "x".repeat(512 * 1024) + val output = readProcessOutput( + FakeProcess(text.byteInputStream()), + 5_000L, + { "adb logcat -d" }, + ) {} + assertEquals(text.length, output.length) + } +} + +// BlockingStream models a wedged adb: no bytes, and no EOF either, until the +// process is killed and the pipe closes under the reader. +private class BlockingStream : InputStream() { + private val released = CountDownLatch(1) + + override fun read(): Int { + released.await() + return -1 + } + + override fun close() { + released.countDown() + } +} + +private class FakeProcess(private val stream: InputStream) : Process() { + @Volatile var destroyed = false + private set + + override fun getOutputStream(): OutputStream = + OutputStream.nullOutputStream() + + override fun getInputStream(): InputStream = stream + + override fun getErrorStream(): InputStream = InputStream.nullInputStream() + + override fun waitFor(): Int = 0 + + override fun exitValue(): Int = 0 + + override fun destroy() { + destroyed = true + stream.close() + } +} diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DadbTargetTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DadbTargetTest.kt index 7db71f8..c51310c 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DadbTargetTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DadbTargetTest.kt @@ -2,6 +2,8 @@ package dev.sanderling.sidecar import org.junit.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue class DadbTargetTest { @@ -10,7 +12,10 @@ class DadbTargetTest { } @Test fun hostPortSerialConnectsDirectly() { - assertEquals(DadbTarget.Tcp("192.168.1.243", 5555), dadbTargetFor("192.168.1.243:5555")) + assertEquals( + DadbTarget.Tcp("192.168.1.243", 5555), + dadbTargetFor("192.168.1.243:5555"), + ) } @Test fun usbSerialRoutesThroughAdbServer() { @@ -20,6 +25,108 @@ class DadbTargetTest { // A colon with a non-numeric port is a USB serial that merely contains a // colon, not a host:port, so it must route through the adb server. @Test fun colonWithNonNumericPortIsAServerSerial() { - assertEquals(DadbTarget.Server("emulator:5554x"), dadbTargetFor("emulator:5554x")) + assertEquals( + DadbTarget.Server("emulator:5554x"), + dadbTargetFor("emulator:5554x"), + ) + } + + // A serial-addressed device is reached through whichever adb server the + // environment names. Ignoring it sends the run to this machine's own + // server, where the serial either is missing or, worse, names a different + // device that happens to share the emulator numbering. + @Test fun adbServerSocketNamesARemoteServer() { + assertEquals( + AdbServerEndpoint("100.68.126.75", 5037), + adbServerEndpoint( + env("ADB_SERVER_SOCKET" to "tcp:100.68.126.75:5037"), + ), + ) + } + + @Test fun adbServerSocketWithOnlyAPortStaysLocal() { + assertEquals( + AdbServerEndpoint("localhost", 5038), + adbServerEndpoint(env("ADB_SERVER_SOCKET" to "tcp:5038")), + ) + } + + @Test fun androidAdbServerAddressAndPortPairIsHonoured() { + assertEquals( + AdbServerEndpoint("10.0.0.4", 5040), + adbServerEndpoint( + env( + "ANDROID_ADB_SERVER_ADDRESS" to "10.0.0.4", + "ANDROID_ADB_SERVER_PORT" to "5040", + ), + ), + ) + } + + @Test fun adbServerSocketOutranksTheOlderPair() { + assertEquals( + AdbServerEndpoint("100.68.126.75", 5037), + adbServerEndpoint( + env( + "ADB_SERVER_SOCKET" to "tcp:100.68.126.75:5037", + "ANDROID_ADB_SERVER_ADDRESS" to "10.0.0.4", + "ANDROID_ADB_SERVER_PORT" to "5040", + ), + ), + ) + } + + @Test fun unsetEnvironmentKeepsTheLoopbackDefault() { + assertEquals( + AdbServerEndpoint("localhost", 5037), + adbServerEndpoint(env()), + ) + assertEquals( + AdbServerEndpoint("localhost", 5037), + adbServerEndpoint(env("ADB_SERVER_SOCKET" to "")), + ) + } + + @Test fun eitherHalfOfTheOlderPairAloneKeepsTheOtherDefault() { + assertEquals( + AdbServerEndpoint("10.0.0.4", 5037), + adbServerEndpoint(env("ANDROID_ADB_SERVER_ADDRESS" to "10.0.0.4")), + ) + assertEquals( + AdbServerEndpoint("localhost", 5040), + adbServerEndpoint(env("ANDROID_ADB_SERVER_PORT" to "5040")), + ) + } + + // A value that cannot be read must stop the run and say which variable + // held what. Falling back to loopback would drive this machine's devices + // while the operator believes the run is on the remote ones. + @Test fun malformedValuesFailNamingTheVariableAndItsContents() { + val cases = mapOf( + "ADB_SERVER_SOCKET" to listOf( + "100.68.126.75:5037", + "tcp:100.68.126.75:pear", + "tcp:", + "tcp::5037", + "unix:/tmp/adb", + "tcp:100.68.126.75:70000", + ), + "ANDROID_ADB_SERVER_PORT" to listOf("pear", "0", "-1"), + ) + for ((variable, values) in cases) { + for (value in values) { + val failure = assertFailsWith(value) { + adbServerEndpoint(env(variable to value)) + } + val message = failure.message.orEmpty() + assertTrue(message.contains(variable), message) + assertTrue(message.contains(value), message) + } + } } } + +private fun env(vararg entries: Pair): (String) -> String? { + val values = entries.toMap() + return { values[it] } +} diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt index afbccbd..37bc806 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt @@ -24,7 +24,8 @@ class DeviceOutputParserTest { assertEquals("FATAL EXCEPTION: main", lines[0].message) val year = java.util.Calendar.getInstance().get(java.util.Calendar.YEAR) - val cal = java.util.Calendar.getInstance().apply { timeInMillis = lines[0].unixMillis } + val cal = java.util.Calendar.getInstance() + .apply { timeInMillis = lines[0].unixMillis } assertEquals(year, cal.get(java.util.Calendar.YEAR)) assertEquals(56, cal.get(java.util.Calendar.SECOND)) assertEquals(789, cal.get(java.util.Calendar.MILLISECOND)) @@ -51,7 +52,9 @@ class DeviceOutputParserTest { @Test fun parseCpuTicksReturnsNullOnTruncatedOrNonNumericStat() { assertNull(parseCpuTicks("1234 (app) S 1 2 3")) - assertNull(parseCpuTicks("1234 (app) S " + (1..12).joinToString(" ") { "x" })) + assertNull( + parseCpuTicks("1234 (app) S " + (1..12).joinToString(" ") { "x" }), + ) assertNull(parseCpuTicks("")) } @@ -79,8 +82,14 @@ class DeviceOutputParserTest { } @Test fun parseBoundsAcceptsWellFormedAndRejectsMalformed() { - assertEquals(listOf(0, 0, 1080, 2340), parseBounds("[0,0,1080,2340]")?.toList()) - assertEquals(listOf(-5, -10, 20, 30), parseBounds("[-5,-10,20,30]")?.toList()) + assertEquals( + listOf(0, 0, 1080, 2340), + parseBounds("[0,0,1080,2340]")?.toList(), + ) + assertEquals( + listOf(-5, -10, 20, 30), + parseBounds("[-5,-10,20,30]")?.toList(), + ) assertNull(parseBounds("[0,0,1080]")) assertNull(parseBounds("0,0,1,1")) assertNull(parseBounds("[0, 0, 1, 1]")) @@ -92,10 +101,14 @@ class DeviceOutputParserTest { "resource-id" to "com.example:id/loginButton", "bounds" to "[10,20,110,80]", ) - assertEquals(listOf(10, 20, 110, 80), findBoundsBySelector(tree, "id:loginButton")?.toList()) assertEquals( listOf(10, 20, 110, 80), - findBoundsBySelector(tree, "id:com.example:id/loginButton")?.toList(), + findBoundsBySelector(tree, "id:loginButton")?.toList(), + ) + assertEquals( + listOf(10, 20, 110, 80), + findBoundsBySelector(tree, "id:com.example:id/loginButton") + ?.toList(), ) } @@ -104,21 +117,36 @@ class DeviceOutputParserTest { "resource-id" to "root", children = listOf( node("text" to "Sign in", "bounds" to "[1,2,3,4]"), - node("content-desc" to "AccountCardRow-7", "bounds" to "[5,6,7,8]"), + node( + "content-desc" to "AccountCardRow-7", + "bounds" to "[5,6,7,8]", + ), ), ) - assertEquals(listOf(1, 2, 3, 4), findBoundsBySelector(tree, "text:Sign in")?.toList()) - assertEquals(listOf(5, 6, 7, 8), findBoundsBySelector(tree, "descPrefix:AccountCard")?.toList()) + assertEquals( + listOf(1, 2, 3, 4), + findBoundsBySelector(tree, "text:Sign in")?.toList(), + ) + assertEquals( + listOf(5, 6, 7, 8), + findBoundsBySelector(tree, "descPrefix:AccountCard")?.toList(), + ) } @Test fun findBoundsBySelectorReturnsNullForBadSelectorOrNoMatch() { - val tree = node("resource-id" to "com.example:id/x", "bounds" to "[0,0,1,1]") + val tree = node( + "resource-id" to "com.example:id/x", + "bounds" to "[0,0,1,1]", + ) assertNull(findBoundsBySelector(tree, "id")) assertNull(findBoundsBySelector(tree, "id:missing")) } @Test fun findBoundsBySelectorReturnsNullWhenMatchHasMalformedBounds() { - val tree = node("resource-id" to "com.example:id/x", "bounds" to "not-bounds") + val tree = node( + "resource-id" to "com.example:id/x", + "bounds" to "not-bounds", + ) assertNull(findBoundsBySelector(tree, "id:x")) } @@ -132,17 +160,26 @@ class DeviceOutputParserTest { private fun ihdr(width: Int, height: Int): ByteArray { val b = ByteArray(33) for (i in 0 until 8) b[8 + i] = 0 - b[12] = 'I'.code.toByte(); b[13] = 'H'.code.toByte() - b[14] = 'D'.code.toByte(); b[15] = 'R'.code.toByte() - b[16] = (width ushr 24).toByte(); b[17] = (width ushr 16).toByte() - b[18] = (width ushr 8).toByte(); b[19] = width.toByte() - b[20] = (height ushr 24).toByte(); b[21] = (height ushr 16).toByte() - b[22] = (height ushr 8).toByte(); b[23] = height.toByte() + b[12] = 'I'.code.toByte() + b[13] = 'H'.code.toByte() + b[14] = 'D'.code.toByte() + b[15] = 'R'.code.toByte() + b[16] = (width ushr 24).toByte() + b[17] = (width ushr 16).toByte() + b[18] = (width ushr 8).toByte() + b[19] = width.toByte() + b[20] = (height ushr 24).toByte() + b[21] = (height ushr 16).toByte() + b[22] = (height ushr 8).toByte() + b[23] = height.toByte() return b } private fun node( vararg attrs: Pair, children: List = emptyList(), - ): maestro.TreeNode = maestro.TreeNode(attributes = attrs.toMap().toMutableMap(), children = children) + ): maestro.TreeNode = maestro.TreeNode( + attributes = attrs.toMap().toMutableMap(), + children = children, + ) } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt index 65d046b..0187cca 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DriverServiceTest.kt @@ -20,20 +20,34 @@ import org.junit.Test import kotlin.test.assertEquals import kotlin.test.assertTrue -private data class Quintuple(val a: A, val b: B, val c: C, val d: D, val e: E) +private data class Quintuple( + val a: A, + val b: B, + val c: C, + val d: D, + val e: E, +) class DriverServiceTest { @get:Rule val grpcCleanup: GrpcCleanupRule = GrpcCleanupRule() - private fun newClient(backend: DriverBackend): DriverGrpc.DriverBlockingStub { + private fun newClient( + backend: DriverBackend, + ): DriverGrpc.DriverBlockingStub { val serverName = InProcessServerBuilder.generateName() val service = DriverService(platform = "android", backend = backend) grpcCleanup.register( - InProcessServerBuilder.forName(serverName).directExecutor().addService(service).build().start() + InProcessServerBuilder.forName(serverName) + .directExecutor() + .addService(service) + .build() + .start(), ) val channel: ManagedChannel = grpcCleanup.register( - InProcessChannelBuilder.forName(serverName).directExecutor().build() + InProcessChannelBuilder.forName(serverName) + .directExecutor() + .build(), ) return DriverGrpc.newBlockingStub(channel) } @@ -59,20 +73,32 @@ class DriverServiceTest { var terminated: String? = null var closed = false val backend = object : DriverBackend by StubDriverBackend("android") { - override fun terminate(bundleId: String) { terminated = bundleId } - override fun close() { closed = true } + override fun terminate(bundleId: String) { + terminated = bundleId + } + override fun close() { + closed = true + } } val serverName = InProcessServerBuilder.generateName() val service = DriverService(platform = "android", backend = backend) grpcCleanup.register( - InProcessServerBuilder.forName(serverName).directExecutor().addService(service).build().start() + InProcessServerBuilder.forName(serverName) + .directExecutor() + .addService(service) + .build() + .start(), ) val channel: ManagedChannel = grpcCleanup.register( - InProcessChannelBuilder.forName(serverName).directExecutor().build() + InProcessChannelBuilder.forName(serverName) + .directExecutor() + .build(), ) val client = DriverGrpc.newBlockingStub(channel) - client.launch(LaunchRequest.newBuilder().setBundleId("com.example").build()) + client.launch( + LaunchRequest.newBuilder().setBundleId("com.example").build(), + ) service.shutdown() assertEquals("com.example", terminated) @@ -83,8 +109,12 @@ class DriverServiceTest { var terminated: String? = null var closed = false val backend = object : DriverBackend by StubDriverBackend("android") { - override fun terminate(bundleId: String) { terminated = bundleId } - override fun close() { closed = true } + override fun terminate(bundleId: String) { + terminated = bundleId + } + override fun close() { + closed = true + } } val service = DriverService(platform = "android", backend = backend) @@ -128,17 +158,17 @@ class DriverServiceTest { // the runner can tell transient failures from fatal ones. @Test fun backendStatusCodePassesThrough() { val backend = object : DriverBackend by StubDriverBackend("android") { - override fun inputText(text: String) { + override fun inputText(text: String): Unit = throw io.grpc.Status.UNAVAILABLE .withDescription("connection dropped mid-action") .asRuntimeException() - } } val client = newClient(backend) - val thrown = kotlin.test.assertFailsWith { - client.inputText(Text.newBuilder().setValue("hello").build()) - } + val thrown = + kotlin.test.assertFailsWith { + client.inputText(Text.newBuilder().setValue("hello").build()) + } assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) } @@ -147,17 +177,19 @@ class DriverServiceTest { // channel-level Unknown the runner cannot classify. @Test fun nonExceptionThrowableMapsToInternal() { val backend = object : DriverBackend by StubDriverBackend("android") { - override fun inputText(text: String) { + override fun inputText(text: String): Unit = throw Throwable("only one gesture can be performed at a time") - } } val client = newClient(backend) - val thrown = kotlin.test.assertFailsWith { - client.inputText(Text.newBuilder().setValue("hello").build()) - } + val thrown = + kotlin.test.assertFailsWith { + client.inputText(Text.newBuilder().setValue("hello").build()) + } assertEquals(io.grpc.Status.Code.INTERNAL, thrown.status.code) - assertTrue(thrown.status.description.orEmpty().contains("only one gesture")) + assertTrue( + thrown.status.description.orEmpty().contains("only one gesture"), + ) } @Test fun reapOrphanIosRunnersKillsStrayXcodebuildAndRunnerApp() { @@ -171,7 +203,16 @@ class DriverServiceTest { assertEquals("pkill", commands[0][0]) assertTrue(commands[0][2].contains("test-without-building")) assertTrue(commands[0][2].contains("UDID-1234")) - assertEquals(listOf("xcrun", "simctl", "terminate", "UDID-1234", IOS_XCTEST_RUNNER_BUNDLE_ID), commands[1]) + assertEquals( + listOf( + "xcrun", + "simctl", + "terminate", + "UDID-1234", + IOS_XCTEST_RUNNER_BUNDLE_ID, + ), + commands[1], + ) } @Test fun reapOrphanIosRunnersReportsNothingFound() { @@ -192,7 +233,9 @@ class DriverServiceTest { // still executing fails instead of queuing. val tapAction = { if (!inFlight.compareAndSet(false, true)) { - throw IllegalStateException("only one gesture can be performed at a time") + throw IllegalStateException( + "only one gesture can be performed at a time", + ) } invocations.incrementAndGet() Thread.sleep(150) @@ -211,7 +254,9 @@ class DriverServiceTest { val tapAction = { if (failedFirst.compareAndSet(false, true)) { Thread.sleep(60) - throw IllegalStateException("only one gesture can be performed at a time") + throw IllegalStateException( + "only one gesture can be performed at a time", + ) } landed.incrementAndGet() Unit @@ -226,21 +271,38 @@ class DriverServiceTest { // directly. val taps = mutableListOf>() val backend = object : DriverBackend { - override fun launch(bundleId: String, clearState: Boolean, env: Map) {} + override fun launch( + bundleId: String, + clearState: Boolean, + env: Map, + ) {} override fun terminate(bundleId: String) {} - override fun tap(x: Int, y: Int) { taps.add(x to y) } + override fun tap(x: Int, y: Int) { + taps.add(x to y) + } override fun tapSelector(selector: String) {} override fun inputText(text: String) {} override fun eraseText(characterCount: Int) {} - override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) {} + override fun swipe( + fromX: Int, + fromY: Int, + toX: Int, + toY: Int, + durationMillis: Long, + ) {} override fun pressKey(key: String) {} override fun longPress(x: Int, y: Int) {} - override fun screenshot(): Triple = Triple(byteArrayOf(), 0, 0) + override fun screenshot(): Triple = + Triple(byteArrayOf(), 0, 0) override fun hierarchy(): String = "{}" - override fun recentLogs(sinceUnixMillis: Long, minLevel: String): List = emptyList() + override fun recentLogs( + sinceUnixMillis: Long, + minLevel: String, + ): List = emptyList() override fun waitForIdle(durationMillis: Long) {} override fun healthy(): Boolean = true - override fun metrics(bundleId: String): MetricsSample = MetricsSample(0.0, 0L, 0L) + override fun metrics(bundleId: String): MetricsSample = + MetricsSample(0.0, 0L, 0L) } val client = newClient(backend) @@ -252,13 +314,16 @@ class DriverServiceTest { val backend = StubDriverBackend("android") val client = newClient(backend) - client.eraseText(EraseTextRequest.newBuilder().setCharacterCount(11).build()) + client.eraseText( + EraseTextRequest.newBuilder().setCharacterCount(11).build(), + ) assertEquals(11, backend.lastEraseCharacterCount) } @Test fun screenshotReturnsBackendBytes() { val backend = object : DriverBackend by StubDriverBackend("android") { - override fun screenshot(): Triple = Triple(byteArrayOf(1, 2, 3), 1080, 2340) + override fun screenshot(): Triple = + Triple(byteArrayOf(1, 2, 3), 1080, 2340) } val client = newClient(backend) @@ -294,7 +359,13 @@ class DriverServiceTest { @Test fun swipeForwardsEndpointsAndDuration() { var observed: Quintuple? = null val backend = object : DriverBackend by StubDriverBackend("android") { - override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) { + override fun swipe( + fromX: Int, + fromY: Int, + toX: Int, + toY: Int, + durationMillis: Long, + ) { observed = Quintuple(fromX, fromY, toX, toY, durationMillis) } } @@ -325,14 +396,18 @@ class DriverServiceTest { @Test fun recentLogsReturnsBackendEntries() { val backend = object : DriverBackend by StubDriverBackend("android") { - override fun recentLogs(sinceUnixMillis: Long, minLevel: String): List { - return listOf(LogLine(1, "E", "AndroidRuntime", "boom")) - } + override fun recentLogs( + sinceUnixMillis: Long, + minLevel: String, + ): List = listOf(LogLine(1, "E", "AndroidRuntime", "boom")) } val client = newClient(backend) val response = client.recentLogs( - RecentLogsRequest.newBuilder().setSinceUnixMillis(0).setLevelAtLeast("E").build(), + RecentLogsRequest.newBuilder() + .setSinceUnixMillis(0) + .setLevelAtLeast("E") + .build(), ) assertEquals(1, response.entriesCount) assertEquals("AndroidRuntime", response.getEntries(0).tag) @@ -351,7 +426,9 @@ class DriverServiceTest { } val client = newClient(backend) - client.launch(LaunchRequest.newBuilder().setBundleId("com.launched").build()) + client.launch( + LaunchRequest.newBuilder().setBundleId("com.launched").build(), + ) client.metrics(MetricsRequest.getDefaultInstance()) assertEquals("com.launched", sampled) @@ -367,8 +444,12 @@ class DriverServiceTest { } val client = newClient(backend) - client.launch(LaunchRequest.newBuilder().setBundleId("com.launched").build()) - client.metrics(MetricsRequest.newBuilder().setBundleId("com.other").build()) + client.launch( + LaunchRequest.newBuilder().setBundleId("com.launched").build(), + ) + client.metrics( + MetricsRequest.newBuilder().setBundleId("com.other").build(), + ) assertEquals("com.other", sampled) } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/EraseTextTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/EraseTextTest.kt new file mode 100644 index 0000000..cfa1131 --- /dev/null +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/EraseTextTest.kt @@ -0,0 +1,128 @@ +package dev.sanderling.sidecar + +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class EraseTextTest { + + // maestro's eraseText sends one delete per character through its + // instrumentation, measured at 29.6 ms/char on the API 34 emulator: the + // 4096-character string the corpus types cost ~121s to clear, a fifth of a + // 20 minute run for one step. Selecting the field and deleting the + // selection is the same two key events whatever the field holds. + @Test fun aClearedFieldCostsTwoKeyEventsWhateverItsLength() { + for (length in listOf(1, 21, 512, 4096)) { + val sent = mutableListOf() + eraseFocusedField(length, { sent.add(it) }) { 0 } + assertEquals( + listOf(SELECT_ALL_COMMAND, DELETE_KEY_COMMAND), + sent, + "length $length must not scale the erase", + ) + } + } + + // The dangerous failure is a fast erase that leaves characters behind: the + // next InputText appends to the residue and every reading downstream is + // wrong with nothing to catch it. A field the select-all did not clear is + // finished off per character rather than assumed empty. + @Test fun aFieldTheSelectAllMissedIsFinishedOffPerCharacter() { + val sent = mutableListOf() + eraseFocusedField(4096, { sent.add(it) }) { 4096 } + + assertEquals(SELECT_ALL_COMMAND, sent.first()) + assertEquals(DELETE_KEY_COMMAND, sent[1]) + assertEquals( + 4096, + sent.drop(2).sumOf { command -> + command.removePrefix("input keyevent ").split(" ").size + }, + "every character must still be deleted", + ) + } + + // Unknown is not empty. A tree that cannot name the focused field is no + // evidence the erase worked, and the safe way to be wrong is the delete + // that costs time rather than the one that leaves residue. + @Test fun aFieldThatCannotBeReadIsFinishedOffRatherThanAssumedEmpty() { + val sent = mutableListOf() + eraseFocusedField(8, { sent.add(it) }) { null } + assertTrue(sent.size > 2, "an unverified erase must not stop at two") + } + + @Test fun nothingToEraseIssuesNoKeysAtAll() { + val sent = mutableListOf() + eraseFocusedField(0, { sent.add(it) }) { 0 } + eraseFocusedField(-1, { sent.add(it) }) { 0 } + assertEquals(emptyList(), sent) + } + + // Batching is what keeps the fallback affordable: one round trip per batch + // rather than one per character, measured 2.3 ms/char against maestro's + // 29.6. The count must survive the batching exactly. + @Test fun deleteKeyCommandsBatchesWithoutLosingACharacter() { + for (count in listOf(1, 199, 200, 201, 4096)) { + val commands = deleteKeyCommands(count, DELETE_BATCH_KEYS) + val keys = commands.flatMap { + it.removePrefix("input keyevent ").split(" ") + } + assertEquals(count, keys.size, "count $count") + assertTrue(keys.all { it == "67" }, "only KEYCODE_DEL") + assertTrue( + commands.size <= (count + DELETE_BATCH_KEYS - 1) / + DELETE_BATCH_KEYS, + "count $count used ${commands.size} round trips", + ) + } + } + + // The erase targets the field the runner just tapped, so the length that + // decides whether it worked is that field's, not some other field that + // legitimately still holds text. + // + // The tree these fixtures copy is the one the device really returns, and + // it holds the trap: an open keyboard puts a SECOND focused node in the + // tree, one of the IME's own keys, and it carries no text. Reading the + // first focused node would call a field that still holds 4096 characters + // empty, which is the one wrong answer that matters here. maestro's tree + // also carries no "editable" attribute at all, so the text field has to be + // recognised by its class. + @Test fun theFocusedFieldIsReadPastTheKeyboardsOwnFocusedKey() { + assertEquals(4096, focusedEditableTextLength(TREE_WITH_FULL_FIELD)) + assertEquals(0, focusedEditableTextLength(TREE_WITH_EMPTY_FIELD)) + } + + @Test fun aTreeWithNoFocusedFieldReadsAsUnknown() { + assertEquals(null, focusedEditableTextLength(TREE_WITH_NO_FOCUS)) + assertEquals(null, focusedEditableTextLength("")) + assertEquals(null, focusedEditableTextLength("not json")) + } +} + +// The keyboard's own focused key, exactly as the device reports it: focused, +// no text, and not a text field. +private val IME_FOCUSED_KEY = + """ + {"attributes":{"text":"","resource-id": + "com.google.android.inputmethod.latin:id/key_pos_header_access", + "focused":"true","class":"android.widget.FrameLayout"},"children":[]} + """.trimIndent() + +private fun tree(focusedText: String?, otherText: String): String { + val field = focusedText?.let { + """,{"attributes":{"resource-id":"AccountNameField","focused":"true", + "class":"android.widget.EditText","text":"$it"},"children":[]}""" + } ?: "" + return """ + {"attributes":{"resource-id":"AddAccountScreen"},"children":[ + $IME_FOCUSED_KEY $field, + {"attributes":{"resource-id":"OtherField","focused":"false", + "class":"android.widget.EditText","text":"$otherText"}, + "children":[]}]} + """.trimIndent() +} + +private val TREE_WITH_FULL_FIELD = tree("a".repeat(4096), "keep me") +private val TREE_WITH_EMPTY_FIELD = tree("", "keep me") +private val TREE_WITH_NO_FOCUS = tree(null, "keep me") diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/IdleDetectionTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/IdleDetectionTest.kt index 82d48bd..201b3fe 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/IdleDetectionTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/IdleDetectionTest.kt @@ -21,11 +21,17 @@ class IdleDetectionTest { assertFalse(StubDriverBackend.isAnimationCountIdle("3\n")) } - @Test fun idleWhenOutputEmpty() { - assertTrue(StubDriverBackend.isAnimationCountIdle("")) + // A dumpsys that said nothing does not say the device is still. Reading + // absence as idle is how a settle returns instantly on a degraded link and + // hands the runner a screen caught mid-animation; the caller bounds its own + // wait, so the cost of being wrong the other way is a wait it already + // budgeted for. The exception path of the same probe already answers false. + @Test fun unreadableOutputIsNotIdle() { + assertFalse(StubDriverBackend.isAnimationCountIdle("")) + assertFalse(StubDriverBackend.isAnimationCountIdle(" \n")) } - @Test fun idleWhenOutputIsNotANumber() { - assertTrue(StubDriverBackend.isAnimationCountIdle("error: no service")) + @Test fun unparseableOutputIsNotIdle() { + assertFalse(StubDriverBackend.isAnimationCountIdle("error: no service")) } } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/InputTextTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/InputTextTest.kt index c466d00..89d7a90 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/InputTextTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/InputTextTest.kt @@ -32,7 +32,8 @@ class InputTextTest { val fallback = listOf( "Emergency Fund", "🙂🔥💸", " ", "\t\n", "'; DROP TABLE--", "", "../../etc/passwd", "%s%n", "", - "-1", "-rf", // a leading dash could be read as an option by `input text` + // a leading dash could be read as an option by `input text` + "-1", "-rf", ) for (text in fallback) { assertTrue( @@ -190,12 +191,12 @@ class InputTextTest { @Test fun parseResumedPackageReadsEachResumedActivityWording() { val cases = mapOf( - " topResumedActivity=ActivityRecord{8b u0 app.folio/.MainActivity t42}" to - "app.folio", - " mResumedActivity: ActivityRecord{1c u0 com.example.app/.Home t9}" to - "com.example.app", - " ResumedActivity: ActivityRecord{2d u0 app.folio/com.folio.Detail t9}" to - "app.folio", + " topResumedActivity=ActivityRecord{8b u0 " + + "app.folio/.MainActivity t42}" to "app.folio", + " mResumedActivity: ActivityRecord{1c u0 " + + "com.example.app/.Home t9}" to "com.example.app", + " ResumedActivity: ActivityRecord{2d u0 " + + "app.folio/com.folio.Detail t9}" to "app.folio", ) for ((line, want) in cases) { assertEquals(want, parseResumedPackage(line), line) @@ -206,10 +207,249 @@ class InputTextTest { ) } + @Test fun aReadableDumpsysNamesTheResumedPackage() { + val warnings = mutableListOf() + assertEquals( + "app.folio", + typingOwner(RESUMED_DUMPSYS, "app.folio") { warnings.add(it) }, + ) + assertTrue(warnings.isEmpty(), "nothing to report when the read worked") + } + + // A dumpsys that said nothing is not evidence that focus is fine. Handing + // typeChunks a null owner turns the guard off outright, and the keystrokes + // then go wherever the foreground happens to be. + @Test fun anUnreadableDumpsysGuardsWithTheLaunchedBundle() { + val warnings = mutableListOf() + assertEquals( + "app.folio", + typingOwner("", "app.folio") { warnings.add(it) }, + ) + assertEquals(1, warnings.size, "a degraded guard must not be silent") + assertTrue(warnings.single().contains("app.folio"), warnings.single()) + } + + // Wording no marker matches is the same "we do not know" as an empty read. + @Test fun dumpsysWithNoResumedMarkerGuardsWithTheLaunchedBundle() { + assertEquals( + "app.folio", + typingOwner(" mFocusedApp=null\n nothing here\n", "app.folio") {}, + ) + } + + // The whole point of the fallback: on a link that cannot answer, typing is + // still guarded, so a foreground that was stolen stops it after the first + // chunk instead of spraying the rest into whatever took focus. + @Test fun unreadableLinkStillStopsTypingWhenFocusWasStolen() { + val owner = typingOwner("", "app.folio") {} + val sent = mutableListOf() + + typeChunks(listOf("aaa", "bbb", "ccc"), owner, { + "com.android.launcher" + }) { sent.add(it) } + + assertEquals( + listOf("aaa"), + sent, + "an unguarded type would have sent every chunk to the launcher", + ) + } + + // And the other half of the trade: the fallback must not turn a degraded + // link into a run that types nothing. A no-op InputText on every step is a + // green run that tested nothing, which is worse than the spray it avoids. + @Test fun unreadableLinkStillTypesWhenTheAppKeepsFocus() { + val owner = typingOwner("", "app.folio") {} + val sent = mutableListOf() + + val typed = typeChunks(listOf("aaa", "bbb", "cc"), owner, { + "app.folio" + }) { sent.add(it) } + + assertEquals(listOf("aaa", "bbb", "cc"), sent) + assertEquals(8, typed) + } + + // With no launch recorded there is nothing to guard against, and the honest + // answer is to say the guard is off rather than imply it ran. + @Test fun noLaunchedBundleLeavesTheGuardOffAndSaysSo() { + val warnings = mutableListOf() + assertEquals(null, typingOwner("", null) { warnings.add(it) }) + assertEquals(1, warnings.size) + } + @Test fun maestroKeyForResolvesAndRejects() { assertEquals(maestro.KeyCode.BACK, maestroKeyFor("back")) assertEquals(maestro.KeyCode.BACK, maestroKeyFor("BACK")) assertEquals(maestro.KeyCode.ENTER, maestroKeyFor("enter")) assertFailsWith { maestroKeyFor("zorp") } } + + // An IME left open hides every app node beneath it from the hierarchy, so a + // form whose submit button sits under the keyboard becomes unreachable for + // as long as the fuzzer keeps typing into it. Typing must close the + // keyboard it raised. + @Test fun dismissSoftKeyboardClosesAnOpenIme() { + val commands = mutableListOf() + dismissSoftKeyboard { + commands.add(it) + IME_OPEN_DUMPSYS + } + assertEquals( + listOf("dumpsys input_method", "input keyevent 4"), + commands, + ) + } + + // The guard is the dangerous half: BACK is only swallowed by an open IME, + // so dismissing unconditionally would turn every InputText into a back + // press and walk the fuzzer straight out of the screen it was filling in. + @Test fun dismissSoftKeyboardSendsNoBackWhenNoImeIsOpen() { + val commands = mutableListOf() + dismissSoftKeyboard { + commands.add(it) + IME_CLOSED_DUMPSYS + } + assertEquals(listOf("dumpsys input_method"), commands) + } + + // Typing is not the only thing that raises the keyboard: tapping a field + // raises it too, and nothing was closing that one. The snapshot the picker + // chooses from is missing every app node the keyboard covers, so the step + // spends its budget choosing between the few targets left. Closing it + // before the tree is read is what gives the step its targets back. + @Test fun aKeyboardInTheTreeIsClosedBeforeTheTreeIsReturned() { + var backs = 0 + val reads = mutableListOf() + val settled = treeWithoutKeyboard( + IME_TREE, + IME_PACKAGE, + dismiss = { backs++ }, + reread = { APP_TREE.also { reads.add(it) } }, + sleep = {}, + ) + assertEquals(APP_TREE, settled) + assertEquals(1, backs, "one BACK closes the keyboard") + assertEquals(1, reads.size, "the tree is re-read once it is gone") + } + + // The guard has to be the tree itself. BACK with no keyboard open + // navigates out of the screen, so a snapshot that pressed it on every read + // would walk the fuzzer backwards out of the app a step at a time. + @Test fun aTreeWithNoKeyboardIsReturnedUntouched() { + var backs = 0 + var reads = 0 + val settled = treeWithoutKeyboard( + APP_TREE, + IME_PACKAGE, + dismiss = { backs++ }, + reread = { + reads++ + APP_TREE + }, + sleep = {}, + ) + assertEquals(APP_TREE, settled) + assertEquals(0, backs, "no keyboard in the tree means no BACK") + assertEquals(0, reads, "and no second hierarchy read to pay for") + } + + // An unknown IME package is the honest "cannot tell", and the safe way to + // be wrong is to leave the keyboard up rather than press BACK blind. + @Test fun anUnknownImePackageSendsNoBack() { + var backs = 0 + assertEquals( + IME_TREE, + treeWithoutKeyboard( + IME_TREE, + null, + dismiss = { backs++ }, + reread = { APP_TREE }, + sleep = {}, + ), + ) + assertEquals(0, backs) + } + + // A keyboard the app puts straight back gets ONE back press, not one per + // re-read. The flag behind the older dismissal lags a BACK by up to 0.6s, + // and a burst of them inside that window is how a dismissal turns into + // navigation. + @Test fun aKeyboardThatStaysUpIsNotBackPressedRepeatedly() { + var backs = 0 + var reads = 0 + val settled = treeWithoutKeyboard( + IME_TREE, + IME_PACKAGE, + dismiss = { backs++ }, + reread = { + reads++ + IME_TREE + }, + sleep = {}, + ) + assertEquals(IME_TREE, settled, "the caller still gets a tree") + assertEquals(1, backs) + assertTrue( + reads in 1..KEYBOARD_DISMISS_READS, + "bounded re-reads, got $reads", + ) + } + + @Test fun imePackageOfReadsTheComponentAndRejectsNonsense() { + assertEquals( + "com.google.android.inputmethod.latin", + imePackageOf( + "com.google.android.inputmethod.latin/.LatinIME\n", + ), + ) + assertEquals(null, imePackageOf("null")) + assertEquals(null, imePackageOf("")) + assertEquals(null, imePackageOf(" \n")) + } + + @Test fun treeShowsImeMatchesTheImesOwnViewIdsOnly() { + assertTrue(treeShowsIme(IME_TREE, IME_PACKAGE)) + assertTrue(!treeShowsIme(APP_TREE, IME_PACKAGE)) + } } + +private val RESUMED_DUMPSYS = + """ + mFocusedApp=ActivityRecord{1a u0 app.folio/.MainActivity t14} + topResumedActivity=ActivityRecord{f3 u0 app.folio/.MainActivity t14} + """.trimIndent() + +private const val IME_PACKAGE = "com.google.android.inputmethod.latin" + +private val APP_TREE = + """ + {"attributes":{"resource-id":"AddAccountScreen"},"children":[ + {"attributes":{"resource-id":"AccountNameField"},"children":[]}, + {"attributes":{"resource-id":"AddAccountSubmit"},"children":[]}]} + """.trimIndent() + +// The submit control is gone: an open keyboard does not merely cover the node, +// it takes it out of the tree the picker enumerates. +private val IME_TREE = + """ + {"attributes":{"resource-id":"AddAccountScreen"},"children":[ + {"attributes":{"resource-id":"AccountNameField"},"children":[]}, + {"attributes":{ + "resource-id":"com.google.android.inputmethod.latin:id/keyboard_holder" + },"children":[]}]} + """.trimIndent() + +private val IME_OPEN_DUMPSYS = + """ + mCurMethodId=com.google.android.inputmethod.latin/.LatinIME + mInputShown=true + mSystemReady=true mInteractive=true + """.trimIndent() + +private val IME_CLOSED_DUMPSYS = + """ + mCurMethodId=com.google.android.inputmethod.latin/.LatinIME + mInputShown=false + mSystemReady=true mInteractive=true + """.trimIndent() diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/ResolveActivityTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/ResolveActivityTest.kt index 5ffaab8..77b25ab 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/ResolveActivityTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/ResolveActivityTest.kt @@ -12,28 +12,40 @@ class ResolveActivityTest { com.example.app/.MainActivity """.trimIndent() - val activity = StubDriverBackend.parseResolvedActivity("com.example.app", output) + val activity = StubDriverBackend.parseResolvedActivity( + "com.example.app", + output, + ) assertEquals(".MainActivity", activity) } @Test fun extractsFullyQualifiedActivity() { val output = "com.example.app/com.example.app.ui.LaunchActivity" - val activity = StubDriverBackend.parseResolvedActivity("com.example.app", output) + val activity = StubDriverBackend.parseResolvedActivity( + "com.example.app", + output, + ) assertEquals("com.example.app.ui.LaunchActivity", activity) } @Test fun returnsNullWhenPackageNotFound() { val output = "No activity found" - val activity = StubDriverBackend.parseResolvedActivity("com.example.app", output) + val activity = StubDriverBackend.parseResolvedActivity( + "com.example.app", + output, + ) assertNull(activity) } @Test fun doesNotMatchDifferentPackagePrefix() { val output = "other.pkg/.MainActivity" - val activity = StubDriverBackend.parseResolvedActivity("com.example.app", output) + val activity = StubDriverBackend.parseResolvedActivity( + "com.example.app", + output, + ) assertNull(activity) } } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/RouteTransitionTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/RouteTransitionTest.kt index 3c7c776..7836f66 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/RouteTransitionTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/RouteTransitionTest.kt @@ -13,10 +13,13 @@ class RouteTransitionTest { private fun screen(id: String, child: String = "") = """{"attributes":{"resource-id":"$id"},"children":[$child]}""" - private fun tree(vararg children: String) = - """{"attributes":{"resource-id":"root"},"children":[${children.joinToString(",")}]}""" + private fun tree(vararg children: String): String { + val joined = children.joinToString(",") + return """{"attributes":{"resource-id":"root"},"children":[$joined]}""" + } - private val crossFade = tree(screen("LedgerScreen"), screen("AddTransactionScreen")) + private val crossFade = + tree(screen("LedgerScreen"), screen("AddTransactionScreen")) private val landed = tree(screen("AddTransactionScreen")) @Test fun waitsForTheCrossFadeToLandAndReturnsTheLandedTree() { @@ -29,8 +32,15 @@ class RouteTransitionTest { reads++ if (reads <= 3) crossFade else landed } - assertTrue(reads > 3, "must keep reading until the fade lands, reads=$reads") - assertEquals(1, countRouteScreens(settled), "must return a tree with one route") + assertTrue( + reads > 3, + "must keep reading until the fade lands, reads=$reads", + ) + assertEquals( + 1, + countRouteScreens(settled), + "must return a tree with one route", + ) } @Test fun settledFrameCostsExactlyOneRead() { @@ -54,13 +64,21 @@ class RouteTransitionTest { // would burn the whole poll budget and still hand over a frame the // runner refuses to act on. val nested = tree(screen("HomeScreen", screen("HomeScreen"))) - assertEquals(1, countRouteScreens(nested), "the same id twice is one route") + assertEquals( + 1, + countRouteScreens(nested), + "the same id twice is one route", + ) var reads = 0 awaitSettledTree { reads++ nested } - assertEquals(1, reads, "a repeated route id must not be treated as a transition") + assertEquals( + 1, + reads, + "a repeated route id must not be treated as a transition", + ) } @Test fun aLayoutThatKeepsTwoRoutesIsBoundedByTheCap() { @@ -78,7 +96,11 @@ class RouteTransitionTest { elapsed < TRANSITION_POLL_CAP_MILLIS + 1000L, "must stop at the cap, elapsed=${elapsed}ms", ) - assertEquals(crossFade, settled, "the caller still gets a tree to record") + assertEquals( + crossFade, + settled, + "the caller still gets a tree to record", + ) } @Test fun capCoversTheNavHostFadePlusTheStreak() { @@ -89,20 +111,29 @@ class RouteTransitionTest { val fadeMillis = 700L val start = System.currentTimeMillis() val settled = awaitSettledTree { - if (System.currentTimeMillis() - start < fadeMillis) crossFade else landed + if (System.currentTimeMillis() - start < fadeMillis) { + crossFade + } else { + landed + } } val elapsed = System.currentTimeMillis() - start - assertEquals(landed, settled, "must hand back the landed tree, not the fade") + assertEquals( + landed, + settled, + "must hand back the landed tree, not the fade", + ) assertTrue( elapsed >= fadeMillis, "cannot have settled before the fade ended, elapsed=${elapsed}ms", ) assertTrue( elapsed < TRANSITION_POLL_CAP_MILLIS, - "the ${TRANSITION_POLL_CAP_MILLIS}ms cap has to leave room for a ${fadeMillis}ms " + - "fade and the ${TRANSITION_STABLE_STREAK_MILLIS}ms streak after it, but the " + - "wait ran to the cap instead, elapsed=${elapsed}ms", + "the ${TRANSITION_POLL_CAP_MILLIS}ms cap has to leave room for " + + "a ${fadeMillis}ms fade and the " + + "${TRANSITION_STABLE_STREAK_MILLIS}ms streak after it, but " + + "the wait ran to the cap instead, elapsed=${elapsed}ms", ) } } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/SidecarServerTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/SidecarServerTest.kt index f449c42..05e24a8 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/SidecarServerTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/SidecarServerTest.kt @@ -6,7 +6,10 @@ import kotlin.test.assertTrue class SidecarServerTest { @Test fun startBindsEphemeralPortAndStopReleasesIt() { - val server = SidecarServer(port = 0, service = DriverService(backend = StubDriverBackend("android"))) + val server = SidecarServer( + port = 0, + service = DriverService(backend = StubDriverBackend("android")), + ) val boundPort = server.start() try { assertTrue(boundPort > 0, "expected ephemeral port, got $boundPort") diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/SnapshotHandlerTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/SnapshotHandlerTest.kt index 13c6e45..2252e4c 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/SnapshotHandlerTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/SnapshotHandlerTest.kt @@ -19,14 +19,22 @@ class SnapshotHandlerTest { @get:Rule val grpcCleanup: GrpcCleanupRule = GrpcCleanupRule() - private fun newClient(backend: DriverBackend): DriverGrpc.DriverBlockingStub { + private fun newClient( + backend: DriverBackend, + ): DriverGrpc.DriverBlockingStub { val serverName = InProcessServerBuilder.generateName() val service = DriverService(platform = "android", backend = backend) grpcCleanup.register( - InProcessServerBuilder.forName(serverName).directExecutor().addService(service).build().start(), + InProcessServerBuilder.forName(serverName) + .directExecutor() + .addService(service) + .build() + .start(), ) val channel: ManagedChannel = grpcCleanup.register( - InProcessChannelBuilder.forName(serverName).directExecutor().build(), + InProcessChannelBuilder.forName(serverName) + .directExecutor() + .build(), ) return DriverGrpc.newBlockingStub(channel) } @@ -37,8 +45,10 @@ class SnapshotHandlerTest { // forward those calls to the delegate, not these overrides. Override // snapshot() directly so the test exercises the wire path end-to-end. val backend = object : DriverBackend by StubDriverBackend("android") { - override fun snapshot(): SnapshotSample = - SnapshotSample("{\"x\":1}", Triple(byteArrayOf(7, 8, 9), 1080, 2340)) + override fun snapshot(): SnapshotSample = SnapshotSample( + "{\"x\":1}", + Triple(byteArrayOf(7, 8, 9), 1080, 2340), + ) } val client = newClient(backend) @@ -55,13 +65,23 @@ class SnapshotHandlerTest { // aligned with the final hierarchy snapshot the runner accepts. val callOrder = mutableListOf() val backend = object : DriverBackend { - override fun launch(bundleId: String, clearState: Boolean, env: Map) {} + override fun launch( + bundleId: String, + clearState: Boolean, + env: Map, + ) {} override fun terminate(bundleId: String) {} override fun tap(x: Int, y: Int) {} override fun tapSelector(selector: String) {} override fun inputText(text: String) {} override fun eraseText(characterCount: Int) {} - override fun swipe(fromX: Int, fromY: Int, toX: Int, toY: Int, durationMillis: Long) {} + override fun swipe( + fromX: Int, + fromY: Int, + toX: Int, + toY: Int, + durationMillis: Long, + ) {} override fun pressKey(key: String) {} override fun longPress(x: Int, y: Int) {} override fun screenshot(): Triple { @@ -72,10 +92,14 @@ class SnapshotHandlerTest { callOrder.add("hierarchy") return "{}" } - override fun recentLogs(sinceUnixMillis: Long, minLevel: String): List = emptyList() + override fun recentLogs( + sinceUnixMillis: Long, + minLevel: String, + ): List = emptyList() override fun waitForIdle(durationMillis: Long) {} override fun healthy(): Boolean = true - override fun metrics(bundleId: String): MetricsSample = MetricsSample(0.0, 0L, 0L) + override fun metrics(bundleId: String): MetricsSample = + MetricsSample(0.0, 0L, 0L) } backend.snapshot() assertEquals(listOf("hierarchy", "screenshot"), callOrder) @@ -88,7 +112,8 @@ class SnapshotHandlerTest { val maxObserved = AtomicInteger(0) val callCount = AtomicInteger(0) val lock = ReentrantLock() - val recordingBackend = object : DriverBackend by StubDriverBackend("android") { + val delegate = StubDriverBackend("android") + val recordingBackend = object : DriverBackend by delegate { override fun snapshot(): SnapshotSample { val now = inFlight.incrementAndGet() try { @@ -109,9 +134,13 @@ class SnapshotHandlerTest { // Use a real (multi-threaded) executor on the server side so the service // is not artificially serialized by directExecutor. val serverName = InProcessServerBuilder.generateName() - val service = DriverService(platform = "android", backend = recordingBackend) + val service = + DriverService(platform = "android", backend = recordingBackend) grpcCleanup.register( - InProcessServerBuilder.forName(serverName).addService(service).build().start(), + InProcessServerBuilder.forName(serverName) + .addService(service) + .build() + .start(), ) val channel: ManagedChannel = grpcCleanup.register( InProcessChannelBuilder.forName(serverName).build(), diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt index c8da826..29d8c79 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt @@ -13,7 +13,10 @@ class StabilityPollTest { elapsed >= MIN_STABLE_STREAK_MILLIS, "must observe a stable streak of at least ${MIN_STABLE_STREAK_MILLIS}ms, elapsed=${elapsed}ms", ) - assertTrue(elapsed < 3000L, "should not run to cap when stable, elapsed=${elapsed}ms") + assertTrue( + elapsed < 3000L, + "should not run to cap when stable, elapsed=${elapsed}ms", + ) } @Test fun slowSnapshotReadsDoNotEatTheStreak() { @@ -38,8 +41,9 @@ class StabilityPollTest { val observedQuiet = sampleStarts.last() - sampleEnds.first() assertTrue( observedQuiet >= MIN_STABLE_STREAK_MILLIS, - "the poll returned having observed only ${observedQuiet}ms of quiet, not " + - "${MIN_STABLE_STREAK_MILLIS}ms; starts=$sampleStarts ends=$sampleEnds", + "the poll returned having observed only ${observedQuiet}ms of " + + "quiet, not ${MIN_STABLE_STREAK_MILLIS}ms; " + + "starts=$sampleStarts ends=$sampleEnds", ) assertTrue( sampleStarts.size >= 3, @@ -60,23 +64,27 @@ class StabilityPollTest { calls++ when { calls <= 2 -> "calm" + calls == 3 -> { transientAt = System.currentTimeMillis() "transient" } + else -> "stable" } } val sinceTransition = System.currentTimeMillis() - transientAt assertTrue( calls >= 8, - "after the transition the poll needs a fresh matching pair and then a full " + - "${MIN_STABLE_STREAK_MILLIS}ms of quiet, which is 8 samples, got $calls", + "after the transition the poll needs a fresh matching pair and " + + "then a full ${MIN_STABLE_STREAK_MILLIS}ms of quiet, which " + + "is 8 samples, got $calls", ) assertTrue( sinceTransition >= MIN_STABLE_STREAK_MILLIS, - "the calm prefix must not count: a full ${MIN_STABLE_STREAK_MILLIS}ms streak has to " + - "start over after the transition, returned ${sinceTransition}ms after it", + "the calm prefix must not count: a full " + + "${MIN_STABLE_STREAK_MILLIS}ms streak has to start over " + + "after the transition, returned ${sinceTransition}ms after it", ) } @@ -105,7 +113,10 @@ class StabilityPollTest { "frame-$calls" } val elapsed = System.currentTimeMillis() - start - assertTrue(elapsed in budget..(budget + 1000L), "expected to hit cap, elapsed=$elapsed") + assertTrue( + elapsed in budget..(budget + 1000L), + "expected to hit cap, elapsed=$elapsed", + ) } @Test fun zeroBudgetReturnsImmediately() { @@ -117,7 +128,8 @@ class StabilityPollTest { assertEquals(0, calls) } - @Test fun structuralHashIgnoresBoundsAndIdenticalForSemanticallyEqualTrees() { + @Test + fun structuralHashIgnoresBoundsAndIdenticalForSemanticallyEqualTrees() { val a = """ {"attributes":{"resource-id":"LoginScreen","bounds":"[0,0,1080,2340]"}, "children":[ @@ -130,13 +142,24 @@ class StabilityPollTest { {"attributes":{"resource-id":"LoginEmail","bounds":"[10,11,1070,101]","text":"a@b"},"children":[]} ]} """.trimIndent() - assertEquals(structuralHash(a), structuralHash(b), "bounds-only flicker must not change hash") + assertEquals( + structuralHash(a), + structuralHash(b), + "bounds-only flicker must not change hash", + ) } @Test fun structuralHashDiffersWhenContentChanges() { - val a = """{"attributes":{"resource-id":"LoginEmail","text":"a@b"},"children":[]}""" - val b = """{"attributes":{"resource-id":"LoginEmail","text":"c@d"},"children":[]}""" - assertTrue(structuralHash(a) != structuralHash(b), "text change must alter hash") + val a = """ + {"attributes":{"resource-id":"LoginEmail","text":"a@b"},"children":[]} + """.trimIndent() + val b = """ + {"attributes":{"resource-id":"LoginEmail","text":"c@d"},"children":[]} + """.trimIndent() + assertTrue( + structuralHash(a) != structuralHash(b), + "text change must alter hash", + ) } @Test fun stabilitySnapshotReturnsNullDuringNavHostCrossFade() { @@ -160,7 +183,10 @@ class StabilityPollTest { ]} """.trimIndent() val hash = stabilitySnapshot(singleScreen) - assertTrue(hash != null && hash.isNotBlank(), "single-screen tree must yield a hash, got $hash") + assertTrue( + hash != null && hash.isNotBlank(), + "single-screen tree must yield a hash, got $hash", + ) } @Test fun stabilitySnapshotIgnoresNonRouteAttributeValues() { @@ -173,7 +199,10 @@ class StabilityPollTest { {"attributes":{"text":"Welcome to MyScreen"},"children":[]} ]} """.trimIndent() - assertTrue(stabilitySnapshot(tree) != null, "non-route attribute must not be counted as a screen") + assertTrue( + stabilitySnapshot(tree) != null, + "non-route attribute must not be counted as a screen", + ) } @Test fun countRouteScreensCountsTestTagAndIdentifier() { diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt index e3eb3d8..6062e41 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/WdaRecoveryTest.kt @@ -12,14 +12,15 @@ import kotlin.test.assertTrue class WdaRecoveryTest { - private fun recovery( - isAlive: () -> Boolean, - restart: () -> Unit, - ) = WdaRecovery(isAlive = isAlive, restart = restart, log = {}) + private fun recovery(isAlive: () -> Boolean, restart: () -> Unit) = + WdaRecovery(isAlive = isAlive, restart = restart, log = {}) @Test fun aliveChannelSkipsRestartAndRetriesReads() { val restarts = AtomicInteger(0) - val recovery = recovery(isAlive = { true }, restart = { restarts.incrementAndGet() }) + val recovery = recovery( + isAlive = { true }, + restart = { restarts.incrementAndGet() }, + ) var calls = 0 val result = recovery.run(replay = true) { @@ -35,10 +36,15 @@ class WdaRecoveryTest { @Test fun aliveChannelSurfacesUnavailableForActions() { val restarts = AtomicInteger(0) - val recovery = recovery(isAlive = { true }, restart = { restarts.incrementAndGet() }) + val recovery = recovery( + isAlive = { true }, + restart = { restarts.incrementAndGet() }, + ) val thrown = assertFailsWith { - recovery.run(replay = false) { throw IOException("connection reset") } + recovery.run(replay = false) { + throw IOException("connection reset") + } } assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) @@ -101,7 +107,9 @@ class WdaRecoveryTest { ) val thrown = assertFailsWith { - recovery.run(replay = true) { throw IOException("connection refused") } + recovery.run(replay = true) { + throw IOException("connection refused") + } } assertTrue(thrown.message.orEmpty().contains("WDA reconnect failed")) @@ -116,7 +124,9 @@ class WdaRecoveryTest { ) assertFailsWith { - recovery.run(replay = true) { throw IllegalArgumentException("bad selector") } + recovery.run(replay = true) { + throw IllegalArgumentException("bad selector") + } } assertEquals(0, restarts.get()) @@ -127,7 +137,9 @@ class WdaRecoveryTest { val recovery = recovery(isAlive = { true }, restart = {}) val thrown = assertFailsWith { - recovery.run(replay = true) { throw IOException("connection reset") } + recovery.run(replay = true) { + throw IOException("connection reset") + } } assertEquals(io.grpc.Status.Code.UNAVAILABLE, thrown.status.code) diff --git a/test/browser/browser_test.go b/test/browser/browser_test.go index 3cae9cd..b94732d 100644 --- a/test/browser/browser_test.go +++ b/test/browser/browser_test.go @@ -23,6 +23,7 @@ import ( "github.com/priyanshujain/sanderling/internal/bundler" "github.com/priyanshujain/sanderling/internal/driver" "github.com/priyanshujain/sanderling/internal/driver/chrome" + "github.com/priyanshujain/sanderling/internal/hierarchy" chromerunner "github.com/priyanshujain/sanderling/internal/runner" "github.com/priyanshujain/sanderling/internal/trace" "github.com/priyanshujain/sanderling/internal/verifier" @@ -222,3 +223,86 @@ func TestBrowserUndefinedExtractorStaysUndefined(t *testing.T) { t.Fatalf("nothing was ever tapped, so the property above held vacuously; violations=%v", violations) } } + +// One page, one selector, two hosts, two answers. +// +// The V8 host resolves state.ax.find against the live DOM; the goja host +// resolves the same selector against the hierarchy dump. The fixture puts a +// shadow-hosted #x above a light-DOM #x, the one shape where the two walks can +// disagree, and on web it is V8's answer that reaches the properties. Each host +// is driven through its production path (EvaluateExtractors in the page, +// PushSnapshot over the dump) and neither is asked what the other said, so the +// comparison is evidence rather than an assertion about one of them. +func TestBrowserAxFindAgreesAcrossHosts(t *testing.T) { + server := httptest.NewServer(http.FileServer(http.Dir(testdataDir(t)))) + t.Cleanup(server.Close) + + gojaBundle, webBundle := bundleSpec(t, filepath.Join(testdataDir(t), "find-order", "spec.ts")) + + driverInstance := chrome.New() + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = driverInstance.Terminate(ctx) + }) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + if err := driverInstance.Launch(ctx, server.URL+"/find-order/", false, nil); err != nil { + t.Fatalf("launch: %v", err) + } + if err := driverInstance.InstallBundle(ctx, webBundle); err != nil { + t.Fatalf("install web bundle: %v", err) + } + readings, err := driverInstance.EvaluateExtractors(ctx) + if err != nil { + t.Fatalf("evaluate extractors in the page: %v", err) + } + fromV8 := string(readings[0]) + + dump, err := driverInstance.Hierarchy(ctx) + if err != nil { + t.Fatalf("hierarchy: %v", err) + } + tree, err := hierarchy.Parse(dump) + if err != nil { + t.Fatalf("parse hierarchy: %v", err) + } + verifierInstance, err := verifier.New( + verifier.WithSeed(fixtureSeed), + verifier.WithPlatform("web"), + ) + if err != nil { + t.Fatalf("verifier: %v", err) + } + if err := verifierInstance.Load(string(gojaBundle)); err != nil { + t.Fatalf("load spec: %v", err) + } + if err := verifierInstance.PushSnapshot(verifier.SnapshotInput{Tree: tree}); err != nil { + t.Fatalf("push snapshot: %v", err) + } + if count := verifierInstance.ExtractorCount(); count != 1 { + t.Fatalf("the goja host registered %d extractors, want the fixture's 1", count) + } + // ChangedExtractors omits an extractor whose reading is null and unchanged, + // which is exactly what a selector resolving nothing produces. With the + // count checked above, an absent entry is a null reading and not a missing + // extractor, so reporting it as null names the real failure. + fromGoja := "null" + if change, ok := verifierInstance.ChangedExtractors()["found"]; ok { + fromGoja = string(change.Curr) + } + + // Pinned, not just compared: two hosts that both resolved nothing would + // agree on undefined and prove nothing about the walk. + if fromGoja != `"shadow"` { + t.Errorf("the goja host read %s off the dump, want the shadow-hosted %q", fromGoja, "shadow") + } + if fromV8 != fromGoja { + t.Fatalf( + "one page, one selector, two answers: the V8 host read %s and the goja host read %s", + fromV8, + fromGoja, + ) + } +} diff --git a/test/browser/testdata/find-order/index.html b/test/browser/testdata/find-order/index.html new file mode 100644 index 0000000..9e81425 --- /dev/null +++ b/test/browser/testdata/find-order/index.html @@ -0,0 +1,16 @@ + + + + +
+ light + + + diff --git a/test/browser/testdata/find-order/spec.ts b/test/browser/testdata/find-order/spec.ts new file mode 100644 index 0000000..2145321 --- /dev/null +++ b/test/browser/testdata/find-order/spec.ts @@ -0,0 +1,17 @@ +import { always, extract, taps } from "@sanderling/spec"; + +// Which of the two #x a selector means is the whole fixture. The reading is +// compared between the two hosts by TestBrowserAxFindAgreesAcrossHosts, which +// drives each host's production path and never asks one what the other said. +// +// Written in the object form on purpose. It used to reach the goja host as a +// plain attribute filter looking for an `id` attribute a dump never carries +// (the web dump files the DOM id under resource-id), so it resolved nothing +// there while the V8 host resolved it against the live DOM. +const found = extract((s) => s.ax.find({ id: "x" })?.text).named("found"); + +const findsTheShadowMatch = always(() => found.current === "shadow"); + +export const properties = { findsTheShadowMatch }; + +export const actionsRoot = taps;