diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 015540029..452a80fbb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,14 +55,41 @@ jobs: exit 1 fi - swift-runner-unit-compile: - name: Swift Runner Unit Compile + # The runner's pure-decision XCTests, on the macOS host with no simulator (#1781 A7). This + # job already compiled the macOS runner with the unit-test flag on every PR (as "Swift + # Runner Unit Compile") and threw the bundle away; running it costs one + # `test-without-building` on the host. Which tests that reaches is not a list: it is + # everything the macOS build compiles, and the simulator-only tests keep themselves out + # with an `os(iOS)` guard (see the classification note in RunnerTests.swift). The + # `-skip-testing:` entry is validated by `pnpm check:xctest-selection` for the same + # reason the nightly's is: `RunnerTests/testCommand` is the runner's 24-hour server + # entry point, and a typo re-arms the hang. The lane's reporter asserts the executed + # count equals the reach the check derives from source, so a build without the compile + # flag or a guard that compiles a file out reads as red rather than as a smaller green. + # + # - Catches: regressions in the ~130 host-runnable tests on every PR, in about a + # minute, independent of ios.yml — which is fifteen minutes long, cancelled ~19% of + # the time, and names only a subset of them. + # - Evidence: the nightly's first execution found #1812 in exactly this class of test + # (a geometry guard); the same tests pass on this lane's platform. + # - Cost: ~1 minute more on a job that already occupies a macOS slot for the build; + # no simulator boot; no new job. + # - Kill criterion: if the host run proves flaky where the simulator run of the same + # tests is not (a platform-branch difference this classification missed), gate the + # offender `os(iOS)` and it returns to the simulator lanes; if that happens to more + # than a handful, the host lane is the wrong tool and goes. + swift-runner-host-tests: + name: Swift Runner Host XCTests runs-on: macos-26 timeout-minutes: 20 # Was an inline `VAR=1 pnpm gate …` prefix on the action's build-command input. The # input is a gate id now, so the variable lives where ios.yml already puts it: the job. env: AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS: '1' + AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH: ${{ github.workspace }}/.tmp/swift-runner-unit-derived + RESULT_BUNDLE_PATH: ${{ github.workspace }}/.tmp/xctest-host/RunnerTests.xcresult + RESULT_SUMMARY_PATH: ${{ github.workspace }}/.tmp/xctest-host/test-results-summary.json + XCTEST_LANE: host steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -73,12 +100,58 @@ jobs: - name: Restore and compile Swift runner unit-test surface uses: ./.github/actions/setup-apple-runner-build with: - derived-path: ${{ github.workspace }}/.tmp/swift-runner-unit-derived + derived-path: ${{ env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH }} cache-key-prefix: swift-runner-unit gate: swift-runner-macos xcuitest-platform: macos xcuitest-destination: platform=macOS,arch=arm64 + - name: Run the host-runnable runner XCTests + run: | + set -euo pipefail + XCTESTRUN_PATH="$(find "$AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH/Build/Products" -maxdepth 1 -name '*.xctestrun' -print -quit)" + test -n "$XCTESTRUN_PATH" + mkdir -p "$(dirname "$RESULT_BUNDLE_PATH")" + xcodebuild test-without-building \ + -xctestrun "$XCTESTRUN_PATH" \ + -destination 'platform=macOS,arch=arm64' \ + -skip-testing:AgentDeviceRunnerUITests/RunnerTests/testCommand \ + -resultBundlePath "$RESULT_BUNDLE_PATH" + + # Same shape as xctest-nightly.yml: extract is best-effort, the report step asserts. + - name: Extract the test-results summary + if: always() + run: | + set -uo pipefail + [ -d "$RESULT_BUNDLE_PATH" ] || exit 0 + xcrun xcresulttool get test-results summary --path "$RESULT_BUNDLE_PATH" --compact \ + > "$RESULT_SUMMARY_PATH" 2>/dev/null || + xcrun xcresulttool get test-results summary --path "$RESULT_BUNDLE_PATH" \ + > "$RESULT_SUMMARY_PATH" 2>/dev/null || + rm -f "$RESULT_SUMMARY_PATH" + exit 0 + + - name: Report the run and assert it executed the source-derived set + if: always() + run: | + set -euo pipefail + if [ ! -s "$RESULT_SUMMARY_PATH" ]; then + echo 'No usable test-results summary was produced, so nothing here can show a test ran.' + echo 'The suite step above failed before or during the run; read its log first.' + echo '### iOS runner host XCTest lane' >> "$GITHUB_STEP_SUMMARY" + echo 'No result-bundle summary; see the run log and the uploaded bundle.' >> "$GITHUB_STEP_SUMMARY" + exit 1 + fi + node --experimental-strip-types scripts/xctest-run-summary.ts + + - name: Upload result bundle + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: xctest-host-results-${{ github.run_id }}-${{ github.run_attempt }} + path: .tmp/xctest-host + if-no-files-found: warn + lint: name: Lint & Format runs-on: ubuntu-latest diff --git a/.github/workflows/xctest-nightly.yml b/.github/workflows/xctest-nightly.yml index 52b74da71..4af521cff 100644 --- a/.github/workflows/xctest-nightly.yml +++ b/.github/workflows/xctest-nightly.yml @@ -15,22 +15,27 @@ name: XCTest Nightly # # Under the lane rule in #1781: # -# - Catches: regressions in the runner-semantics tests the PR list leaves out — selector -# matching, snapshot traversal, command journal, sequence execution, tv remote, keyboard, -# scroll/gesture policy. Nothing else executes them. Note the reachable set is smaller -# than the declared set: `testCommand` is skipped, and the `…OnTvOS` tests sit behind -# `#if os(tvOS)` and cannot execute on this lane's iOS Simulator destination. -# - Evidence: the list has never been re-derived since it was written by hand; the tests it -# omits have been dark for their whole life, so their current state is unmeasured. The -# first red night is the evidence this lane exists to produce. +# - Catches: regressions in the simulator-only tests (guard `… && os(iOS)`: they launch +# the host app, route through SpringBoard, or assert an iOS-only branch) that ios.yml's +# PR list leaves out. Since the A7 classification the pure-decision tests also run on +# the macOS host lane on every PR (ci.yml "Swift Runner Host XCTests"), so this lane's +# unique reach is the simulator-only remainder — `pnpm check:xctest-selection` prints +# both numbers. `testCommand` is skipped, and the tests guarded to macOS/tvOS only +# cannot execute on this lane's iOS Simulator destination (the host lane runs them). +# - Evidence: the first execution found #1812 in the dark set (2 of 155 red, both real); +# the other ~114 never-run tests all passed. # - Cost: this is a public repository, so nothing here is billed. The real cost is one macOS # concurrency slot per night. The Apple runner build shares ios.yml's cache (identical # inputs and build variant), but scheduled runs only see DEFAULT-BRANCH caches, so the hit # depends on ios.yml's `push: main` run having populated the cache at the same source # hash — on a miss this lane also pays a full runner build. Adds nothing to any PR. -# - Kill criterion: when A7's classification lands — pure Swift decision tests moved to a -# fast unit target, stale tests deleted, the rest kept on the simulator — this lane goes -# if the surviving set is small enough to run on every PR. +# - Kill criterion: what ONLY this lane reaches is 11 tests / ~42 s of simulator time +# (measured from the first night's xcresult) — the app-launching, SpringBoard-probing +# class. Once ios.yml runs the whole iOS-compiled bundle instead of its hand-written +# list — the follow-up stacked on the classification — every test reaches a PR lane and +# this lane goes. That trade is not free: those 11 are the flakiest tests in the suite, +# and they would land on a blocking 15-minute gate that is already cancelled ~19% of the +# time, so measure their flake rate here before moving them. # # Scheduled + manual only. It is deliberately NOT a PR gate: ios.yml is already over half of # this repo's CI spend (#1781 A9), and moving the unrun set onto it would make that worse for @@ -79,6 +84,7 @@ jobs: AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH: ${{ github.workspace }}/.tmp/ios-runner-derived RESULT_BUNDLE_PATH: ${{ github.workspace }}/.tmp/xctest-nightly/RunnerTests.xcresult RESULT_SUMMARY_PATH: ${{ github.workspace }}/.tmp/xctest-nightly/test-results-summary.json + XCTEST_LANE: nightly steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -155,10 +161,12 @@ jobs: # Reports the run AND is the lane's own liveness check. `xcodebuild` exits 0 when a # selection matches nothing, so "green" and "ran no tests" are the same log tail: a # build variant without the unit-test compile flag, an empty test plan, or a renamed - # target would all read as a healthy night. Asserting the run executed tests is what - # tells those apart. The failure list is capped so a badly red night cannot blow the - # 1 MiB job-summary limit and lose the headline with it. - - name: Report the run and assert it executed tests + # target would all read as a healthy night. The reporter asserts the executed count + # equals what `pnpm check:xctest-selection` derives this lane reaches from the `#if` + # guards (XCTEST_LANE above), so a guard that compiles a file of tests out of iOS reads + # as red too. The failure list is capped so a badly red night cannot blow the 1 MiB + # job-summary limit and lose the headline with it. + - name: Report the run and assert it executed the source-derived set if: always() run: | set -euo pipefail diff --git a/AGENTS.md b/AGENTS.md index 4d2f97db8..9fca306a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,9 @@ the new thing — never to suppress or allowlist it. - every command declares a timeout policy on its descriptor (timeout-policy completeness test) - TS/Swift rule parity: golden tables under `contracts/fixtures/`, consumed by vitest and the gated XCTest — change the rule only via the table +- runner XCTest reach: `check:xctest-selection` evaluates each test's `#if` guard per platform and + fails when a declared method reaches no lane (host/PR/nightly) — classify with the guard, do not + list by hand - cross-command apple-leak guard; folder DAG/import lint (zero value-import cycles, zero target-spine back-edges); fallow (dead code, duplication, complexity) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+BlockingSystemModalResolution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+BlockingSystemModalResolution.swift index 04fd87e12..2b92c09bd 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+BlockingSystemModalResolution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+BlockingSystemModalResolution.swift @@ -95,19 +95,23 @@ extension RunnerTests { XCTAssertFalse(RemoteHostedSystemModalPolicy.isEligibleHostState(.unknown)) } - // tvOS has no SpringBoard host, so both the snapshot and alert-resolution paths - // must resolve without probing com.apple.springboard (#1351). - #if os(tvOS) - func testResolveBlockingSystemModalIsAbsentWithoutSpringBoardOnTvOS() { + // No SpringBoard host (`hasSpringBoardSystemModalHost`) means modal resolution must return + // `.absent` without probing com.apple.springboard (#1351). Written for tvOS, where no lane + // ever executed it; `resolveBlockingSystemModal` takes that decision at RUNTIME off the same + // flag on macOS, so the host lane runs the real branch on every PR. + // + // Its former sibling `testBlockingSystemAlertSnapshotIsNilOnTvOS` is deleted rather than + // widened: `blockingSystemAlertSnapshot` is `#if os(macOS) return nil`, so on the only lane + // that could run it the assertion would pin a compile-time literal — a green that no change + // to the runner could turn red. The runtime gate it meant to cover is this test's subject, + // and the nil it returns on macOS is the compiler's business, not a test's. + #if os(tvOS) || os(macOS) + func testResolveBlockingSystemModalIsAbsentWithoutSpringBoardHost() { guard case .absent = resolveBlockingSystemModal(deadline: .distantFuture) else { - XCTFail("tvOS blocking system-modal resolution must be .absent") + XCTFail("blocking system-modal resolution must be .absent without a SpringBoard host") return } } - - func testBlockingSystemAlertSnapshotIsNilOnTvOS() { - XCTAssertNil(blockingSystemAlertSnapshot(deadline: .distantFuture)) - } #endif } #endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index 9af70637b..a1a3f9e8c 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -481,6 +481,11 @@ extension RunnerTests { XCTAssertNil(xctestRecordedFailureResponse(command: tapCommand, response: runnerFatalResponse)) } + // Simulator-only from here to the matching #endif: these launch the host app, route through + // SpringBoard, or assert the iOS-only alert/system-modal branches. Tests outside the + // `os(iOS)` regions in this file are pure runner decisions and also run on the macOS host + // lane (ci.yml) — see the classification convention in RunnerTests.swift. +#if os(iOS) func testMissingBundleCommandInvalidatesCompleteCachedTargetState() throws { app.launch() currentApp = app @@ -681,6 +686,10 @@ extension RunnerTests { XCTAssertFalse(shouldSkipAppActivationPreflight(mixedSequence)) } + // Launches nothing, but still simulator-only: `shouldSkipAppActivationPreflight` is + // `#if os(iOS) …guards… #else return false #endif`, so on macOS this asserts a compile-time + // literal and no edit to the iOS body could make it red. Its five siblings above and below + // are gated for the same reason. func testSkipAppActivationPreflightRequiresCachedForegroundTarget() throws { currentApp = nil currentBundleId = nil @@ -727,6 +736,7 @@ extension RunnerTests { XCTAssertTrue(shouldSkipAppActivationPreflight(alert)) } +#endif func testExecuteDispatchedReturnsBusyBeforeMainThreadFastPath() throws { let command = try runnerCommandFixture(#"{"command":"snapshot","commandId":"snapshot-busy"}"#) @@ -760,6 +770,7 @@ extension RunnerTests { XCTAssertTrue(response.error?.hint?.contains("runner session will be restarted") == true) } +#if os(iOS) func testAlertResolutionCannotBypassRequestedDeadline() throws { final class ResultBox { var error: Error? @@ -821,6 +832,7 @@ extension RunnerTests { releaseResolution.signal() wait(for: [resolutionExited], timeout: 1) } +#endif func testRunMainThreadWorkExecutesOffMainCallerOnMainThread() { final class ResultBox { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift index 5e49549e3..a2a36c835 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Snapshot.swift @@ -616,6 +616,11 @@ extension RunnerTests { XCTAssertEqual(payload.truncated, true) XCTAssertEqual(payload.runnerFatal, true) XCTAssertEqual(payload.runnerFatalReason, Self.axSnapshotUnavailableReason) + // The planned terminal result carries the structured verdict like every other planned + // snapshot — downstream sparse handling keys off it, not off node shapes. + XCTAssertEqual(payload.snapshotQuality?.state, "sparse") + XCTAssertEqual(payload.snapshotQuality?.reasonCode, "ax-rejected") + XCTAssertEqual(payload.snapshotQuality?.reason, Self.axSnapshotFailureMessage) XCTAssertNil(currentApp) XCTAssertNil(currentBundleId) } @@ -651,6 +656,9 @@ extension RunnerTests { XCTAssertEqual(Self.systemModalProbeSlice(budget: 4, deadlineRemaining: -5), 0) } + // Simulator-only: the bounded probe body returns nil on macOS (no SpringBoard host), so the + // timeout/penalty/drain machinery below only exists on the iOS branch. +#if os(iOS) /// Regression for #1244/#1248: drives the bounded system-modal probe through a real, /// production-only command entry point (`snapshotFast` or `snapshotRaw` -- see the two test /// methods below), not `boundedBlockingSystemAlertSnapshot` directly, with @@ -783,6 +791,7 @@ extension RunnerTests { try self.snapshotRaw(app: target, options: options) } } +#endif func testDispatchRecoverySkipsBookkeepingWhileXCTestChannelOccupied() { // The #1244 recovery shape: the modal probe abandoned an XCTest query that is still grinding on diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift index 603286ffe..a28db1dfe 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+SnapshotCapturePlan.swift @@ -966,24 +966,5 @@ extension RunnerTests { ) ) } - - func testSnapshotAccessibilityUnavailableCarriesSparseVerdict() { - currentApp = app - currentBundleId = "com.example.app" - defer { - currentApp = nil - currentBundleId = nil - } - let payload = snapshotAccessibilityUnavailable( - failure: SnapshotCaptureFailure( - code: "IOS_AX_SNAPSHOT_FAILED", - message: "kAXErrorIllegalArgument", - hint: "use screenshot" - ) - ) - XCTAssertEqual(payload.runnerFatal, true) - XCTAssertEqual(payload.snapshotQuality?.state, "sparse") - XCTAssertEqual(payload.snapshotQuality?.reasonCode, "ax-rejected") - } } #endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift index babe904ce..18de2d408 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift @@ -119,6 +119,17 @@ final class RunnerTests: XCTestCase { // Bounds the pre-plan SpringBoard system-modal probe, which can otherwise grind for tens of // seconds on remote-hosted consent dialogs and bypass the plan budget (#1244). let systemModalProbeBudget: TimeInterval = 4 + // In-bundle unit tests (every `func test…` except `testCommand` below) compile only under + // `-D AGENT_DEVICE_RUNNER_UNIT_TESTS` and are classified by their `#if` guard (#1781 A7): + // - `#if AGENT_DEVICE_RUNNER_UNIT_TESTS` alone: a pure runner decision (rule table, + // geometry, parser, policy, journal, dispatch bookkeeping) that needs no launched app. + // Runs on the macOS host lane on every PR (ci.yml, no simulator) and on the iOS lanes. + // - `… && os(iOS)` (or a nested `#if os(iOS)`): runner/XCTest semantics — launches the + // host app, routes through SpringBoard, swizzles XCUIApplication, or asserts an + // iOS-only branch. Simulator lanes only (ios.yml PR list, xctest-nightly.yml). + // `pnpm check:xctest-selection` derives each lane's reachable set from these guards and + // fails when a declared test is reachable by no lane, so a test gated to a platform nothing + // runs (the old tvOS-only pair) cannot go dark silently. #if AGENT_DEVICE_RUNNER_UNIT_TESTS // #1605 merge gate: deterministic live reproduction of the field ambiguity — // a tap whose coordinate activation LANDS while XCTest bookkeeping records a diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 9bd5f7ef8..44d3e0723 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -37,7 +37,7 @@ The mapping it encodes, for when you need to run a gate directly or reason about | Platform/device response — anything emitting `platform`/`appleOs` on the wire, or shaping a daemon response | `pnpm test:integration:provider` **and** `pnpm test:coverage` | | Cross-platform behavior | `pnpm test:integration` | | Apple runner / Swift | Build the changed target with `pnpm build:xcuitest:`; use `pnpm build:xcuitest` only for shared iOS/macOS changes | -| Runner XCTest methods (`apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/**`) | `pnpm check:xctest-selection`, which prints the live declared/PR-selected/skipped/nightly-only split — the counts move often enough that quoting one here would rot. `ios.yml` runs a hand-written subset through an `-only-testing:` list and `xctest-nightly.yml` excludes one method via `-skip-testing:`; `xcodebuild` treats an identifier matching nothing as an empty selection rather than an error, in both directions — a rename drops a test from the PR lane silently, or re-admits `RunnerTests/testCommand`, the runner's 24-hour server entry point, into the nightly and hangs it. The check fails only on a listed name no source declares; a test in no list is expected, because the nightly runs the suite whole | +| Runner XCTest methods (`apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/**`) | `pnpm check:xctest-selection`, which prints how many methods each lane reaches — the counts move often enough that quoting one here would rot. Three lanes run the bundle, and a test's `#if` guard is its classification (the convention is written next to the flag in `RunnerTests.swift`): `#if AGENT_DEVICE_RUNNER_UNIT_TESTS` alone marks a pure runner decision, which the macOS host lane (`ci.yml` "Swift Runner Host XCTests", no simulator) runs on every PR; `… && os(iOS)` (or a nested `#if os(iOS)`) marks runner/XCTest semantics — launches the host app, routes through SpringBoard, asserts an iOS-only branch — which only the simulator lanes reach (`ios.yml`'s hand-written `-only-testing:` list on PRs, `xctest-nightly.yml` whole). The check evaluates the guards per platform, so it fails on a listed name no source declares or that lane's platform never compiles, on a declared test no lane reaches (a guard naming a platform nothing runs — the state two tvOS-only tests sat in), and on `RunnerTests/testCommand` — the runner's 24-hour server entry point, not a test — reaching any lane; `xcodebuild` treats an identifier matching nothing as an empty selection rather than an error, in both directions, so a renamed `-skip-testing:` entry would otherwise re-admit it silently. The host and nightly lanes assert their executed count equals the reach the check derives (`scripts/xctest-run-summary.ts`), so a build without the `-D` flag or a guard that compiles a file out reads as red rather than as a smaller green. To run the host set locally: build macOS with `AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS=1 pnpm build:xcuitest:macos`, then `xcodebuild test-without-building -xctestrun /Build/Products/*.xctestrun -destination 'platform=macOS,arch=arm64' -skip-testing:AgentDeviceRunnerUITests/RunnerTests/testCommand`. Two local-only snags CI does not hit. (1) If system policy refuses to load the unsigned bundle (`library load disallowed by system policy`, surfacing as `Early unexpected exit … crashed with signal kill`), rebuild it signed — but the incantation is machine-dependent, so try both: `CODE_SIGN_IDENTITY="Apple Development"` on a Mac with automatic signing configured, and the certificate's SHA-1 from `security find-identity -v -p codesigning` plus `CODE_SIGN_STYLE=Manual DEVELOPMENT_TEAM=` where the generic name resolves to "Mac Development" and fails. The wrong one of the two fails at signing, not silently. (2) The first run needs XCUITest automation permission for the host. GitHub's macOS runners need neither — they load the unsigned bundle as built | | CLI help/guidance (`src/cli-schema/cli-help.ts`, `src/cli-schema/`) | `pnpm exec vitest run src/cli-schema src/cli/parser/__tests__ scripts/__tests__` — the `scripts/__tests__` gates enforce help-topic benchmark coverage and pin the bench's quoted CLI samples to the real renderers | | Help benchmark cases (`scripts/help-conformance-*.mjs`) | `pnpm exec vitest run scripts/__tests__` (deterministic gates); model-backed: `pnpm bench:help-conformance` (paid LLM calls, local only) | | `.ad` grammar (`src/replay/script.ts`, gesture arity, replay vars) | `pnpm exec vitest run --project unit-core test/replay-compat` — the frozen replay-compat corpus asserts which released script surfaces still parse; a flipped verdict is edited in `test/replay-compat/manifest.ts`, never in the script. Adding or re-pinning a corpus entry also runs `pnpm check:replay-compat`, which re-derives each entry from its release tag in git history | diff --git a/scripts/__tests__/swift-conditional-compilation.test.ts b/scripts/__tests__/swift-conditional-compilation.test.ts new file mode 100644 index 000000000..685779c12 --- /dev/null +++ b/scripts/__tests__/swift-conditional-compilation.test.ts @@ -0,0 +1,69 @@ +// The `#if` evaluator decides which platform compiles which test, so a wrong answer here +// re-buckets a test silently — which is why it refuses vocabulary it does not know instead of +// guessing, and why that refusal is asserted below alongside the precedence and nesting cases. + +import { describe, expect, test } from 'vitest'; +import { + activeSource, + evaluateCondition, + PLATFORMS, + UnsupportedConditionError, + type Platform, +} from '../swift-conditional-compilation.ts'; + +describe('the #if evaluator', () => { + test.each([ + ['AGENT_DEVICE_RUNNER_UNIT_TESTS', { iOS: true, macOS: true, tvOS: true }], + ['os(iOS)', { iOS: true, macOS: false, tvOS: false }], + ['!os(macOS)', { iOS: true, macOS: false, tvOS: true }], + ['AGENT_DEVICE_RUNNER_UNIT_TESTS && os(iOS)', { iOS: true, macOS: false, tvOS: false }], + ['os(tvOS) || os(macOS)', { iOS: false, macOS: true, tvOS: true }], + ['os(macOS) || os(tvOS) || os(visionOS)', { iOS: false, macOS: true, tvOS: true }], + ['canImport(UIKit)', { iOS: true, macOS: false, tvOS: true }], + ['canImport(AppKit)', { iOS: false, macOS: true, tvOS: false }], + ['os(iOS) && targetEnvironment(simulator)', { iOS: true, macOS: false, tvOS: false }], + ['!(os(iOS) || os(tvOS)) // desktop', { iOS: false, macOS: true, tvOS: false }], + ] as const)('%s', (condition, expected) => { + for (const platform of PLATFORMS) { + expect(evaluateCondition(condition, platform)).toBe(expected[platform]); + } + }); + + test('refuses vocabulary it does not know rather than guessing', () => { + for (const condition of ['swift(>=5.9)', 'canImport(SwiftUI)', 'RELEASE', 'os(iOS) &&']) { + expect(() => evaluateCondition(condition, 'iOS')).toThrow(UnsupportedConditionError); + } + expect(() => activeSource('#if compiler(>=6)\nx\n#endif\n', 'iOS', 'F.swift')).toThrow( + /F\.swift:1/, + ); + }); + + test('blanks inactive branches, follows #elseif/#else, and nests', () => { + const text = [ + '#if os(iOS)', + 'ios', + '#elseif os(tvOS)', + 'tv', + '#else', + 'other', + ' #if AGENT_DEVICE_RUNNER_UNIT_TESTS', + ' nested', + ' #endif', + '#endif', + 'always', + ].join('\n'); + const kept = (platform: Platform) => + activeSource(text, platform) + .split('\n') + .filter((line) => line.trim() !== ''); + expect(kept('iOS')).toEqual(['ios', 'always']); + expect(kept('tvOS')).toEqual(['tv', 'always']); + expect(kept('macOS')).toEqual(['other', ' nested', 'always']); + }); + + test('a nested #if under an inactive branch stays inactive whatever it says', () => { + const text = '#if os(iOS)\n#if os(macOS) || !os(macOS)\ninner\n#endif\n#endif\n'; + expect(activeSource(text, 'macOS').trim()).toBe(''); + expect(activeSource(text, 'iOS').trim()).toBe('inner'); + }); +}); diff --git a/scripts/__tests__/xctest-declarations.test.ts b/scripts/__tests__/xctest-declarations.test.ts new file mode 100644 index 000000000..a56e7eb4e --- /dev/null +++ b/scripts/__tests__/xctest-declarations.test.ts @@ -0,0 +1,86 @@ +// The declaration scan decides what "declared" means — which `func test…` is addressable as +// `Target/Class/method`, and for which platforms — so both halves are proven here: the shapes +// no filter could address, and the per-platform attribution that turns a `#if` guard into a +// lane classification. + +import { describe, expect, test } from 'vitest'; +import { parseDeclaredTests, parseDeclaredTestsByPlatform } from '../xctest-declarations.ts'; + +const TARGET = 'AgentDeviceRunnerUITests'; + +function source(text: string) { + return [{ file: 'RunnerTests+Fixture.swift', text }]; +} + +describe('the declaration scan', () => { + test('binds a method to the top-level type that encloses it', () => { + expect( + parseDeclaredTests( + TARGET, + source( + 'final class RunnerTests: XCTestCase {\n func testInClass() {}\n}\n\n' + + 'extension RunnerTests {\n func testInExtension() throws {}\n}\n\n' + + 'final class OtherTests: XCTestCase {\n func testElsewhere() async throws {}\n}\n', + ), + ), + ).toEqual([ + `${TARGET}/OtherTests/testElsewhere`, + `${TARGET}/RunnerTests/testInClass`, + `${TARGET}/RunnerTests/testInExtension`, + ]); + }); + + test('ignores declarations no filter could address', () => { + expect( + parseDeclaredTests( + TARGET, + source( + 'extension RunnerTests {\n' + + // A helper type declared inside a test body must not capture the methods after + // it, and `class func` must not read as a type declaration named `func`. + ' class func makeHelper() {}\n' + + ' func testWithNestedHelper() {\n' + + ' final class ResultBox {}\n' + + ' func testLocal() {}\n' + + ' }\n' + + ' // func testCommentedOut() {}\n' + + ' func testAfterNesting() {}\n' + + '}\n', + ), + ), + ).toEqual([ + `${TARGET}/RunnerTests/testAfterNesting`, + `${TARGET}/RunnerTests/testWithNestedHelper`, + ]); + }); + + test('reads a file whose name does not start with RunnerTests', () => { + // RunnerTapPointPolicy.swift is the real instance: the synchronized-root-group project + // compiles every .swift in the directory, so file naming carries no membership meaning. + expect( + parseDeclaredTests(TARGET, [ + { + file: 'RunnerTapPointPolicy.swift', + text: 'extension RunnerTests {\n func testGolden() {}\n}\n', + }, + ]), + ).toEqual([`${TARGET}/RunnerTests/testGolden`]); + }); + + test('attributes each declared method to the platforms that compile it', () => { + expect( + parseDeclaredTestsByPlatform( + TARGET, + source( + 'extension RunnerTests {\n#if AGENT_DEVICE_RUNNER_UNIT_TESTS\n func testPure() {}\n' + + '#if os(iOS)\n func testSim() {}\n#endif\n#if os(tvOS) || os(macOS)\n' + + ' func testNoSpringBoard() {}\n#endif\n#endif\n}\n', + ), + ), + ).toEqual([ + { identifier: `${TARGET}/RunnerTests/testNoSpringBoard`, platforms: ['macOS', 'tvOS'] }, + { identifier: `${TARGET}/RunnerTests/testPure`, platforms: ['iOS', 'macOS', 'tvOS'] }, + { identifier: `${TARGET}/RunnerTests/testSim`, platforms: ['iOS'] }, + ]); + }); +}); diff --git a/scripts/__tests__/xctest-run-summary.test.ts b/scripts/__tests__/xctest-run-summary.test.ts index 51987bac7..2905673da 100644 --- a/scripts/__tests__/xctest-run-summary.test.ts +++ b/scripts/__tests__/xctest-run-summary.test.ts @@ -1,5 +1,5 @@ -// The nightly XCTest lane's reporter is also its liveness check, and both halves only ever -// execute on a macOS runner at 04:30 — so they are proven here instead. +// A whole-bundle XCTest lane's reporter is also its liveness check, and both halves only +// ever execute on a macOS runner — so they are proven here instead. import { describe, expect, test } from 'vitest'; import { @@ -10,6 +10,8 @@ import { type ResultSummary, } from '../xctest-run-summary.ts'; +const TITLE = 'iOS runner full XCTest suite'; + function failures(count: number, text = 'assertion failed') { return Array.from({ length: count }, (_, index) => ({ testName: `testCase${index}()`, @@ -21,19 +23,35 @@ describe('the liveness check', () => { test('a run that executed no tests fails, because xcodebuild calls that success', () => { // The lane's whole reason to assert: a build without the unit-test compile flag, an // empty test plan, or a `-skip-testing:` entry that swallowed the suite all exit 0. - const failure = livenessFailure({ result: 'Passed', totalTestCount: 0 }); + const failure = livenessFailure({ result: 'Passed', totalTestCount: 0 }, 153); expect(failure).toContain('executed no tests'); expect(failure).toContain('AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS'); expect(failure).toContain('-skip-testing'); }); test('a summary missing the count entirely is treated as no tests, not as unknown', () => { - expect(livenessFailure({})).not.toBeNull(); + expect(livenessFailure({}, 153)).not.toBeNull(); + }); + + test('a run that executed exactly the source-derived reach passes, red or green', () => { + expect( + livenessFailure({ totalTestCount: 153, result: 'Failed', failedTests: 9 }, 153), + ).toBeNull(); + expect(livenessFailure({ totalTestCount: 1, result: 'Passed' }, 1)).toBeNull(); + }); + + test('a run that executed fewer or more tests than the source reaches fails', () => { + // The quieter failure "at least one ran" never sees: a `#if` guard that compiles a file + // of tests out of the lane's platform, or a flag that matched more than it names. + const fewer = livenessFailure({ totalTestCount: 120, result: 'Passed' }, 133); + expect(fewer).toContain('executed 120 test(s) but the source says it reaches 133'); + expect(fewer).toContain('check:xctest-selection'); + expect(livenessFailure({ totalTestCount: 134, result: 'Passed' }, 133)).not.toBeNull(); }); - test('a run that executed tests passes, red or green', () => { - expect(livenessFailure({ totalTestCount: 153, result: 'Failed', failedTests: 9 })).toBeNull(); - expect(livenessFailure({ totalTestCount: 1, result: 'Passed' })).toBeNull(); + test('a lane the source says reaches nothing can never be credible', () => { + expect(livenessFailure({ totalTestCount: 0 }, 0)).toContain('derives no reachable tests'); + expect(livenessFailure({ totalTestCount: 5 }, 0)).toContain('derives no reachable tests'); }); }); @@ -50,19 +68,25 @@ describe('the job summary', () => { }; test('leads with the headline a reader needs', () => { - const rendered = renderSummary(green); + const rendered = renderSummary(green, TITLE, 153); expect(rendered).toContain('- result: **Passed**'); - expect(rendered).toContain('- executed: **153** (passed 153, failed 0, skipped 0'); + expect(rendered).toContain( + '- executed: **153** of the 153 the source reaches on this lane (passed 153, failed 0, skipped 0', + ); expect(rendered).toContain('- duration: 450s'); expect(rendered).not.toContain('#### Failures'); }); test('reports an unknown duration rather than NaN when the times are absent', () => { - expect(renderSummary({ totalTestCount: 1 })).toContain('- duration: unknown'); + expect(renderSummary({ totalTestCount: 1 }, TITLE, 1)).toContain('- duration: unknown'); }); test('lists failures and names each one', () => { - const rendered = renderSummary({ ...green, result: 'Failed', testFailures: failures(3) }); + const rendered = renderSummary( + { ...green, result: 'Failed', testFailures: failures(3) }, + TITLE, + 153, + ); expect(rendered).toContain('#### Failures'); expect(rendered).toContain('`testCase0()`'); expect(rendered).toContain('assertion failed'); @@ -72,46 +96,54 @@ describe('the job summary', () => { // The night this lane matters most is the night the failure list is longest, and the // job summary is capped at 1 MiB — losing the headline to the tail would be the worst // possible trade. - const rendered = renderSummary({ - ...green, - result: 'Failed', - testFailures: failures(MAX_LISTED_FAILURES + 7), - }); + const rendered = renderSummary( + { ...green, result: 'Failed', testFailures: failures(MAX_LISTED_FAILURES + 7) }, + TITLE, + 153, + ); const listed = rendered.split('\n').filter((line) => line.startsWith('- `test')).length; expect(listed).toBe(MAX_LISTED_FAILURES); expect(rendered).toContain('…and 7 more'); }); test('truncates one enormous failure message instead of letting it dominate', () => { - const rendered = renderSummary({ - ...green, - result: 'Failed', - testFailures: failures(1, 'x'.repeat(5000)), - }); + const rendered = renderSummary( + { ...green, result: 'Failed', testFailures: failures(1, 'x'.repeat(5000)) }, + TITLE, + 153, + ); expect(rendered).toContain('x'.repeat(MAX_FAILURE_TEXT)); expect(rendered).not.toContain('x'.repeat(MAX_FAILURE_TEXT + 1)); }); test('flattens newlines so a multi-line stack cannot forge markdown structure', () => { - const rendered = renderSummary({ - ...green, - result: 'Failed', - testFailures: [{ testName: 'testX()', failureText: 'line one\n#### Injected\nline two' }], - }); + const rendered = renderSummary( + { + ...green, + result: 'Failed', + testFailures: [{ testName: 'testX()', failureText: 'line one\n#### Injected\nline two' }], + }, + TITLE, + 153, + ); // Markdown headings only bind at the start of a line, so flattening is what disarms // the injection: the text survives verbatim, but it can no longer open a section. expect(rendered.split('\n').filter((line) => line.startsWith('#'))).toEqual([ - '### iOS runner full XCTest suite', + `### ${TITLE}`, '#### Failures', ]); expect(rendered).toContain('line one #### Injected line two'); }); test('falls back to the identifier when a failure carries no test name', () => { - const rendered = renderSummary({ - ...green, - testFailures: [{ testIdentifierString: 'RunnerTests/testY()', failureText: 'boom' }], - }); + const rendered = renderSummary( + { + ...green, + testFailures: [{ testIdentifierString: 'RunnerTests/testY()', failureText: 'boom' }], + }, + TITLE, + 153, + ); expect(rendered).toContain('`RunnerTests/testY()`'); }); }); diff --git a/scripts/__tests__/xctest-selection.test.ts b/scripts/__tests__/xctest-selection.test.ts index 041b74c4e..41cb799c5 100644 --- a/scripts/__tests__/xctest-selection.test.ts +++ b/scripts/__tests__/xctest-selection.test.ts @@ -1,9 +1,10 @@ -// The check that keeps the iOS workflows' hand-written test identifiers honest is itself -// only as good as its two parsers, and both of its inputs are files nobody edits with this -// check in mind. So: the real tree must pass, and a planted typo in the real workflow text -// must fail — in both directions, because an unknown `-skip-testing:` entry re-arms the -// nightly's 24-hour hang on `RunnerTests/testCommand`. Synthetic sources cover the shapes -// the real tree happens not to contain today. +// The check that keeps the runner XCTest lanes honest is itself only as good as its +// parsers, and all of its inputs are files nobody edits with this check in mind. So: the +// real tree must pass, a planted typo in the real workflow text must fail — in both flag +// directions, because an unknown `-skip-testing:` entry re-arms a whole-bundle lane's +// 24-hour hang on `RunnerTests/testCommand` — and a planted guard that compiles a test out +// of every lane must fail as "dark". Synthetic sources cover the shapes the real tree +// happens not to contain today. import fs from 'node:fs'; import path from 'node:path'; @@ -12,22 +13,25 @@ import { mkdtempForTestSync } from '../../src/__tests__/test-utils/tmp-dir.ts'; import { buildReport, counts, + ENTRY_POINT_METHOD, formatSummary, GUARDED_WORKFLOWS, + HOST_WORKFLOW_FILE, + LANES, loadReport, NIGHTLY_WORKFLOW_FILE, - parseDeclaredTests, parseFlaggedTests, PR_WORKFLOW_FILE, - readSwiftSources, reportFailures, runnerPackageSourceFailures, - RUNNER_TESTS_DIR, type WorkflowSource, } from '../check-xctest-selection.ts'; +import { activeSource, PLATFORMS } from '../swift-conditional-compilation.ts'; +import { readSwiftSources, RUNNER_TESTS_DIR } from '../xctest-declarations.ts'; const repoRoot = path.resolve(import.meta.dirname, '..', '..'); const TARGET = 'AgentDeviceRunnerUITests'; +const ENTRY_POINT = `${TARGET}/${ENTRY_POINT_METHOD}`; function source(text: string) { return [{ file: 'RunnerTests+Fixture.swift', text }]; @@ -44,28 +48,64 @@ function realSources() { return readSwiftSources(path.join(repoRoot, RUNNER_TESTS_DIR)); } +/** Workflow texts that skip the entry point on both whole-bundle lanes and list `pr` on the PR lane. */ +function laneWorkflows(pr: readonly string[] = []): WorkflowSource[] { + return [ + { workflow: HOST_WORKFLOW_FILE, text: `-skip-testing:${ENTRY_POINT}` }, + { workflow: NIGHTLY_WORKFLOW_FILE, text: `-skip-testing:${ENTRY_POINT}` }, + { workflow: PR_WORKFLOW_FILE, text: pr.map((id) => `-only-testing:${id}`).join('\n') }, + ]; +} + +const ENTRY_SOURCE = 'final class RunnerTests: XCTestCase {\n func testCommand() {}\n}\n'; + describe('the real tree', () => { - test('every flagged identifier in both workflows names a declared test', () => { + test('every flagged identifier names a declared test its lane compiles, and nothing is dark', () => { expect(reportFailures(loadReport(repoRoot))).toEqual([]); }); - test('the PR lane selects a real subset — most tests are reached only by the nightly', () => { + test('the three lanes partition the suite the way the classification says', () => { const report = loadReport(repoRoot); - const { declared, pr, skipped, nightlyOnly } = counts(report); - // Not pinned to today's exact numbers; the invariants are that the PR list is a proper - // subset, that the nightly skips something, and that the three partition the suite. + const { declared, host, pr, nightly, dark } = counts(report); + // Not pinned to today's exact numbers; the invariants are the shape. The host lane + // (macOS) and the nightly (iOS) both skip only the entry point, so together with the + // simulator-only guard they cover everything else; the PR list is a proper subset of + // the nightly; and the entry point is the only method outside every lane. + expect(host).toBeGreaterThan(0); + expect(nightly).toBeGreaterThan(pr); expect(pr).toBeGreaterThan(0); - expect(skipped).toBeGreaterThan(0); - expect(declared).toBeGreaterThan(pr + skipped); - expect(pr + skipped + nightlyOnly).toBe(declared); + expect(dark).toBe(0); + const reachedAnywhere = new Set(LANES.flatMap((entry) => [...report.reach[entry.id]])); + expect(reachedAnywhere.size).toBe(declared - 1); + expect(reachedAnywhere.has(ENTRY_POINT)).toBe(false); + for (const id of report.reach.pr) expect(report.reach.nightly.has(id)).toBe(true); }); - test('the nightly skips the runner server entry point, which is not a test', () => { + test('the whole-bundle lanes skip the runner server entry point, which is not a test', () => { // The whole reason -skip-testing: exists in this repo. `testCommand` opens an // NWListener and waits 24 hours; an unfiltered run would hang the lane to its timeout. const skipped = loadReport(repoRoot).flagged.filter((entry) => entry.flag === 'skip-testing'); - expect(skipped.map((entry) => entry.identifier)).toContain(`${TARGET}/RunnerTests/testCommand`); - for (const entry of skipped) expect(entry.workflow).toBe(NIGHTLY_WORKFLOW_FILE); + for (const entry of LANES.filter((entry) => entry.selection === 'whole')) { + expect( + skipped.filter((flag) => flag.workflow === entry.workflow).map((flag) => flag.identifier), + ).toContain(ENTRY_POINT); + } + for (const entry of skipped) expect(entry.identifier).toBe(ENTRY_POINT); + }); + + test('the simulator-only tests are exactly the ones the macOS build compiles out', () => { + // The classification convention (RunnerTests.swift): a test the host lane must not run + // says so with an `os(iOS)` guard. Everything the iOS build compiles and the macOS build + // does not is therefore simulator-only, and the nightly is the lane that runs it. + const report = loadReport(repoRoot); + const simulatorOnly = report.declaredTests.filter( + (test) => test.platforms.includes('iOS') && !test.platforms.includes('macOS'), + ); + expect(simulatorOnly.length).toBeGreaterThan(0); + for (const test of simulatorOnly) { + expect(report.reach.host.has(test.identifier)).toBe(false); + expect(report.reach.nightly.has(test.identifier)).toBe(true); + } }); test('the declared set covers every addressable method in the target directory', () => { @@ -87,6 +127,16 @@ describe('the real tree', () => { expect(loadReport(repoRoot).declared).toHaveLength(counted); }); + test('every #if condition in the tree is one the evaluator understands', () => { + // The evaluator throws on vocabulary it does not know rather than guessing; the tree + // must therefore stay inside that vocabulary, or the check goes red on the new guard. + for (const entry of realSources()) { + for (const platform of PLATFORMS) { + expect(() => activeSource(entry.text, platform, entry.file)).not.toThrow(); + } + } + }); + test('the package-source boundary rejects an unguarded runner unit test', () => { const root = mkdtempForTestSync('agent-device-runner-package-selection-'); onTestFinished(() => fs.rmSync(root, { recursive: true, force: true })); @@ -129,37 +179,37 @@ describe('a planted typo', () => { expect(reportFailures(report).join('\n')).toContain(typo); }); - test('a renamed test in the nightly `-skip-testing:` list is reported too', () => { - // Without this the typo is invisible: the nightly would simply stop skipping, run - // testCommand, and hang until timeout-minutes with no clue in the log. - const nightly = fs.readFileSync(path.join(repoRoot, NIGHTLY_WORKFLOW_FILE), 'utf8'); - const typo = `${TARGET}/RunnerTests/testCommandd`; - const report = buildReport( - TARGET, - realSources(), - realWorkflows({ - [NIGHTLY_WORKFLOW_FILE]: nightly.replace(`${TARGET}/RunnerTests/testCommand`, typo), - }), - ); - - expect(report.unknown.map((entry) => [entry.flag, entry.identifier])).toEqual([ - ['skip-testing', typo], - ]); - expect(reportFailures(report).join('\n')).toContain('testCommand'); - }); + test.each([ + ['nightly', NIGHTLY_WORKFLOW_FILE], + ['host', HOST_WORKFLOW_FILE], + ])( + 'a renamed `-skip-testing:` entry on the %s lane is reported as unknown AND as the hang it re-arms', + (laneId, workflow) => { + // Without this the typo is invisible: the lane would simply stop skipping, run + // testCommand, and hang until timeout-minutes with no clue in the log. + const text = fs.readFileSync(path.join(repoRoot, workflow), 'utf8'); + const typo = `${ENTRY_POINT}d`; + const report = buildReport( + TARGET, + realSources(), + realWorkflows({ [workflow]: text.replace(ENTRY_POINT, typo) }), + ); + + expect(report.unknown.map((entry) => [entry.flag, entry.identifier])).toEqual([ + ['skip-testing', typo], + ]); + const failures = reportFailures(report).join('\n'); + expect(failures).toContain('testCommand'); + expect(failures).toContain(`reachable by lane(s): ${laneId}`); + }, + ); test('a deleted test is reported even though the surviving list still passes', () => { - const workflows = [ - { - workflow: PR_WORKFLOW_FILE, - text: [ - ` -only-testing:${TARGET}/RunnerTests/testKept \\`, - ` -only-testing:${TARGET}/RunnerTests/testGone`, - ].join('\n'), - }, - { workflow: NIGHTLY_WORKFLOW_FILE, text: `-skip-testing:${TARGET}/RunnerTests/testKept` }, - ]; - const kept = 'extension RunnerTests {\n func testKept() {}\n}\n'; + const kept = `${ENTRY_SOURCE}extension RunnerTests {\n func testKept() {}\n}\n`; + const workflows = laneWorkflows([ + `${TARGET}/RunnerTests/testKept`, + `${TARGET}/RunnerTests/testGone`, + ]); expect( buildReport( @@ -172,61 +222,72 @@ describe('a planted typo', () => { buildReport(TARGET, source(kept), workflows).unknown.map((entry) => entry.identifier), ).toEqual([`${TARGET}/RunnerTests/testGone`]); }); -}); -describe('the declaration scan', () => { - test('binds a method to the top-level type that encloses it', () => { - expect( - parseDeclaredTests( - TARGET, - source( - 'final class RunnerTests: XCTestCase {\n func testInClass() {}\n}\n\n' + - 'extension RunnerTests {\n func testInExtension() throws {}\n}\n\n' + - 'final class OtherTests: XCTestCase {\n func testElsewhere() async throws {}\n}\n', - ), + test('a listed test the PR lane platform never compiles is reported, not silently unmatched', () => { + // Declared, so the rename check passes — but ios.yml builds for iOS, and an + // `os(macOS)` guard means the identifier matches nothing there. + const report = buildReport( + TARGET, + source( + `${ENTRY_SOURCE}extension RunnerTests {\n#if os(macOS)\n func testHostOnly() {}\n#endif\n}\n`, ), - ).toEqual([ - `${TARGET}/OtherTests/testElsewhere`, - `${TARGET}/RunnerTests/testInClass`, - `${TARGET}/RunnerTests/testInExtension`, + laneWorkflows([`${TARGET}/RunnerTests/testHostOnly`]), + ); + expect(report.unknown).toEqual([]); + expect(report.uncompiled.map((entry) => entry.identifier)).toEqual([ + `${TARGET}/RunnerTests/testHostOnly`, ]); + expect(reportFailures(report).join('\n')).toContain('never compiles'); }); +}); - test('ignores declarations no filter could address', () => { - expect( - parseDeclaredTests( - TARGET, - source( - 'extension RunnerTests {\n' + - // A helper type declared inside a test body must not capture the methods after - // it, and `class func` must not read as a type declaration named `func`. - ' class func makeHelper() {}\n' + - ' func testWithNestedHelper() {\n' + - ' final class ResultBox {}\n' + - ' func testLocal() {}\n' + - ' }\n' + - ' // func testCommentedOut() {}\n' + - ' func testAfterNesting() {}\n' + - '}\n', - ), +describe('a planted guard', () => { + test('a test gated to a platform no lane runs is reported as dark', () => { + // The real instance this rule was written for: two tests under `#if os(tvOS)` that no + // lane had ever executed. Widening the guard to `|| os(macOS)` put them on the host lane. + const report = buildReport( + TARGET, + source( + `${ENTRY_SOURCE}#if AGENT_DEVICE_RUNNER_UNIT_TESTS\nextension RunnerTests {\n` + + ' func testEverywhere() {}\n#if os(tvOS)\n func testTvOnly() {}\n#endif\n}\n#endif\n', ), - ).toEqual([ - `${TARGET}/RunnerTests/testAfterNesting`, - `${TARGET}/RunnerTests/testWithNestedHelper`, + laneWorkflows(), + ); + expect(report.dark).toEqual([`${TARGET}/RunnerTests/testTvOnly`]); + expect(report.reach.host.has(`${TARGET}/RunnerTests/testEverywhere`)).toBe(true); + expect(report.reach.nightly.has(`${TARGET}/RunnerTests/testEverywhere`)).toBe(true); + expect(reportFailures(report).join('\n')).toContain('reachable by no lane'); + }); + + test('a simulator-only guard keeps a test off the host lane and on the nightly', () => { + const report = buildReport( + TARGET, + source( + `${ENTRY_SOURCE}extension RunnerTests {\n#if AGENT_DEVICE_RUNNER_UNIT_TESTS && os(iOS)\n` + + ' func testLaunchesApp() {}\n#endif\n}\n', + ), + laneWorkflows(), + ); + const id = `${TARGET}/RunnerTests/testLaunchesApp`; + expect(report.reach.host.has(id)).toBe(false); + expect(report.reach.nightly.has(id)).toBe(true); + expect(report.reach.pr.has(id)).toBe(false); + expect(reportFailures(report)).toEqual([]); + }); + + test('a whole-bundle lane that stops skipping the entry point is reported as reaching it', () => { + const report = buildReport(TARGET, source(ENTRY_SOURCE), [ + { workflow: HOST_WORKFLOW_FILE, text: 'run: xcodebuild test-without-building' }, + { workflow: NIGHTLY_WORKFLOW_FILE, text: `-skip-testing:${ENTRY_POINT}` }, + { workflow: PR_WORKFLOW_FILE, text: `-only-testing:${TARGET}/RunnerTests/testOther` }, ]); + expect(report.entryPointReachedBy).toEqual(['host']); + expect(reportFailures(report).join('\n')).toContain('reachable by lane(s): host'); }); - test('reads a file whose name does not start with RunnerTests', () => { - // RunnerTapPointPolicy.swift is the real instance: the synchronized-root-group project - // compiles every .swift in the directory, so file naming carries no membership meaning. - expect( - parseDeclaredTests(TARGET, [ - { - file: 'RunnerTapPointPolicy.swift', - text: 'extension RunnerTests {\n func testGolden() {}\n}\n', - }, - ]), - ).toEqual([`${TARGET}/RunnerTests/testGolden`]); + test('the PR list naming the entry point is reported too', () => { + const report = buildReport(TARGET, source(ENTRY_SOURCE), laneWorkflows([ENTRY_POINT])); + expect(report.entryPointReachedBy).toEqual(['pr']); }); }); @@ -280,6 +341,18 @@ describe('the workflow scan', () => { ]); }); + test('reads every flag on one line, not just the first', () => { + // Nothing stops two flags sharing a line, and matching only the first is silently + // permissive in the skip direction — the unseen entry would be a lane that stopped + // skipping the 24-hour entry point with no signal. + expect( + parseFlaggedTests( + NIGHTLY_WORKFLOW_FILE, + `-skip-testing:${TARGET}/RunnerTests/testOne -skip-testing:${TARGET}/RunnerTests/testTwo`, + ).map((entry) => entry.identifier), + ).toEqual([`${TARGET}/RunnerTests/testOne`, `${TARGET}/RunnerTests/testTwo`]); + }); + test('a bare flag mention with no identifier after it is not a selection', () => { expect(parseFlaggedTests(PR_WORKFLOW_FILE, 'run: echo "-only-testing: is a flag"')).toEqual([]); }); @@ -287,35 +360,37 @@ describe('the workflow scan', () => { test('leaves another target alone rather than guessing about sources it cannot see', () => { const report = buildReport( TARGET, - source('extension RunnerTests {\n func testOne() {}\n}\n'), + source(`${ENTRY_SOURCE}extension RunnerTests {\n func testOne() {}\n}\n`), [ { workflow: PR_WORKFLOW_FILE, text: `-only-testing:SomeOtherTarget/OtherTests/testUnknown\n-only-testing:${TARGET}/RunnerTests/testOne`, }, + { workflow: HOST_WORKFLOW_FILE, text: `-skip-testing:${ENTRY_POINT}` }, + { workflow: NIGHTLY_WORKFLOW_FILE, text: `-skip-testing:${ENTRY_POINT}` }, ], ); expect(report.unknown).toEqual([]); - expect(report.flagged).toHaveLength(2); + expect(report.uncompiled).toEqual([]); + expect(report.flagged).toHaveLength(4); }); }); describe('the blind-parse guards', () => { - const oneTest = () => source('extension RunnerTests {\n func testOne() {}\n}\n'); - const oneFlag = (workflow: string) => [ - { workflow, text: `-only-testing:${TARGET}/RunnerTests/testOne` }, - ]; + const oneTest = () => source(`${ENTRY_SOURCE}extension RunnerTests {\n func testOne() {}\n}\n`); test('a guarded workflow that no longer exists fails instead of leaving a stale claim', () => { const report = buildReport(TARGET, oneTest(), [ - ...oneFlag(PR_WORKFLOW_FILE), + ...laneWorkflows([`${TARGET}/RunnerTests/testOne`]).filter( + (entry) => entry.workflow !== NIGHTLY_WORKFLOW_FILE, + ), { workflow: NIGHTLY_WORKFLOW_FILE, text: null }, ]); expect(reportFailures(report).join('\n')).toContain(NIGHTLY_WORKFLOW_FILE); }); test('an empty declaration scan fails instead of reporting a healthy list', () => { - const report = buildReport(TARGET, source('// nothing here\n'), oneFlag(PR_WORKFLOW_FILE)); + const report = buildReport(TARGET, source('// nothing here\n'), laneWorkflows()); expect(reportFailures(report).join('\n')).toContain('declaration scan is broken'); }); @@ -326,13 +401,14 @@ describe('the blind-parse guards', () => { }); describe('the summary line', () => { - test('reports the partition a reader needs to see the PR lane shrinking', () => { + test('reports the per-lane reach a reader needs to see the partition', () => { const report = loadReport(repoRoot); - const { declared, pr, skipped, nightlyOnly } = counts(report); + const { declared, host, pr, nightly, dark } = counts(report); const summary = formatSummary(report); expect(summary).toContain(`${declared} declared`); - expect(summary).toContain(`${pr} selected on every PR`); - expect(summary).toContain(`${skipped} skipped by the nightly`); - expect(summary).toContain(`${nightlyOnly} reached only by the nightly`); + expect(summary).toContain(`reaches ${host}, PR list`); + expect(summary).toContain(`selects ${pr}, nightly`); + expect(summary).toContain(`reaches ${nightly};`); + expect(summary).toContain(`${dark} reachable by no lane`); }); }); diff --git a/scripts/check-xctest-selection.ts b/scripts/check-xctest-selection.ts index 30f05040e..bc9020aa8 100644 --- a/scripts/check-xctest-selection.ts +++ b/scripts/check-xctest-selection.ts @@ -1,63 +1,119 @@ -// `pnpm check:xctest-selection` — hold the hand-written `-only-testing:` and -// `-skip-testing:` lists to the tests that actually exist, and ensure those unit-test methods -// are stripped from the Apple runner source copied into the npm package (#1781 A7). +// `pnpm check:xctest-selection` — derive, from the Swift sources and the workflow files, which +// runner XCTest methods each CI lane actually reaches, and fail when a method reaches none +// (#1781 A7). Also ensures those unit-test methods are stripped from the Apple runner source +// copied into the npm package. // -// `xcodebuild` treats a test identifier that matches nothing as an empty set rather than an -// error, in BOTH directions, and each direction fails silently in its own way: +// Three lanes run the `AgentDeviceRunnerUITests` bundle, and each reaches a different set: // -// - `-only-testing:` (ios.yml, 37 hand-written entries) — a renamed or deleted test stops -// running with no signal. The lane stays green with fewer tests than the list claims. -// - `-skip-testing:` (xctest-nightly.yml) — the nightly skips `RunnerTests/testCommand`, -// which is not a test at all: it is the runner's server entry point (RunnerTests.swift), -// compiled unconditionally, and it starts an NWListener and waits 24 hours. A typo in -// that entry re-arms a full-timeout hang, and a hang is the most expensive way a lane -// can fail. +// - host ci.yml, macOS host, every PR: the whole bundle as compiled for macOS, minus +// `-skip-testing:` — the pure runner-decision tests, whose guard is +// `#if AGENT_DEVICE_RUNNER_UNIT_TESTS` alone. +// - pr ios.yml, iOS Simulator, every PR: the hand-written `-only-testing:` list. +// - nightly xctest-nightly.yml, iOS Simulator, scheduled: the whole bundle as compiled for +// iOS, minus `-skip-testing:` — includes the simulator-only tests, whose guard is +// `… && os(iOS)` (they launch the host app, route through SpringBoard, or assert an +// iOS-only branch). // -// Deliberately one-directional about coverage: a test in no `-only-testing:` list is fine, -// the nightly runs it. Only an identifier naming nothing is a defect. +// The classification therefore lives in the `#if` guards, so this check evaluates them per +// platform rather than treating a source-level `func test…` as running everywhere. What it +// holds: // -// The declaration scan is source-level, so a method compiled out by `#if` still counts as -// declared. That is the intended precision — this check guards the lists against renames and -// deletions, not against platform availability, which the nightly observes directly. +// 1. Every `-only-testing:`/`-skip-testing:` identifier names a declared method that +// compiles for that lane's platform. `xcodebuild` treats an identifier matching nothing +// as an empty set rather than an error, in BOTH directions: an unknown `-only-testing:` +// drops a test from the PR lane silently, and an unknown `-skip-testing:` re-admits +// `RunnerTests/testCommand` — not a test but the runner's server entry point, which opens +// an NWListener and waits 24 hours — into a whole-bundle lane and hangs it. +// 2. Every declared method is reachable by at least one lane. A test gated to a platform +// no lane runs (the tvOS-only pair this check found) is dark from the day it is written. +// 3. The entry point is reachable by no lane at all. +// +// The nightly and host lanes also assert their executed count equals the reach derived here +// (scripts/xctest-run-summary.ts), so a build variant without the unit-test compile flag, or a +// guard that quietly compiles a file out, reads as red instead of as a smaller green. import fs from 'node:fs'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; import { runCmdSync } from '../src/utils/exec.ts'; +import type { Platform } from './swift-conditional-compilation.ts'; +import { + parseDeclaredTestsByPlatform, + readSwiftSources, + RUNNER_TESTS_DIR, + type DeclaredTest, + type SwiftSource, +} from './xctest-declarations.ts'; const repoRoot = path.resolve(import.meta.dirname, '..'); const packageAppleRunnerScript = path.join(repoRoot, 'scripts/package-apple-runner-source.mjs'); -/** The XCTest target directory; its basename is the target name the identifiers use. */ -export const RUNNER_TESTS_DIR = 'apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests'; +/** The macOS host lane, which runs the whole macOS-compiled bundle on every PR. */ +export const HOST_WORKFLOW_FILE = '.github/workflows/ci.yml'; -/** The PR lane, whose `-only-testing:` list decides what every pull request runs. */ +/** The PR lane, whose `-only-testing:` list decides what every pull request runs on the simulator. */ export const PR_WORKFLOW_FILE = '.github/workflows/ios.yml'; -/** The nightly lane, whose `-skip-testing:` list decides what the full suite leaves out. */ +/** The nightly lane, whose `-skip-testing:` list decides what the full simulator suite leaves out. */ export const NIGHTLY_WORKFLOW_FILE = '.github/workflows/xctest-nightly.yml'; +export type LaneId = 'host' | 'pr' | 'nightly'; + +export type Lane = { + readonly id: LaneId; + readonly workflow: string; + readonly platform: Platform; + /** `whole`: everything compiled minus `-skip-testing:`; `list`: the `-only-testing:` entries. */ + readonly selection: 'whole' | 'list'; + /** The job-summary heading the lane's reporter prints. */ + readonly title: string; +}; + +export const LANES: readonly Lane[] = [ + { + id: 'host', + workflow: HOST_WORKFLOW_FILE, + platform: 'macOS', + selection: 'whole', + title: 'iOS runner host XCTest lane (macOS, no simulator)', + }, + { + id: 'pr', + workflow: PR_WORKFLOW_FILE, + platform: 'iOS', + selection: 'list', + title: 'iOS runner PR XCTest list', + }, + { + id: 'nightly', + workflow: NIGHTLY_WORKFLOW_FILE, + platform: 'iOS', + selection: 'whole', + title: 'iOS runner full XCTest suite', + }, +]; + +export function lane(id: string): Lane { + const found = LANES.find((entry) => entry.id === id); + if (!found) { + throw new Error( + `Unknown XCTest lane "${id}"; expected one of ${LANES.map((e) => e.id).join(', ')}.`, + ); + } + return found; +} + /** - * Every workflow whose test identifiers this check owns. Both are read, so a workflow that - * is renamed or deleted fails here rather than leaving a stale claim in the output. + * Every workflow whose test identifiers this check owns. All are read, so a workflow that is + * renamed or deleted fails here rather than leaving a stale claim in the output. */ -export const GUARDED_WORKFLOWS: readonly string[] = [PR_WORKFLOW_FILE, NIGHTLY_WORKFLOW_FILE]; +export const GUARDED_WORKFLOWS: readonly string[] = LANES.map((entry) => entry.workflow); -// Every .swift file in the target directory is a member: the Xcode project uses a -// PBXFileSystemSynchronizedRootGroup, so membership is the directory, not a file list. A -// `RunnerTests*` name filter would miss RunnerTapPointPolicy.swift, which declares a real -// addressable test inside `extension RunnerTests`. -const SWIFT_SOURCE = /\.swift$/; - -// One ordered pass over the source. A column-0 type declaration moves the enclosing type; -// a `func test…` indented exactly one level binds to it. Position carries the meaning -// rather than brace counting, which would have to know which `{` sits inside a string -// literal. It is also the more precise rule: only a method declared directly in a -// top-level `class`/`extension` block is addressable as `Target/Class/method`, so a -// helper type nested inside a test body (`final class ResultBox` — several of these -// exist) contributes no test identifiers, and neither does a closure-local `func test…`. -const DECLARATION = - /^(?:[\w@]+[ \t]+)*(?:class|extension|struct|enum|actor|protocol)[ \t]+([A-Za-z_]\w*)|^ {2}(?:[\w@]+[ \t]+)*func[ \t]+(test\w*)[ \t]*\(/gm; +/** + * The one method that must be reachable by no lane: `testCommand` is the runner's server + * entry point (RunnerTests.swift), compiled unconditionally. + */ +export const ENTRY_POINT_METHOD = 'RunnerTests/testCommand'; // Two guards against reading prose as configuration, both learned the hard way: these // workflows discuss their own flags in comments, and this check's first draft counted the @@ -67,9 +123,10 @@ const DECLARATION = // colon matches nothing. A typo'd identifier is still identifier-shaped, so both guards // narrow what counts as a flag without narrowing what counts as a defect. const YAML_COMMENT = /^\s*#/; -const TEST_FLAG = /-(only|skip)-testing:([A-Za-z_][\w.+-]*(?:\/[A-Za-z_]\w*){1,2})/; - -export type SwiftSource = { readonly file: string; readonly text: string }; +// Global: a `run:` line may carry more than one flag (nothing stops `-only-testing:A -only-testing:B` +// on one line), and matching only the first would make the second invisible — silently permissive in +// the skip direction, where an unseen entry is a lane that stops skipping the 24-hour entry point. +const TEST_FLAG = /-(only|skip)-testing:([A-Za-z_][\w.+-]*(?:\/[A-Za-z_]\w*){1,2})/g; export type TestFlag = 'only-testing' | 'skip-testing'; @@ -83,78 +140,106 @@ export type FlaggedTest = { export type SelectionReport = { readonly target: string; - /** Every `Target/Class/method` the Swift sources declare, sorted. */ + /** Every `Target/Class/method` the Swift sources declare, sorted, regardless of guards. */ readonly declared: readonly string[]; + /** The same set with the platforms each method compiles for. */ + readonly declaredTests: readonly DeclaredTest[]; /** Every flagged identifier across the guarded workflows, in file order. */ readonly flagged: readonly FlaggedTest[]; /** Guarded workflows that do not exist — a claim this check can no longer make. */ readonly missingWorkflows: readonly string[]; - /** Flagged identifiers naming no declared method — the failure. */ + /** Flagged identifiers naming no declared method — a failure. */ readonly unknown: readonly FlaggedTest[]; + /** Flagged identifiers naming a method its lane's platform never compiles — a failure. */ + readonly uncompiled: readonly FlaggedTest[]; + /** What each lane reaches, once guards and flags are resolved. */ + readonly reach: Readonly>>; + /** Declared methods no lane reaches, entry point excluded — a failure. */ + readonly dark: readonly string[]; + /** Lanes that reach the entry point — a failure (a 24-hour hang). */ + readonly entryPointReachedBy: readonly LaneId[]; }; export type WorkflowSource = { readonly workflow: string; readonly text: string | null }; -export function readSwiftSources(directory: string): SwiftSource[] { - return fs - .readdirSync(directory) - .filter((entry) => SWIFT_SOURCE.test(entry)) - .sort() - .map((entry) => ({ - file: entry, - text: fs.readFileSync(path.join(directory, entry), 'utf8'), - })); -} - -/** Every `Target/Class/method` identifier the sources declare. */ -export function parseDeclaredTests(target: string, sources: readonly SwiftSource[]): string[] { - const declared = new Set(); - for (const source of sources) { - let enclosing = ''; - for (const [, type, method] of source.text.matchAll(DECLARATION)) { - if (type !== undefined) enclosing = type; - else if (method !== undefined && enclosing) declared.add(`${target}/${enclosing}/${method}`); - } - } - return [...declared].sort(); -} - /** Every `-only-testing:`/`-skip-testing:` identifier a workflow names, with its line. */ export function parseFlaggedTests(workflow: string, text: string): FlaggedTest[] { return text.split('\n').flatMap((line, index) => { if (YAML_COMMENT.test(line)) return []; - const match = TEST_FLAG.exec(line); - if (!match) return []; - return [ - { - workflow, - flag: `${match[1]}-testing` as TestFlag, - identifier: match[2] as string, - line: index + 1, - }, - ]; + return [...line.matchAll(TEST_FLAG)].map((match) => ({ + workflow, + flag: `${match[1]}-testing` as TestFlag, + identifier: match[2] as string, + line: index + 1, + })); }); } +/** Attributed per workflow, not just per flag: each lane's number has to be its own. */ +function identifiers( + flagged: readonly FlaggedTest[], + workflow: string, + flag: TestFlag, +): Set { + return new Set( + flagged + .filter((entry) => entry.workflow === workflow && entry.flag === flag) + .map((entry) => entry.identifier), + ); +} + +/** What one lane reaches: its platform's compiled set, narrowed by its flags. */ +function laneReach( + entry: Lane, + declaredTests: readonly DeclaredTest[], + flagged: readonly FlaggedTest[], +): Set { + const only = identifiers(flagged, entry.workflow, 'only-testing'); + const skipped = identifiers(flagged, entry.workflow, 'skip-testing'); + return new Set( + declaredTests + .filter((test) => test.platforms.includes(entry.platform)) + .map((test) => test.identifier) + .filter((id) => (entry.selection === 'whole' || only.has(id)) && !skipped.has(id)), + ); +} + export function buildReport( target: string, sources: readonly SwiftSource[], workflows: readonly WorkflowSource[], ): SelectionReport { - const declared = parseDeclaredTests(target, sources); - const known = new Set(declared); + const declaredTests = parseDeclaredTestsByPlatform(target, sources); + const declared = declaredTests.map((test) => test.identifier); + const known = new Map(declaredTests.map((test) => [test.identifier, test])); const flagged = workflows.flatMap((entry) => entry.text === null ? [] : parseFlaggedTests(entry.workflow, entry.text), ); + const platformOf = new Map(LANES.map((entry) => [entry.workflow, entry.platform])); + // Identifiers for another target are left alone: this check owns one target's sources + // and cannot speak for anything else a workflow might select. + const owned = flagged.filter((entry) => entry.identifier.startsWith(`${target}/`)); + const reach = Object.fromEntries( + LANES.map((entry) => [entry.id, laneReach(entry, declaredTests, flagged)]), + ) as Record>; + const entryPoint = `${target}/${ENTRY_POINT_METHOD}`; + const reachedAnywhere = new Set(LANES.flatMap((entry) => [...reach[entry.id]])); return { target, declared, + declaredTests, flagged, missingWorkflows: workflows.filter((entry) => entry.text === null).map((e) => e.workflow), - // Identifiers for another target are left alone: this check owns one target's - // sources and cannot speak for anything else a workflow might select. - unknown: flagged.filter( - (entry) => entry.identifier.startsWith(`${target}/`) && !known.has(entry.identifier), + unknown: owned.filter((entry) => !known.has(entry.identifier)), + uncompiled: owned.filter((entry) => { + const test = known.get(entry.identifier); + const platform = platformOf.get(entry.workflow); + return test !== undefined && platform !== undefined && !test.platforms.includes(platform); + }), + reach, + dark: declared.filter((id) => id !== entryPoint && !reachedAnywhere.has(id)), + entryPointReachedBy: LANES.filter((entry) => reach[entry.id].has(entryPoint)).map( + (entry) => entry.id, ), }; } @@ -171,29 +256,20 @@ export function loadReport(root: string = repoRoot): SelectionReport { ); } -/** Attributed per workflow, not just per flag: each lane's number has to be its own. */ -function identifiers(report: SelectionReport, workflow: string, flag: TestFlag): Set { - return new Set( - report.flagged - .filter((entry) => entry.workflow === workflow && entry.flag === flag) - .map((entry) => entry.identifier), - ); -} - -/** What each lane reaches, once the two flags are resolved against the declared set. */ +/** The partition a reader needs: how many methods each lane reaches, and how many none does. */ export function counts(report: SelectionReport): { declared: number; + host: number; pr: number; - skipped: number; - nightlyOnly: number; + nightly: number; + dark: number; } { - const pr = identifiers(report, PR_WORKFLOW_FILE, 'only-testing'); - const skipped = identifiers(report, NIGHTLY_WORKFLOW_FILE, 'skip-testing'); return { declared: report.declared.length, - pr: pr.size, - skipped: skipped.size, - nightlyOnly: report.declared.filter((id) => !pr.has(id) && !skipped.has(id)).length, + host: report.reach.host.size, + pr: report.reach.pr.size, + nightly: report.reach.nightly.size, + dark: report.dark.length, }; } @@ -206,7 +282,7 @@ export function reportFailures(report: SelectionReport): string[] { return [ `Missing guarded workflow(s): ${report.missingWorkflows.join(', ')}. This check names ` + 'them in its own output, so a renamed or deleted lane must be reflected in ' + - 'GUARDED_WORKFLOWS rather than leaving a claim nothing backs.', + 'LANES rather than leaving a claim nothing backs.', ]; } if (report.declared.length === 0) { @@ -218,30 +294,63 @@ export function reportFailures(report: SelectionReport): string[] { if (report.flagged.length === 0) { return [ 'Found no `-only-testing:`/`-skip-testing:` entries in ' + - `${GUARDED_WORKFLOWS.join(', ')}. Either both lanes stopped filtering (drop this ` + + `${GUARDED_WORKFLOWS.join(', ')}. Either every lane stopped filtering (drop this ` + 'check), or the scan is broken and can no longer see a dropped test.', ]; } - if (report.unknown.length === 0) return []; - return [ - `${report.unknown.length} XCTest identifier(s) name a method no source declares:`, - ...report.unknown.map( - (entry) => ` - ${entry.workflow}:${entry.line} (-${entry.flag}) ${entry.identifier}`, - ), - 'xcodebuild matches nothing and still exits 0 for an unknown identifier, in both', - 'directions: an unknown `-only-testing:` drops a test from the PR lane silently, and an', - 'unknown `-skip-testing:` re-admits whatever the nightly meant to leave out — including', - `${report.target}/RunnerTests/testCommand, the runner's 24-hour server entry point.`, - 'Update the entry to the current name, or remove it.', - ]; + const failures: string[] = []; + const flagLine = (entry: FlaggedTest) => + ` - ${entry.workflow}:${entry.line} (-${entry.flag}) ${entry.identifier}`; + if (report.unknown.length > 0) { + failures.push( + `${report.unknown.length} XCTest identifier(s) name a method no source declares:`, + ...report.unknown.map(flagLine), + 'xcodebuild matches nothing and still exits 0 for an unknown identifier, in both', + 'directions: an unknown `-only-testing:` drops a test from the PR lane silently, and an', + 'unknown `-skip-testing:` re-admits whatever a whole-bundle lane meant to leave out —', + `including ${report.target}/${ENTRY_POINT_METHOD}, the runner's 24-hour server entry point.`, + 'Update the entry to the current name, or remove it.', + ); + } + if (report.uncompiled.length > 0) { + failures.push( + `${report.uncompiled.length} XCTest identifier(s) name a method that lane's platform ` + + 'never compiles:', + ...report.uncompiled.map(flagLine), + "The method exists, but its `#if` guard compiles it out of that lane's build, so the", + 'flag matches nothing there. Move the entry to a lane whose platform compiles it, or', + 'widen the guard.', + ); + } + if (report.dark.length > 0) { + failures.push( + `${report.dark.length} declared XCTest method(s) are reachable by no lane:`, + ...report.dark.map((identifier) => ` - ${identifier}`), + 'The host lane runs everything the macOS build compiles, the nightly everything the iOS', + 'build compiles, and the PR list names its methods; a method outside all three — usually', + 'a guard naming a platform no lane runs — is dark from the day it is written. Widen the', + 'guard, list it, or delete it.', + ); + } + if (report.entryPointReachedBy.length > 0) { + failures.push( + `${report.target}/${ENTRY_POINT_METHOD} is reachable by lane(s): ` + + `${report.entryPointReachedBy.join(', ')}.`, + 'It is not a test: it opens an NWListener and waits 24 hours for a client, so a lane that', + 'runs it hangs until timeout-minutes. Whole-bundle lanes must keep their -skip-testing:', + 'entry for it; the PR list must not name it.', + ); + } + return failures; } export function formatSummary(report: SelectionReport): string { - const { declared, pr, skipped, nightlyOnly } = counts(report); + const { declared, host, pr, nightly, dark } = counts(report); return ( - `xctest selection: ${declared} declared ${report.target} methods — ${pr} selected on ` + - `every PR (${PR_WORKFLOW_FILE}), ${skipped} skipped by the nightly ` + - `(${NIGHTLY_WORKFLOW_FILE}), ${nightlyOnly} reached only by the nightly.\n` + `xctest selection: ${declared} declared ${report.target} methods — host lane ` + + `(${HOST_WORKFLOW_FILE}, macOS, every PR) reaches ${host}, PR list (${PR_WORKFLOW_FILE}, ` + + `iOS Simulator, every PR) selects ${pr}, nightly (${NIGHTLY_WORKFLOW_FILE}, iOS Simulator) ` + + `reaches ${nightly}; ${dark} reachable by no lane; ${ENTRY_POINT_METHOD} skipped everywhere.\n` ); } diff --git a/scripts/swift-conditional-compilation.ts b/scripts/swift-conditional-compilation.ts new file mode 100644 index 000000000..9193c2ee6 --- /dev/null +++ b/scripts/swift-conditional-compilation.ts @@ -0,0 +1,152 @@ +// Evaluating Swift's `#if` for one platform — the question "does this source line compile for +// iOS / macOS / tvOS?", kept apart from `check-xctest-selection.ts`, which answers the different +// question of which CI lane reaches which test. +// +// This is deliberately NOT a Swift parser reconstructing the compiler. It understands exactly +// the directive vocabulary the runner sources use and THROWS on anything else, so a new guard +// shape fails the gate loudly instead of being silently mis-bucketed. The gate's callers back +// it with an executed-count assertion, so a wrong answer here cannot pass as a smaller green. + +/** The platforms a lane can build the bundle for. visionOS builds exist but no lane runs tests on one. */ +export type Platform = 'iOS' | 'macOS' | 'tvOS'; + +export const PLATFORMS: readonly Platform[] = ['iOS', 'macOS', 'tvOS']; + +// Swift conditional-compilation directives, at any indentation. A `//` comment cannot start +// with `#`, so prose about a guard never reads as one. +const DIRECTIVE = /^\s*#(if|elseif|else|endif)\b(.*)$/; + +/** A `#if` condition outside the vocabulary below. Thrown, never guessed at. */ +export class UnsupportedConditionError extends Error {} + +function tokenize(condition: string): string[] { + const tokens: string[] = []; + const pattern = /\s*(&&|\|\||!|\(|\)|[A-Za-z_][\w.]*)/y; + let index = 0; + while (index < condition.length) { + pattern.lastIndex = index; + const match = pattern.exec(condition); + if (!match) throw new UnsupportedConditionError(`cannot read #if condition: ${condition}`); + tokens.push(match[1] as string); + index = pattern.lastIndex; + if (index === condition.length || /^\s*$/.test(condition.slice(index))) break; + } + return tokens; +} + +/** + * Truth of one `#if` condition when compiling the unit-test variant for `platform`. The + * vocabulary is exactly what the runner sources use; anything else throws rather than + * guessing, because a guess in either direction makes this check see a test on a platform + * that never compiles it, or miss one that does. + */ +export function evaluateCondition(condition: string, platform: Platform): boolean { + const tokens = tokenize(condition.replace(/\/\/.*$/, '').trim()); + let position = 0; + const peek = () => tokens[position]; + const take = () => tokens[position++]; + const fail = (reason: string): never => { + throw new UnsupportedConditionError(`${reason} in #if condition: ${condition.trim()}`); + }; + const expectToken = (expected: string) => { + if (take() !== expected) fail(`expected ${expected}`); + }; + const call = (name: string, argument: string): boolean => { + if (name === 'os') return argument === platform; + if (name === 'canImport' && argument === 'UIKit') return platform !== 'macOS'; + if (name === 'canImport' && argument === 'AppKit') return platform === 'macOS'; + // Every lane that runs tests builds for a simulator; the physical-device build is the + // daemon's, not a test lane's. + if (name === 'targetEnvironment' && argument === 'simulator') return platform !== 'macOS'; + return fail(`unsupported ${name}(${argument})`); + }; + const primary = (): boolean => { + const token = take(); + if (token === undefined) return fail('truncated expression'); + if (token === '(') { + const value = or(); + expectToken(')'); + return value; + } + if (peek() === '(') { + take(); + const argument = take(); + if (argument === undefined) return fail('truncated expression'); + expectToken(')'); + return call(token, argument); + } + // Both are defined by every unit-test build: the compile flag by the build script, and + // DEBUG by the Debug configuration every lane builds. + if (token === 'AGENT_DEVICE_RUNNER_UNIT_TESTS' || token === 'DEBUG') return true; + return fail(`unsupported ${token}`); + }; + const unary = (): boolean => (peek() === '!' ? (take(), !unary()) : primary()); + const and = (): boolean => { + let value = unary(); + while (peek() === '&&') { + take(); + value = unary() && value; + } + return value; + }; + const or = (): boolean => { + let value = and(); + while (peek() === '||') { + take(); + value = and() || value; + } + return value; + }; + const value = or(); + if (position !== tokens.length) fail('trailing tokens'); + return value; +} + +type ConditionalFrame = { parentActive: boolean; active: boolean; taken: boolean }; + +/** One directive's effect on the frame stack; the frames' `active` flags decide what compiles. */ +function applyDirective( + frames: ConditionalFrame[], + keyword: string, + condition: string, + platform: Platform, +): void { + const parentActive = () => frames.every((frame) => frame.active); + if (keyword === 'if') { + const outer = parentActive(); + const taken = evaluateCondition(condition, platform); + frames.push({ parentActive: outer, active: outer && taken, taken }); + return; + } + const frame = frames[frames.length - 1]; + if (!frame) throw new UnsupportedConditionError(`#${keyword} without a matching #if`); + if (keyword === 'endif') { + frames.pop(); + return; + } + const taken = !frame.taken && (keyword === 'else' || evaluateCondition(condition, platform)); + frame.active = frame.parentActive && taken; + frame.taken ||= taken; +} + +/** + * The source with every line inside an inactive `#if` branch (and every directive line) + * blanked, so the declaration scan sees what the compiler sees for `platform`. + */ +export function activeSource(text: string, platform: Platform, file = ''): string { + const frames: ConditionalFrame[] = []; + return text + .split('\n') + .map((line, index) => { + const directive = DIRECTIVE.exec(line); + if (!directive) return frames.every((frame) => frame.active) ? line : ''; + try { + applyDirective(frames, directive[1] as string, directive[2] ?? '', platform); + } catch (error) { + if (!(error instanceof UnsupportedConditionError)) throw error; + throw new UnsupportedConditionError(`${file}:${index + 1}: ${error.message}`); + } + return ''; + }) + .join('\n'); +} diff --git a/scripts/xctest-declarations.ts b/scripts/xctest-declarations.ts new file mode 100644 index 000000000..45ef20987 --- /dev/null +++ b/scripts/xctest-declarations.ts @@ -0,0 +1,87 @@ +// What the runner's XCTest target declares, and on which platforms — the scan behind +// "which lane reaches what" (`check-xctest-selection.ts`), kept separate because it answers a +// different question: this module reads Swift sources, that one reads workflow manifests. +// +// The per-platform attribution is the whole point. A `func test…` is not simply "declared": +// it is declared *for the platforms whose `#if` guards let it compile*, which is what makes a +// test's guard its lane classification (#1781 A7). + +import fs from 'node:fs'; +import path from 'node:path'; +import { activeSource, PLATFORMS, type Platform } from './swift-conditional-compilation.ts'; + +/** The XCTest target directory; its basename is the target name the identifiers use. */ +export const RUNNER_TESTS_DIR = 'apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests'; + +// Every .swift file in the target directory is a member: the Xcode project uses a +// PBXFileSystemSynchronizedRootGroup, so membership is the directory, not a file list. A +// `RunnerTests*` name filter would miss RunnerTapPointPolicy.swift, which declares a real +// addressable test inside `extension RunnerTests`. +const SWIFT_SOURCE = /\.swift$/; + +// One ordered pass over the source. A column-0 type declaration moves the enclosing type; +// a `func test…` indented exactly one level binds to it. Position carries the meaning +// rather than brace counting, which would have to know which `{` sits inside a string +// literal. It is also the more precise rule: only a method declared directly in a +// top-level `class`/`extension` block is addressable as `Target/Class/method`, so a +// helper type nested inside a test body (`final class ResultBox` — several of these +// exist) contributes no test identifiers, and neither does a closure-local `func test…`. +const DECLARATION = + /^(?:[\w@]+[ \t]+)*(?:class|extension|struct|enum|actor|protocol)[ \t]+([A-Za-z_]\w*)|^ {2}(?:[\w@]+[ \t]+)*func[ \t]+(test\w*)[ \t]*\(/gm; + +export type SwiftSource = { readonly file: string; readonly text: string }; + +/** A declared method and the platforms its `#if` guards let it compile for. */ +export type DeclaredTest = { + readonly identifier: string; + readonly platforms: readonly Platform[]; +}; + +export function readSwiftSources(directory: string): SwiftSource[] { + return fs + .readdirSync(directory) + .filter((entry) => SWIFT_SOURCE.test(entry)) + .sort() + .map((entry) => ({ + file: entry, + text: fs.readFileSync(path.join(directory, entry), 'utf8'), + })); +} + +/** Every `Target/Class/method` identifier the sources declare, sorted, guards ignored. */ +export function parseDeclaredTests(target: string, sources: readonly SwiftSource[]): string[] { + const declared = new Set(); + for (const source of sources) { + let enclosing = ''; + for (const [, type, method] of source.text.matchAll(DECLARATION)) { + if (type !== undefined) enclosing = type; + else if (method !== undefined && enclosing) declared.add(`${target}/${enclosing}/${method}`); + } + } + return [...declared].sort(); +} + +/** The declared identifiers, each with the platforms whose unit-test build compiles it. */ +export function parseDeclaredTestsByPlatform( + target: string, + sources: readonly SwiftSource[], +): DeclaredTest[] { + const compiled = new Map>( + PLATFORMS.map((platform) => [ + platform, + new Set( + parseDeclaredTests( + target, + sources.map((source) => ({ + file: source.file, + text: activeSource(source.text, platform, source.file), + })), + ), + ), + ]), + ); + return parseDeclaredTests(target, sources).map((identifier) => ({ + identifier, + platforms: PLATFORMS.filter((platform) => compiled.get(platform)?.has(identifier)), + })); +} diff --git a/scripts/xctest-run-summary.ts b/scripts/xctest-run-summary.ts index eb7f95a07..3f2709762 100644 --- a/scripts/xctest-run-summary.ts +++ b/scripts/xctest-run-summary.ts @@ -1,19 +1,24 @@ -// Renders the nightly XCTest lane's job summary, and asserts the run executed tests. +// Renders a whole-bundle XCTest lane's job summary, and asserts the run executed exactly the +// tests the source says that lane reaches. // // Two jobs in one place because they read the same file. `xcodebuild` exits 0 when a test // selection matches nothing, so "green" and "ran no tests" produce the same log tail: a // build variant missing the unit-test compile flag, an empty test plan, a renamed target, -// or a `-skip-testing:` entry that swallowed the suite would all read as a healthy night. -// The executed-count assertion is what tells those apart. +// or a `-skip-testing:` entry that swallowed the suite would all read as a healthy run. +// Asserting the executed count against `check:xctest-selection`'s reach for the lane +// (`XCTEST_LANE`) is what tells those apart — and it also catches the quieter failure, a +// `#if` guard that compiles a file of tests out of the lane's platform, which "at least one +// test ran" never would. // // A script rather than `node -e` in the workflow: this is quoting-sensitive string building // inside YAML inside shell, and it has a cap to enforce — the job summary is limited to -// 1 MiB, and the night this lane matters most is the night the failure list is longest. +// 1 MiB, and the run this lane matters most on is the run whose failure list is longest. // // Reads `xcrun xcresulttool get test-results summary` JSON (Xcode 16+ shape). import fs from 'node:fs'; import { pathToFileURL } from 'node:url'; +import { lane, loadReport, type Lane } from './check-xctest-selection.ts'; /** Failure entries beyond this are counted, not listed. */ export const MAX_LISTED_FAILURES = 60; @@ -53,16 +58,16 @@ function failureLine(failure: Failure): string { return `- \`${name}\`${text ? ` — ${text}` : ''}`; } -export function renderSummary(summary: ResultSummary): string { +export function renderSummary(summary: ResultSummary, title: string, expected: number): string { const failures = Array.isArray(summary.testFailures) ? summary.testFailures : []; const shown = failures.slice(0, MAX_LISTED_FAILURES); const lines = [ - '### iOS runner full XCTest suite', + `### ${title}`, '', `- result: **${summary.result ?? 'unknown'}**`, - `- executed: **${summary.totalTestCount ?? 0}** (passed ${summary.passedTests ?? 0}, ` + - `failed ${summary.failedTests ?? 0}, skipped ${summary.skippedTests ?? 0}, ` + - `expected failures ${summary.expectedFailures ?? 0})`, + `- executed: **${summary.totalTestCount ?? 0}** of the ${expected} the source reaches on ` + + `this lane (passed ${summary.passedTests ?? 0}, failed ${summary.failedTests ?? 0}, ` + + `skipped ${summary.skippedTests ?? 0}, expected failures ${summary.expectedFailures ?? 0})`, `- duration: ${duration(summary)}`, ]; if (shown.length > 0) { @@ -75,23 +80,43 @@ export function renderSummary(summary: ResultSummary): string { } /** Null when the run is credible, or the operator-facing reason it is not. */ -export function livenessFailure(summary: ResultSummary): string | null { - if ((summary.totalTestCount ?? 0) >= 1) return null; +export function livenessFailure(summary: ResultSummary, expected: number): string | null { + const executed = summary.totalTestCount ?? 0; + if (expected < 1) { + return [ + 'check:xctest-selection derives no reachable tests for this lane, so no run can be', + 'credible: the source scan or the lane definition is broken.', + ].join('\n'); + } + if (executed === expected) return null; + if (executed === 0) { + return [ + `This lane executed no tests (the source says it reaches ${expected}), which xcodebuild`, + 'reports as success. Check AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS (the -D flag that', + 'compiles the tests in), the xctestrun test plan, the target name, and the -skip-testing', + 'entry.', + ].join('\n'); + } return [ - 'The full-suite lane executed no tests, which xcodebuild reports as success.', - 'Check AGENT_DEVICE_XCUITEST_INCLUDE_UNIT_TESTS (the -D flag that compiles the tests in),', - 'the xctestrun test plan, the target name, and the -skip-testing entry.', + `This lane executed ${executed} test(s) but the source says it reaches ${expected}.`, + 'A `#if` guard compiled tests out of (or into) this platform without the classification', + 'moving with it, or a selection flag matched more or less than it names. Run', + '`pnpm check:xctest-selection` and compare its per-lane reach with the result bundle.', ].join('\n'); } function main(): number { const summaryPath = process.env.RESULT_SUMMARY_PATH; if (!summaryPath) throw new Error('RESULT_SUMMARY_PATH is not set.'); + const current: Lane = lane(process.env.XCTEST_LANE ?? ''); + const expected = loadReport().reach[current.id].size; const summary = JSON.parse(fs.readFileSync(summaryPath, 'utf8')) as ResultSummary; const stepSummary = process.env.GITHUB_STEP_SUMMARY; - if (stepSummary) fs.appendFileSync(stepSummary, renderSummary(summary)); - process.stdout.write(`Executed ${summary.totalTestCount ?? 0} test(s).\n`); - const failure = livenessFailure(summary); + if (stepSummary) fs.appendFileSync(stepSummary, renderSummary(summary, current.title, expected)); + process.stdout.write( + `Executed ${summary.totalTestCount ?? 0} test(s); the source reaches ${expected} on this lane.\n`, + ); + const failure = livenessFailure(summary, expected); if (!failure) return 0; process.stderr.write(`${failure}\n`); return 1;