refactor(move): move the runner's inline unit tests into UnitTests/ - #2854
Merged
Merged
Conversation
Every whole `#if AGENT_DEVICE_RUNNER_UNIT_TESTS` test block in a production runner source moves to `UnitTests/RunnerTests+<Source>Tests.swift` under the same guard, with the same test names and bodies. Production sources keep only the guarded test seams (overrides, injected failures, the recorder timestamp accessor, and RunnerTests' stored test properties). Declarations a moved test reads widen from private to internal. Fixture loaders that resolve contracts/fixtures from #filePath step up one more directory, as the existing UnitTests loaders do. Part of #2792 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… inline test RunnerTapPointPolicy.swift no longer declares a test, so the declaration scan and its test stop citing it as the file whose name hid one. Part of #2792 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Size Report
Startup median (7 runs, lower is better):
|
Member
Author
|
This is a pure move, and 61fcc69 checks out: I compared the multiset of test lines before and after, and the content is preserved. CI is green across all 18 checks, including the runner unit-test lanes and the xctest-selection check, which are the routes that matter here. I did not run the macOS host lane or the iOS build-for-testing myself, and the 233-test counts before and after come from the PR body and CI, not from a run I did. The multiset match proves line content survived the move, but it does not prove each test kept an equivalent guard (for example Not blocking: the PR says this is part of #2792 and the CommandExecution split under 1,000 lines (apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift) is still to do, so keep #2792 open for that follow-up, but that's optional to act on now. Nothing here stops this from merging. |
thymikee
added
the
ready-for-human
thymikee
added this pull request to stack #2859
September 24, 2026 05:43
|
thymikee
added a commit
that referenced
this pull request
Sep 24, 2026
main (#2854, #2855) moved the runner's inline unit tests into UnitTests/ and split RunnerTests+CommandExecution.swift along command families. Rebase conflicts dropped this PR's edits from the inline blocks the rebase kept on main; relocate them into the file main now owns for each family: - RunnerTests+SnapshotTests.swift: the query-sweep slice-deadline and fail-closed-invalidation tests, and the bounded-modal-probe helper now takes a SnapshotCaptureTarget. - RunnerTests+SnapshotCapturePlanTests.swift: the tier-timeout rejection pair and the private-AX depth helper now builds a SnapshotCaptureTarget. - RunnerTests+SnapshotTimingTests.swift: the deadline-exhausted tier penalty test.
thymikee
added a commit
that referenced
this pull request
Sep 24, 2026
…e query sweep by its slice (#2836) * fix(ios): take snapshot target identity on main and hop plan state writes The capture plan runs on the command queue but read currentBundleId, currentAppProcessIdentifier and the penalty warm-up exemption, and wrote runnerAccessibilityHealth and invalidated the cached target directly. Snapshot preparation now takes a SnapshotCaptureTarget on main (app, bundle id, pid, consumed warm-up exemption) and the plan, the modal probe penalty, the XCTest penalty recorder and the private AX tier read only that copy. Health writes and the fail-closed invalidation go through applyMainOwnedSnapshotState, the abandoned-work-guarded main hop that invalidateCachedTargetAfterSnapshotFailure now also uses. Fixes #2781 * fix(ios): bound the query sweep by its tier slice deadline A non-interactive query sweep used Date.distantFuture as its own deadline, so only the up-to-20 s plan deadline bounded it while the caller abandoned the tier after its 1 s slice. XCTest queries cannot be cancelled, so the sweep kept the main thread busy and later commands got RUNNER_BUSY. The caller now derives one slice deadline (querySweepSliceDeadline) for both its wait and the sweep, for interactive and non-interactive requests alike. Neither the element queries nor the per-element reads start with less than flatInteractiveQueryBudget left before that deadline (querySweepCanStartQuery). Fixes #2783 * fix(ios): spend the penalty warm-up exemption only in a capture plan that runs Taking the snapshot target on main consumed the exemption during command preparation, so a snapshot answered by the blocking system-modal probe, or an abandoned preparation that ran late, spent it without running a plan. The exemption now lives in a lock-owned type that lifecycle code arms on main and runSnapshotCapturePlan consumes at its start. * test(ios): pin the non-interactive query sweep to its tier slice through the plan Swizzles XCUIElementQuery.allElementsBoundByIndex with a slow query and runs a non-interactive [.querySweep] plan with a 20 s plan deadline. The sweep must not be abandoned, and no query may start after the plan answers, so passing the plan deadline to the sweep in either loop turns it red. * test(ios): pin the query-sweep tier timeout and the tap routing identity to main Two regressions #2781 still leaves open, both red on this head: A query sweep that stops at its own slice deadline is a tier timeout, but the plan classified its payload by node count, so a partial sweep that beat the sparse threshold armed no XCTest-channel penalty and no later capture of the same screen was deferred. The coordinate tap's system-modal routing probe armed its penalty with currentBundleId read on the command queue, while main owned that state and could still be clearing or rebinding it. * fix(ios): count a query sweep that ends on its slice deadline as a tier timeout The sweep tier stopped starting queries at its slice deadline, but the plan classified only what it collected. sparsePayloadReason accepts anything above the sparse node threshold, so a partial flat sweep became a `recovered` capture: no XCTest-channel penalty was armed, private AX never ran, and every later capture of that screen paid for the full sweep again. `runFlatInteractiveQueries` now returns a typed `SnapshotTierOutcome`, the sweep tier carries it into the attempt, and the plan rejects a `.deadlineExhausted` tier through `snapshotTierRejectionReason` — keeping its payload only as the fallback, arming the penalty through the existing tier timeout reason, and letting the next backend answer. * fix(ios): take the system-modal probe's target identity on main The coordinate tap resolved its system-modal routing on the command queue and armed an abandoned probe's penalty with `currentBundleId` read there, while `applyMainOwnedSnapshotState` and the lifecycle code write that identity through `DispatchQueue.main.async`. A tap could penalize a target main was still clearing or rebinding. The probe now takes a `SnapshotProbePenaltyTarget`: a capture hands over the identity it already took on main, and the tap route hands over nothing and lets the probe's own main-side block capture the identity main holds once its work starts. No off-main path reads target identity any more. * test(ios): place this PR's runner tests in main's split UnitTests files main (#2854, #2855) moved the runner's inline unit tests into UnitTests/ and split RunnerTests+CommandExecution.swift along command families. Rebase conflicts dropped this PR's edits from the inline blocks the rebase kept on main; relocate them into the file main now owns for each family: - RunnerTests+SnapshotTests.swift: the query-sweep slice-deadline and fail-closed-invalidation tests, and the bounded-modal-probe helper now takes a SnapshotCaptureTarget. - RunnerTests+SnapshotCapturePlanTests.swift: the tier-timeout rejection pair and the private-AX depth helper now builds a SnapshotCaptureTarget. - RunnerTests+SnapshotTimingTests.swift: the deadline-exhausted tier penalty test.
Member
Author
|
Follow-up on the guard question: yes. I compared each test's full The earlier review had cross-checked this a second way. Running the repo's own #2792 is now closed by #2855. 🤖 Generated with Claude Code |
22 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of #2792. Based on main: the #2825–#2845 stack merged, and main
fa3b7291fhas the same tree as60e533284.Pure move: every whole
#if AGENT_DEVICE_RUNNER_UNIT_TESTStest block in a production runner source moves toUnitTests/RunnerTests+<Source>Tests.swiftunder the same guard, with the same test names and bodies. This includes the blocks the rebased base added inRunnerTests+Lifecycle.swiftandRunnerTests+SynthesizedGesturePolicy.swift. Production files keep only test seams: overrides, injected failures, the recorder timestamp accessor, and stored test properties. The move widens 20 declarations fromprivatetointernal, each read by a moved test. Five fixture loaders step up one more directory. Comments that pointed at the old test locations now point at the new ones. The finalchore(gates)commit rewords the declaration-scan comments. 54 files.git diff -M90% --stat origin/main...HEAD: 54 files, +4088/−4033. A removed-vs-added line multiset differs only inprivate→internal, imports,extension RunnerTests {wrappers, and the five// UnitTestspath steps.Validation
Tested at
5785e206b..61fcc6969(tip61fcc6969, tree-identical to the originally testedea85fd32e):60e533284) and 233, 0 failures after.pnpm check:xctest-selection: host 233, iOS PR list 103, nightly 280, 0 unreachable, green.pnpm check:packaged-runner-swift: green.pnpm check:affected --run: all runnable checks passed. It selected the full set because of the workflow and tooling paths.build-for-testingwith the unit-test flag: succeeded.func testnames: 285 before and after, with no duplicates.An adversarial fresh-context review tried to refute the pure move and confirmed test bodies, guards, lane reach, and every access widening. It found three stale comments, which are now fixed.
🤖 Generated with Claude Code