From a683c12be71598fad89e7c3a452abe576f608e82 Mon Sep 17 00:00:00 2001 From: Jason Prasad Date: Tue, 21 Jul 2026 11:42:16 -0400 Subject: [PATCH 1/3] =?UTF-8?q?tests+ci:=20regress=20guard=20tranche=203?= =?UTF-8?q?=20=E2=80=94=20tool=20rows=20P01/B01/B02/B03/F01/L04?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds RegressToolGuardTests, a manual-tagged sibling swift_test compiling the shared CLIIntegrationTests harness, for the regress-matrix rows that need tooling the required gate's deterministic target must not depend on: generator scripts (P01 cold-build heartbeat, B02/B03 XCFramework renders, F01 classified slice error), a Bazel fetch of the fixture's pinned rules (B01, via an explicit PATH pin — bazelisk is invisible in bazel's scrubbed test env), and an iOS simulator (L04's deterministic simctl-terminate crash/respawn trigger, dedicated device index 9). Tool preconditions skip locally but fail under PREVIEWSMCP_REQUIRE_DEDICATED_SIM so CI can never silently skip a row. The new non-required regress-tools job in ci.yml runs the target on the same labels (needs: ci keeps the serial runner working the required gate first) to build a standing flake record; promotion into the required gate is dropping the manual tag. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 86 +++++ docs/ci.md | 13 + .../Tests/CLIIntegrationTests/BUILD.bazel | 57 +++- .../RegressGuardTests.swift | 7 +- .../RegressToolGuardTests.swift | 300 ++++++++++++++++++ 5 files changed, 459 insertions(+), 4 deletions(-) create mode 100644 previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23dca297..0e03d017 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -191,3 +191,89 @@ jobs: pkill -9 -f '\.xctest' || true killall -9 Simulator || true xcrun simctl shutdown all || true + + # Non-required signal for the regress-matrix rows that need fixture artifact + # generation, a Bazel fetch of a fixture's pinned rules, or an iOS simulator + # (//previewsmcp/Tests/CLIIntegrationTests:RegressToolGuardTests, tagged + # `manual` so the required `ci` job's `//...` expansion never runs it). Runs + # on the same triggers to build a standing flake record; promote into the + # required gate by dropping the `manual` tag once that record is clean. This + # check is NOT in the ruleset's required contexts — a red here never blocks + # auto-merge. `needs: ci` keeps the single serial runner working the required + # gate first, so this job never delays a merge. + regress-tools: + needs: ci + if: >- + github.event_name != 'pull_request' || + github.event.label.name == 'ci' || + github.event.label.name == 'merge' + runs-on: [self-hosted, macOS, ARM64] + timeout-minutes: 60 + permissions: + contents: read + checks: read + env: + DEVELOPER_DIR: /Applications/Xcode-26.2.0.app/Contents/Developer + # Same required-gate coverage signal as `ci`: a host that cannot + # provision the dedicated simulator (or is missing bazel) FAILS the row + # instead of silently skipping it (RegressToolGuardTests.requireTool / + # SimulatorTestDevices.requiresDedicatedSim). + PREVIEWSMCP_REQUIRE_DEDICATED_SIM: "1" + steps: + # Dedup like `ci`: a `merge` label after a green `ci`-label run skips + # the second identical pass. + - name: Skip suite if this commit is already green + id: dedup + if: github.event_name == 'pull_request' && github.event.label.name == 'merge' + env: + GH_TOKEN: ${{ github.token }} + SHA: ${{ github.event.pull_request.head.sha }} + run: | + if gh api "repos/$GITHUB_REPOSITORY/commits/$SHA/check-runs" \ + --jq '.check_runs[] | select(.name=="regress-tools" and .conclusion=="success") | .id' \ + | grep -q .; then + echo "skip=true" >> "$GITHUB_OUTPUT" + echo "A green regress-tools check already exists for $SHA — skipping the suite." + fi + + - if: steps.dedup.outputs.skip != 'true' + uses: actions/checkout@v7 + + # Same clean-host reset as `ci` — this job boots simulators too. + - name: Reset simulator and daemon state + if: steps.dedup.outputs.skip != 'true' + run: | + pkill -9 -f 'previewsmcp serve' || true + pkill -9 -f 'previewsmcp run' || true + pkill -9 -x PreviewAgent || true + pkill -9 -f '\.xctest' || true + killall -9 Simulator || true + xcrun simctl shutdown all || true + launchctl kickstart -k "gui/$(id -u)/com.apple.CoreSimulator.CoreSimulatorService" 2>/dev/null \ + || launchctl kickstart -k "system/com.apple.CoreSimulator.CoreSimulatorService" 2>/dev/null || true + xcrun simctl delete unavailable || true + + - name: Test (regress tool-guard rows) + if: steps.dedup.outputs.skip != 'true' + run: >- + bazel test //previewsmcp/Tests/CLIIntegrationTests:RegressToolGuardTests + --test_output=errors + + - name: Failure diagnostics + if: failure() + run: | + xcrun simctl list devices booted || true + pgrep -fl "previewsmcp|simctl" || true + for f in "${TMPDIR:-/tmp}"previewsmcp-test-*.log /tmp/previewsmcp-test-*.log; do + [ -f "$f" ] && { echo "=== $f (tail)"; tail -100 "$f"; } + done || true + + - name: Reap leaked processes and simulators + if: always() + run: | + pkill -9 -f 'previewsmcp serve' || true + pkill -9 -f 'previewsmcp run' || true + pkill -9 -x PreviewAgent || true + pkill -9 -f '\.xctest' || true + killall -9 Simulator || true + xcrun simctl shutdown all || true diff --git a/docs/ci.md b/docs/ci.md index f2cd11e5..922d953e 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -33,6 +33,19 @@ tier failing still blocks the merge through the same required `ci` check. The `integration` tag is only a selection label; existing `exclusive` tags and runtime simulator locks continue to control scheduling and isolation. +## The non-required `regress-tools` signal + +A second job, `regress-tools`, runs +`//previewsmcp/Tests/CLIIntegrationTests:RegressToolGuardTests` — the +regress-matrix guard rows that need fixture artifact generation, a Bazel fetch +of a fixture's pinned rules, or an iOS simulator. The target is tagged +`manual`, so neither required tier's `bazel test //...` expansion ever runs +it; only this job does. The check is deliberately NOT in the ruleset's +required contexts: it runs on the same labels to build a standing flake +record, and a red never blocks auto-merge. `needs: ci` keeps the single serial +runner working the required gate first. Promote the target into the required +gate by dropping its `manual` tag once the record is clean. + ## Normal flow 1. Open the PR. Nothing runs. diff --git a/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel b/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel index da43ef8d..f2ad5fc7 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel +++ b/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel @@ -1,9 +1,21 @@ load("@rules_swift//swift:swift.bzl", "swift_test") +# Shared subprocess/daemon harness. CLIIntegrationTests picks these up via +# its glob; RegressToolGuardTests compiles them again as its own sources. +HARNESS_SRCS = [ + "CLIRunner.swift", + "CoreSimulatorHygiene.swift", + "DaemonTestLock.swift", + "PipeBuffer.swift", +] + swift_test( name = "CLIIntegrationTests", timeout = "eternal", - srcs = glob(["**/*.swift"]), + srcs = glob( + ["**/*.swift"], + exclude = ["RegressToolGuardTests.swift"], + ), copts = [ "-swift-version", "6", @@ -34,3 +46,46 @@ swift_test( visibility = ["//visibility:public"], deps = ["//previewsmcp/Tests/TestSupport"], ) + +# Guards for regress-matrix rows that need build tooling the deterministic +# target above must not depend on: fixture artifact generation, a Bazel +# fetch of the fixture's pinned rules, or an iOS simulator. Tagged `manual` +# so `bazel test //...` (both required-gate tiers) never expands to it; the +# non-required `regress-tools` job in .github/workflows/ci.yml names it +# explicitly. Promote into the required gate by dropping `manual` once its +# flake record is clean. +swift_test( + name = "RegressToolGuardTests", + timeout = "eternal", + srcs = HARNESS_SRCS + ["RegressToolGuardTests.swift"], + copts = [ + "-swift-version", + "6", + ], + data = [ + "//:ios_jit_resources", + "//:orc_rt_archive", + "//previewsmcp/PreviewAgent", + "//previewsmcp/cli:previewsmcp", + ], + env = { + "PREVIEWSMCP_AGENT": "$(rlocationpath //previewsmcp/PreviewAgent)", + "PREVIEWSMCP_BINARY": "$(rlocationpath //previewsmcp/cli:previewsmcp)", + "PREVIEWSMCP_IOS_JIT_DIR": "$(rlocationpath //:ios_jit_resources)", + "PREVIEWSMCP_ORC_RT": "$(rlocationpath //:orc_rt_archive)", + # Bazel scrubs PATH to /bin:/usr/bin:/usr/local/bin in test actions, + # which hides Homebrew's bazelisk from the CLI's `/usr/bin/env bazel` + # (the reason SnapshotCommandTests' bazel test tool-gate skips under + # `bazel test`). B01 must not skip, so this target pins an explicit + # PATH that includes /opt/homebrew/bin. + "PATH": "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin", + }, + tags = [ + "exclusive", + "integration", + "local", + "manual", + ], + visibility = ["//visibility:public"], + deps = ["//previewsmcp/Tests/TestSupport"], +) diff --git a/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift b/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift index ec922bd5..2892a657 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift +++ b/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift @@ -24,9 +24,10 @@ import Testing /// classification, and phase-clock contracts on plain SwiftPM fixtures. /// Rows staying manual-only for a named flake reason: W03 (FSEvents timing, /// #298), L05 (concurrency), S05/S06 (swift-syntax fetch), M01/M02 -/// (launch assertions elsewhere). Future tranches that need simulators, -/// artifact generation, or network must land in a separate test target, -/// not this file — this target's glob feeds the required `ci` gate. +/// (launch assertions elsewhere). Rows that need simulators, artifact +/// generation, or network must land in `RegressToolGuardTests` (the +/// `manual`-tagged sibling target, run by the non-required `regress-tools` +/// job), not this file — this target's glob feeds the required `ci` gate. @Suite("Regress guard rows", .serialized) struct RegressGuardTests { private static func cleanSlate() async throws { diff --git a/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift b/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift new file mode 100644 index 00000000..a55966d4 --- /dev/null +++ b/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift @@ -0,0 +1,300 @@ +import AppKit +import Foundation +import PreviewsTestSupport +import Testing + +/// Automated guards for `examples/regress` matrix rows that need build +/// tooling the deterministic `RegressGuardTests` target must not depend +/// on: fixture artifact generation (`binary-frameworks/ +/// generate-artifacts.sh`, `large-tier2/generate-sources.sh`), a Bazel +/// fetch of the fixture's pinned rules (B01), or an iOS simulator. Row +/// conventions match `RegressGuardTests`: one test per row, the row's +/// healthy-result contract, no detection overrides, presence-only tick +/// regexes, `.minutes(10)` time limits (the DaemonTestLock rule). +/// +/// This target is tagged `manual`, so neither required-gate tier's +/// `bazel test //...` expansion runs it; the non-required `regress-tools` +/// job in `.github/workflows/ci.yml` names it explicitly for signal. +/// Tool preconditions skip locally but FAIL when +/// `SimulatorTestDevices.requiresDedicatedSim` is set (the required-gate +/// coverage signal ci.yml exports), so the CI job can never silently +/// skip a row. +@Suite("Regress tool-guard rows", .serialized) +struct RegressToolGuardTests { + private static func cleanSlate() async throws { + _ = try? await CLIRunner.run("kill-daemon", arguments: ["--timeout", "2"]) + } + + private static func fixture(_ relativePath: String) -> String { + CLIRunner.regressRoot.appendingPathComponent(relativePath).path + } + + /// Run one of the fixture generator scripts, exactly as the manual + /// matrix pass does (`VERIFICATION.md` repeatability notes). The + /// scripts regenerate from scratch on every run, so a stale artifact + /// can never satisfy a guard. + private static func generate( + _ scriptRelativePath: String, environment: [String] = [] + ) async throws { + let result = try await CLIRunner.runExternal( + "/usr/bin/env", + arguments: environment + ["/bin/bash", fixture(scriptRelativePath)] + ) + try #require( + result.exitCode == 0, + "\(scriptRelativePath) failed: \(result.stdout)\n\(result.stderr)" + ) + } + + /// Skip (locally) or fail (on the gate) when `tool` is not reachable + /// on this target's pinned PATH. Returns false to skip. + private static func requireTool(_ tool: String) async throws -> Bool { + if await CLIRunner.toolAvailable(tool) { return true } + try #require( + !SimulatorTestDevices.requiresDedicatedSim, + "\(tool) is required on the regress-tools gate but is not available" + ) + print("\(tool) not available — skipping") + return false + } + + @discardableResult + private static func assertRenders( + _ relativePath: String, + extraArguments: [String] = [], + thenWhileAlive: @Sendable () async throws -> Void = {} + ) async throws -> CLIResult { + try await DaemonTestLock.run { + try await cleanSlate() + let tempDir = try CLIRunner.makeTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + + let outputPath = tempDir.appendingPathComponent("snapshot.png").path + let result = try await CLIRunner.run( + "snapshot", + arguments: [fixture(relativePath), "-o", outputPath] + extraArguments + ) + #expect(result.exitCode == 0, "stderr: \(result.stderr)") + try CLIRunner.assertValidPNG(at: outputPath) + try CLIRunner.assertNonBlankPNG(at: outputPath) + try await thenWhileAlive() + return result + } + } + + private static func assertFails( + _ relativePath: String, + extraArguments: [String] = [], + containing expected: [String], + thenWhileAlive: @Sendable () async throws -> Void = {} + ) async throws { + try await DaemonTestLock.run { + try await cleanSlate() + let tempDir = try CLIRunner.makeTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + + let outputPath = tempDir.appendingPathComponent("snapshot.png").path + let result = try await CLIRunner.run( + "snapshot", + arguments: [fixture(relativePath), "-o", outputPath] + extraArguments + ) + #expect(result.exitCode != 0, "expected a classified failure, got success") + let combined = result.stdout + result.stderr + for token in expected { + #expect( + combined.contains(token), + "diagnostic should contain '\(token)'; got: \(combined)" + ) + } + try await thenWhileAlive() + } + } + + // MARK: - Artifact-generation rows + + /// P01: a cold build of a 2,000-file target ticks an elapsed-time + /// heartbeat on the build phase (phase/error stage 2). The fixture's + /// SwiftPM products are cleaned first — the row's contract is the + /// previously silent COLD interval. Presence-only tick regex. + @Test("P01: cold large-target build heartbeats", .timeLimit(.minutes(10))) + func p01ColdBuildHeartbeat() async throws { + try await Self.generate( + "large-tier2/generate-sources.sh", environment: ["FILE_COUNT=2000"] + ) + try? FileManager.default.removeItem( + at: CLIRunner.regressRoot.appendingPathComponent("large-tier2/.build") + ) + let result = try await Self.assertRenders( + "large-tier2/Sources/LargeTier2/LargeTier2Preview.swift" + ) + #expect( + result.stderr.range( + of: #"Building \(SPMBuildSystem\)\.\.\. \(\d+s\)"#, options: .regularExpression + ) != nil, + "build phase should tick an elapsed heartbeat: \(result.stderr)" + ) + } + + // MARK: - iOS simulator support + + /// Provision the dedicated device for this target, holding + /// `SimulatorTestLock` as `SimulatorTestDevices.udid` requires, and + /// reset host CoreSimulator state once before the first boot. + /// Returns nil to skip locally; on the gate a missing device throws + /// (`requiresDedicatedSim`). + private static func provisionSimulator() async throws + -> (udid: String, lock: SimulatorTestLock.Guard)? + { + let simLock = try await SimulatorTestLock.acquire() + guard let udid = try await SimulatorTestDevices.udid(index: 9) else { + print("Host cannot create \(SimulatorTestDevices.name(index: 9)) — skipping") + simLock.release() + return nil + } + await CoreSimulatorHygiene.resetOnce() + return (udid, simLock) + } + + // MARK: - Binary-framework rows (iOS) + + /// B02: the combined static + dynamic XCFramework package renders on + /// the iOS simulator — the captured flags resolve the static module + /// and its copied archive links alongside the dynamic framework. + @Test("B02: combined XCFrameworks render on iOS", .timeLimit(.minutes(10))) + func b02CombinedXCFrameworks() async throws { + guard let sim = try await Self.provisionSimulator() else { return } + defer { sim.lock.release() } + try await Self.generate("binary-frameworks/generate-artifacts.sh") + try await Self.assertRenders( + "binary-frameworks/combined/Sources/CombinedBinaryFixture/BinaryFrameworkPreview.swift", + extraArguments: ["--platform", "ios", "--device", sim.udid] + ) + } + + /// B03: the static-only XCFramework package renders on the iOS + /// simulator — the module resolves from the captured flags and the + /// copied `libStaticBadge.a` links from binPath. + @Test("B03: static XCFramework renders on iOS", .timeLimit(.minutes(10))) + func b03StaticXCFramework() async throws { + guard let sim = try await Self.provisionSimulator() else { return } + defer { sim.lock.release() } + try await Self.generate("binary-frameworks/generate-artifacts.sh") + try await Self.assertRenders( + "binary-frameworks/static-only/Sources/StaticBinaryFixture/StaticBinaryPreview.swift", + extraArguments: ["--platform", "ios", "--device", sim.udid] + ) + } + + /// F01: an XCFramework with no iOS simulator slice fails the iOS + /// start with the classified slice error naming the available + /// identifiers (phase/error stage 4); the daemon stays responsive. + @Test("F01: missing simulator slice is classified", .timeLimit(.minutes(10))) + func f01BadSliceClassified() async throws { + guard let sim = try await Self.provisionSimulator() else { return } + defer { sim.lock.release() } + try await Self.generate("binary-frameworks/generate-artifacts.sh") + try await Self.assertFails( + "binary-frameworks/bad-slice/Sources/BadSliceFixture/BadSlicePreview.swift", + extraArguments: ["--platform", "ios", "--device", sim.udid], + containing: ["has no iOS simulator slice", "ios-arm64"] + ) { + let status = try await CLIRunner.run("status") + #expect(status.exitCode == 0, "daemon should survive the classified slice error") + #expect(status.stdout.contains("daemon running"), "status: \(status.stdout)") + } + } + + // MARK: - Lifecycle row (iOS) + + /// L04: an out-of-band agent death (deterministic trigger: + /// `simctl terminate` on the agent bundle) is logged as a crash and + /// respawned; the next command succeeds and carries the crash notice, + /// and the notice clears on delivery. + @Test("L04: agent kill respawns with a crash notice", .timeLimit(.minutes(10))) + func l04AgentKillRespawns() async throws { + guard let sim = try await Self.provisionSimulator() else { return } + defer { sim.lock.release() } + try await DaemonTestLock.run { + try await Self.cleanSlate() + let runResult = try await CLIRunner.run( + "run", + arguments: [ + Self.fixture("lifecycle-faults/Sources/AgentCrash/AgentCrashPreview.swift"), + "--platform", "ios", "--device", sim.udid, "--detach", "--headless", + ] + ) + #expect(runResult.exitCode == 0, "detach stderr: \(runResult.stderr)") + + let terminate = try await CLIRunner.runExternal( + "/usr/bin/xcrun", + arguments: ["simctl", "terminate", sim.udid, "com.previewsmcp.agent"] + ) + try #require(terminate.exitCode == 0, "terminate stderr: \(terminate.stderr)") + + let clock = SuspendingClock() + var deadline = clock.now.advanced(by: .seconds(60)) + var respawnLogged = false + while clock.now < deadline { + let logs = try await CLIRunner.run("logs", arguments: ["-n", "200"]) + if logs.stdout.contains("agent died out of band (crash #1); respawning") { + respawnLogged = true + break + } + try await clock.sleep(for: .milliseconds(500)) + } + #expect(respawnLogged, "daemon should log the out-of-band death and respawn") + + // The respawn log line marks relaunch START; commands issued + // mid-relaunch fail before a response is assembled, which + // leaves the crash notice unconsumed. Retry until the respawned + // agent serves a response — that first success must carry the + // notice. + deadline = clock.now.advanced(by: .seconds(120)) + var elements = try await CLIRunner.run("elements") + while elements.exitCode != 0, clock.now < deadline { + try await clock.sleep(for: .seconds(2)) + elements = try await CLIRunner.run("elements") + } + #expect(elements.exitCode == 0, "post-crash elements stderr: \(elements.stderr)") + let combined = elements.stdout + elements.stderr + #expect( + combined.contains("The preview agent crashed and was relaunched"), + "first post-crash response should carry the crash notice: \(combined)" + ) + #expect( + combined.contains("crash #1"), + "notice should count the crash: \(combined)" + ) + + let followUp = try await CLIRunner.run("elements") + #expect(followUp.exitCode == 0, "follow-up stderr: \(followUp.stderr)") + #expect( + !(followUp.stdout + followUp.stderr) + .contains("The preview agent crashed and was relaunched"), + "crash notice must clear on delivery: \(followUp.stdout)" + ) + + let stop = try await CLIRunner.run("stop") + #expect(stop.exitCode == 0, "stop stderr: \(stop.stderr)") + } + } + + // MARK: - Bazel row + + /// B01: the Bzlmod fixture renders through the aquery capture — the + /// genrule source resolves at its execroot path and the canonical + /// external repo's search path works. A capture regression fails the + /// fixture build, so exit code + ownership log are the observables. + @Test("B01: bzlmod fixture renders via aquery capture", .timeLimit(.minutes(10))) + func b01BazelBzlmod() async throws { + guard try await Self.requireTool("bazel") else { return } + try await Self.assertRenders("bazel-bzlmod/Sources/BzlmodPreview.swift") { + let logs = try await CLIRunner.run("logs", arguments: ["-n", "300"]) + #expect(logs.exitCode == 0, "logs stderr: \(logs.stderr)") + let confirmed = logs.stdout.split(separator: "\n").contains { + $0.contains("ownership: bazel confirmed") && $0.contains("BzlmodPreview.swift") + } + #expect(confirmed, "daemon log should record bazel confirming BzlmodPreview.swift") + } + } +} From 25c26cfeaeb302bb9a3e4cd606449acd87d586fe Mon Sep 17 00:00:00 2001 From: Jason Prasad Date: Tue, 21 Jul 2026 12:03:13 -0400 Subject: [PATCH 2/3] =?UTF-8?q?tests:=20fold=20gate=20findings=20=E2=80=94?= =?UTF-8?q?=20shared=20row=20asserts,=20one=20artifact=20pass,=20gate=20ac?= =?UTF-8?q?cessor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /simplify fold: assertRenders/assertFails/cleanSlate/fixture move to RegressRowAsserts (compiled into both guard targets via HARNESS_SRCS) so the two suites cannot drift; the three binary-framework rows share one per-process artifact generation; the fail-on-gate-vs-skip-locally policy gets its general name (RequiredGateEnforcement.enforced) instead of riding the sim-specific flag, which now delegates to it. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 5 +- .../Tests/CLIIntegrationTests/BUILD.bazel | 1 + .../RegressGuardTests.swift | 117 ++++-------------- .../RegressRowAsserts.swift | 78 ++++++++++++ .../RegressToolGuardTests.swift | 104 ++++------------ .../TestSupport/SimulatorTestDevices.swift | 18 ++- 6 files changed, 151 insertions(+), 172 deletions(-) create mode 100644 previewsmcp/Tests/CLIIntegrationTests/RegressRowAsserts.swift diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e03d017..8625421a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -216,8 +216,9 @@ jobs: DEVELOPER_DIR: /Applications/Xcode-26.2.0.app/Contents/Developer # Same required-gate coverage signal as `ci`: a host that cannot # provision the dedicated simulator (or is missing bazel) FAILS the row - # instead of silently skipping it (RegressToolGuardTests.requireTool / - # SimulatorTestDevices.requiresDedicatedSim). + # instead of silently skipping it (RequiredGateEnforcement.enforced in + # TestSupport, read by RegressToolGuardTests.requireTool and the sim + # resolver). PREVIEWSMCP_REQUIRE_DEDICATED_SIM: "1" steps: # Dedup like `ci`: a `merge` label after a green `ci`-label run skips diff --git a/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel b/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel index f2ad5fc7..cbdda54a 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel +++ b/previewsmcp/Tests/CLIIntegrationTests/BUILD.bazel @@ -7,6 +7,7 @@ HARNESS_SRCS = [ "CoreSimulatorHygiene.swift", "DaemonTestLock.swift", "PipeBuffer.swift", + "RegressRowAsserts.swift", ] swift_test( diff --git a/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift b/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift index 2892a657..fe13104d 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift +++ b/previewsmcp/Tests/CLIIntegrationTests/RegressGuardTests.swift @@ -30,73 +30,6 @@ import Testing /// job), not this file — this target's glob feeds the required `ci` gate. @Suite("Regress guard rows", .serialized) struct RegressGuardTests { - private static func cleanSlate() async throws { - _ = try? await CLIRunner.run("kill-daemon", arguments: ["--timeout", "2"]) - } - - private static func fixture(_ relativePath: String) -> String { - CLIRunner.regressRoot.appendingPathComponent(relativePath).path - } - - /// Run a one-shot snapshot of `relativePath` with no detection overrides - /// and assert it renders a valid, non-blank PNG. Returns the CLI result - /// so callers can pin stderr notice/progress tokens. `thenWhileAlive` - /// runs inside the same lock block, before the writer-fence kills the - /// daemon, for assertions that need the daemon's state (logs, status). - @discardableResult - private static func assertRenders( - _ relativePath: String, - extraArguments: [String] = [], - thenWhileAlive: @Sendable () async throws -> Void = {} - ) async throws -> CLIResult { - try await DaemonTestLock.run { - try await cleanSlate() - let tempDir = try CLIRunner.makeTempDir() - defer { try? FileManager.default.removeItem(at: tempDir) } - - let outputPath = tempDir.appendingPathComponent("snapshot.png").path - let result = try await CLIRunner.run( - "snapshot", - arguments: [fixture(relativePath), "-o", outputPath] + extraArguments - ) - #expect(result.exitCode == 0, "stderr: \(result.stderr)") - try CLIRunner.assertValidPNG(at: outputPath) - try CLIRunner.assertNonBlankPNG(at: outputPath) - try await thenWhileAlive() - return result - } - } - - /// Run a one-shot snapshot of `relativePath` and assert it fails with - /// every expected diagnostic token in the combined output. `thenWhileAlive` - /// runs inside the same lock block, before the writer-fence kills the - /// daemon. - private static func assertFails( - _ relativePath: String, - containing expected: [String], - thenWhileAlive: @Sendable () async throws -> Void = {} - ) async throws { - try await DaemonTestLock.run { - try await cleanSlate() - let tempDir = try CLIRunner.makeTempDir() - defer { try? FileManager.default.removeItem(at: tempDir) } - - let outputPath = tempDir.appendingPathComponent("snapshot.png").path - let result = try await CLIRunner.run( - "snapshot", arguments: [fixture(relativePath), "-o", outputPath] - ) - #expect(result.exitCode != 0, "expected a classified failure, got success") - let combined = result.stdout + result.stderr - for token in expected { - #expect( - combined.contains(token), - "diagnostic should contain '\(token)'; got: \(combined)" - ) - } - try await thenWhileAlive() - } - } - /// Assert the daemon log records the ownership walk confirming `kind` /// for `fileName`. This is the observable for rows whose regression /// still renders (a wrong build system compiling the same source). @@ -119,7 +52,7 @@ struct RegressGuardTests { /// Previously the distant root claimed the file and failed. @Test("D01: nested Xcode project below a Bazel root", .timeLimit(.minutes(10))) func d01NestedXcodeBelowBazelRoot() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "detection/mixed-marker-workspace/XcodeOnlyApp/Sources/MarkerPreview.swift" ) { try await Self.assertOwnershipLogged(kind: "xcode", fileName: "MarkerPreview.swift") @@ -129,7 +62,7 @@ struct RegressGuardTests { /// D02: a Swift package nested below a Bazel root is selected and renders. @Test("D02: nested package below a Bazel root", .timeLimit(.minutes(10))) func d02NestedPackageBelowBazelRoot() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "detection/mixed-marker-workspace/NestedPackage/Sources/NestedPackage/NestedPackagePreview.swift" ) { try await Self.assertOwnershipLogged(kind: "spm", fileName: "NestedPackagePreview.swift") @@ -140,7 +73,7 @@ struct RegressGuardTests { /// the outer package must not claim the file. @Test("D03: nested Xcode project below an outer package", .timeLimit(.minutes(10))) func d03NestedXcodeBelowOuterPackage() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "detection/outer-spm-workspace/NestedXcode/Sources/OuterBoundaryPreview.swift" ) { try await Self.assertOwnershipLogged( @@ -155,7 +88,7 @@ struct RegressGuardTests { /// daemon's ownership log line, not the render. @Test("D05: same-directory marker tie-break", .timeLimit(.minutes(10))) func d05SameDirectoryMarkers() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "detection/same-directory-markers/Sources/HybridMarker/HybridMarkerPreview.swift" ) { try await Self.assertOwnershipLogged(kind: "spm", fileName: "HybridMarkerPreview.swift") @@ -166,7 +99,7 @@ struct RegressGuardTests { /// the missing-output-specific message and the regeneration hint. @Test("D06: missing generated project diagnosis", .timeLimit(.minutes(10))) func d06MissingGeneratedOutput() async throws { - try await Self.assertFails( + try await RegressRowAsserts.assertFails( "generated-project-state/missing-output/Sources/MissingOutputPreview.swift", containing: ["no generated .xcodeproj", "xcodegen generate"] ) @@ -177,7 +110,7 @@ struct RegressGuardTests { /// prose: the connective sentence may be reworded. @Test("D07: stale generated project diagnosis", .timeLimit(.minutes(10))) func d07StaleGeneratedOutput() async throws { - try await Self.assertFails( + try await RegressRowAsserts.assertFails( "generated-project-state/stale-output/Sources/NewPreview.swift", containing: ["StaleOutput.xcodeproj", "NewPreview.swift", "stale"] ) @@ -188,7 +121,7 @@ struct RegressGuardTests { /// sibling module and failed. @Test("D08: multi-target scheme selects the owning target", .timeLimit(.minutes(10))) func d08MultiTargetOwnership() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "generated-project-state/multi-target/Sources/Beta/BetaPreview.swift" ) { try await Self.assertOwnershipLogged(kind: "xcode", fileName: "BetaPreview.swift") @@ -204,7 +137,7 @@ struct RegressGuardTests { /// header. @Test("X02: bridging header forwarded and ObjC linked", .timeLimit(.minutes(10))) func x02BridgingHeader() async throws { - try await Self.assertRenders("xcode-bridging/Sources/BridgingPreview.swift") + try await RegressRowAsserts.assertRenders("xcode-bridging/Sources/BridgingPreview.swift") } // MARK: - SwiftPM compile-capture rows @@ -216,7 +149,7 @@ struct RegressGuardTests { /// non-blank PNG covers the whole contract. @Test("S01: captured settings fixture renders", .timeLimit(.minutes(10))) func s01SettingsFixture() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "spm-settings/Sources/SettingsFixture/SettingsPreview.swift" ) } @@ -226,7 +159,7 @@ struct RegressGuardTests { /// a dropped define (#error). @Test("S02: compiler settings preserved", .timeLimit(.minutes(10))) func s02CompilerSettings() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "spm-settings/Sources/CompilerSettings/CompilerSettingsPreview.swift" ) } @@ -235,7 +168,7 @@ struct RegressGuardTests { /// compile input (a miss is a compile error). @Test("S03: plugin-generated source compiles", .timeLimit(.minutes(10))) func s03GeneratedPlugin() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "spm-settings/Sources/GeneratedPlugin/GeneratedPluginPreview.swift" ) } @@ -244,7 +177,7 @@ struct RegressGuardTests { /// resolves (a miss is a compile error). @Test("S04: membership exclusion and C module", .timeLimit(.minutes(10))) func s04MembershipAndC() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "spm-settings/Sources/MembershipAndC/MembershipAndCPreview.swift" ) } @@ -254,7 +187,7 @@ struct RegressGuardTests { /// V01: a legacy `PreviewProvider` declaration renders. @Test("V01: legacy PreviewProvider renders", .timeLimit(.minutes(10))) func v01LegacyProvider() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "preview-forms/Sources/PreviewForms/LegacyProvider.swift" ) } @@ -266,11 +199,11 @@ struct RegressGuardTests { @Test("V03: duplicate names select by index", .timeLimit(.minutes(10))) func v03DuplicateNames() async throws { try await DaemonTestLock.run { - try await Self.cleanSlate() + try await RegressRowAsserts.cleanSlate() let tempDir = try CLIRunner.makeTempDir() defer { try? FileManager.default.removeItem(at: tempDir) } - let file = Self.fixture("preview-forms/Sources/PreviewForms/DuplicateNames.swift") + let file = RegressRowAsserts.fixture("preview-forms/Sources/PreviewForms/DuplicateNames.swift") let output0 = tempDir.appendingPathComponent("index0.png").path let output1 = tempDir.appendingPathComponent("index1.png").path @@ -295,7 +228,7 @@ struct RegressGuardTests { /// V04: a preview in a constrained generic context compiles and renders. @Test("V04: constrained generic context renders", .timeLimit(.minutes(10))) func v04GenericContext() async throws { - try await Self.assertRenders( + try await RegressRowAsserts.assertRenders( "preview-forms/Sources/PreviewForms/GenericContext.swift" ) } @@ -304,7 +237,7 @@ struct RegressGuardTests { /// zero-preview diagnostic (a deliberate UX string, pinned verbatim). @Test("V05: zero-preview diagnostic", .timeLimit(.minutes(10))) func v05NoPreview() async throws { - try await Self.assertFails( + try await RegressRowAsserts.assertFails( "preview-forms/Sources/PreviewForms/NoPreview.swift", containing: ["Preview index 0 not found. File has 0 preview(s)."] ) @@ -321,13 +254,13 @@ struct RegressGuardTests { @Test("C03/C04/C05: config discovery fresh per start", .timeLimit(.minutes(10))) func configRowsFreshPerStart() async throws { try await DaemonTestLock.run { - try await Self.cleanSlate() + try await RegressRowAsserts.cleanSlate() let tempDir = try CLIRunner.makeTempDir() defer { try? FileManager.default.removeItem(at: tempDir) } - let nearer = URL(fileURLWithPath: Self.fixture("config-cache/Nested/.previewsmcp.json")) + let nearer = URL(fileURLWithPath: RegressRowAsserts.fixture("config-cache/Nested/.previewsmcp.json")) try? FileManager.default.removeItem(at: nearer) defer { try? FileManager.default.removeItem(at: nearer) } - let source = Self.fixture( + let source = RegressRowAsserts.fixture( "config-cache/Nested/Sources/ConfigCache/ConfigCachePreview.swift" ) @@ -369,7 +302,7 @@ struct RegressGuardTests { /// the block ends. @Test("T02: setup build failure is classified", .timeLimit(.minutes(10))) func t02SetupBuildFailure() async throws { - try await Self.assertFails( + try await RegressRowAsserts.assertFails( "setup-faults/build-failure/Sources/SetupFaultApp/SetupFaultPreview.swift", containing: ["Setup package 'BrokenPreviewSetup' build failed"] ) { @@ -385,7 +318,7 @@ struct RegressGuardTests { /// response's notices was found writing this guard). @Test("T01: throwing setup renders with a warning notice", .timeLimit(.minutes(10))) func t01ThrowingSetupNotice() async throws { - let result = try await Self.assertRenders( + let result = try await RegressRowAsserts.assertRenders( "setup-faults/throwing/Sources/SetupFaultApp/SetupFaultPreview.swift" ) #expect( @@ -404,7 +337,7 @@ struct RegressGuardTests { /// value) — never scheduler-dependent counts. @Test("T03: slow setup heartbeats and precedes render", .timeLimit(.minutes(10))) func t03SlowSetupHeartbeat() async throws { - let result = try await Self.assertRenders( + let result = try await RegressRowAsserts.assertRenders( "setup-faults/slow/Sources/SetupFaultApp/SetupFaultPreview.swift" ) #expect( @@ -428,7 +361,7 @@ struct RegressGuardTests { /// "Symbols not found" WITH the failure); the daemon stays alive. @Test("L02: unresolved symbol is classified, daemon survives", .timeLimit(.minutes(10))) func l02UnresolvedSymbol() async throws { - try await Self.assertFails( + try await RegressRowAsserts.assertFails( "lifecycle-faults/Sources/MissingSymbol/MissingSymbolPreview.swift", containing: [ "JIT link could not resolve", @@ -445,7 +378,7 @@ struct RegressGuardTests { /// previously silent interval. Presence-only, like T03. @Test("L03: slow render heartbeats", .timeLimit(.minutes(10))) func l03SlowRenderHeartbeat() async throws { - let result = try await Self.assertRenders( + let result = try await RegressRowAsserts.assertRenders( "lifecycle-faults/Sources/SlowRender/SlowRenderPreview.swift" ) #expect( diff --git a/previewsmcp/Tests/CLIIntegrationTests/RegressRowAsserts.swift b/previewsmcp/Tests/CLIIntegrationTests/RegressRowAsserts.swift new file mode 100644 index 00000000..d3eafe3a --- /dev/null +++ b/previewsmcp/Tests/CLIIntegrationTests/RegressRowAsserts.swift @@ -0,0 +1,78 @@ +import Foundation +import Testing + +/// Shared row-contract helpers for the regress guard suites +/// (`RegressGuardTests` in the required-gate target, +/// `RegressToolGuardTests` in the `manual` tool target). Both targets +/// compile this file (`HARNESS_SRCS`), so the snapshot/PNG/lock +/// composition cannot drift between the two suites. +enum RegressRowAsserts { + static func cleanSlate() async throws { + _ = try? await CLIRunner.run("kill-daemon", arguments: ["--timeout", "2"]) + } + + static func fixture(_ relativePath: String) -> String { + CLIRunner.regressRoot.appendingPathComponent(relativePath).path + } + + /// Run a one-shot snapshot of `relativePath` with no detection overrides + /// and assert it renders a valid, non-blank PNG. Returns the CLI result + /// so callers can pin stderr notice/progress tokens. `thenWhileAlive` + /// runs inside the same lock block, before the writer-fence kills the + /// daemon, for assertions that need the daemon's state (logs, status). + @discardableResult + static func assertRenders( + _ relativePath: String, + extraArguments: [String] = [], + thenWhileAlive: @Sendable () async throws -> Void = {} + ) async throws -> CLIResult { + try await DaemonTestLock.run { + try await cleanSlate() + let tempDir = try CLIRunner.makeTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + + let outputPath = tempDir.appendingPathComponent("snapshot.png").path + let result = try await CLIRunner.run( + "snapshot", + arguments: [fixture(relativePath), "-o", outputPath] + extraArguments + ) + #expect(result.exitCode == 0, "stderr: \(result.stderr)") + try CLIRunner.assertValidPNG(at: outputPath) + try CLIRunner.assertNonBlankPNG(at: outputPath) + try await thenWhileAlive() + return result + } + } + + /// Run a one-shot snapshot of `relativePath` and assert it fails with + /// every expected diagnostic token in the combined output. `thenWhileAlive` + /// runs inside the same lock block, before the writer-fence kills the + /// daemon. + static func assertFails( + _ relativePath: String, + extraArguments: [String] = [], + containing expected: [String], + thenWhileAlive: @Sendable () async throws -> Void = {} + ) async throws { + try await DaemonTestLock.run { + try await cleanSlate() + let tempDir = try CLIRunner.makeTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + + let outputPath = tempDir.appendingPathComponent("snapshot.png").path + let result = try await CLIRunner.run( + "snapshot", + arguments: [fixture(relativePath), "-o", outputPath] + extraArguments + ) + #expect(result.exitCode != 0, "expected a classified failure, got success") + let combined = result.stdout + result.stderr + for token in expected { + #expect( + combined.contains(token), + "diagnostic should contain '\(token)'; got: \(combined)" + ) + } + try await thenWhileAlive() + } + } +} diff --git a/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift b/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift index a55966d4..7066683f 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift +++ b/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift @@ -16,29 +16,21 @@ import Testing /// `bazel test //...` expansion runs it; the non-required `regress-tools` /// job in `.github/workflows/ci.yml` names it explicitly for signal. /// Tool preconditions skip locally but FAIL when -/// `SimulatorTestDevices.requiresDedicatedSim` is set (the required-gate -/// coverage signal ci.yml exports), so the CI job can never silently -/// skip a row. +/// `RequiredGateEnforcement.enforced` is set (the required-gate coverage +/// signal ci.yml exports), so the CI job can never silently skip a row. @Suite("Regress tool-guard rows", .serialized) struct RegressToolGuardTests { - private static func cleanSlate() async throws { - _ = try? await CLIRunner.run("kill-daemon", arguments: ["--timeout", "2"]) - } - - private static func fixture(_ relativePath: String) -> String { - CLIRunner.regressRoot.appendingPathComponent(relativePath).path - } - /// Run one of the fixture generator scripts, exactly as the manual /// matrix pass does (`VERIFICATION.md` repeatability notes). The - /// scripts regenerate from scratch on every run, so a stale artifact - /// can never satisfy a guard. + /// scripts regenerate from scratch, so a stale artifact can never + /// satisfy a guard. private static func generate( _ scriptRelativePath: String, environment: [String] = [] ) async throws { let result = try await CLIRunner.runExternal( "/usr/bin/env", - arguments: environment + ["/bin/bash", fixture(scriptRelativePath)] + arguments: environment + + ["/bin/bash", RegressRowAsserts.fixture(scriptRelativePath)] ) try #require( result.exitCode == 0, @@ -46,70 +38,28 @@ struct RegressToolGuardTests { ) } + /// One fresh XCFramework generation per test process, shared by the + /// three binary-framework rows (B02/B03/F01) — the script rebuilds + /// every artifact from scratch, so a second run in the same process + /// buys nothing but ~15s of xcodebuild on the serial runner. The Task + /// runs `generate` lazily on first await and caches its outcome + /// (including a failure) for the rest. + private static let binaryFrameworkArtifacts = Task { + try await generate("binary-frameworks/generate-artifacts.sh") + } + /// Skip (locally) or fail (on the gate) when `tool` is not reachable /// on this target's pinned PATH. Returns false to skip. private static func requireTool(_ tool: String) async throws -> Bool { if await CLIRunner.toolAvailable(tool) { return true } try #require( - !SimulatorTestDevices.requiresDedicatedSim, + !RequiredGateEnforcement.enforced, "\(tool) is required on the regress-tools gate but is not available" ) print("\(tool) not available — skipping") return false } - @discardableResult - private static func assertRenders( - _ relativePath: String, - extraArguments: [String] = [], - thenWhileAlive: @Sendable () async throws -> Void = {} - ) async throws -> CLIResult { - try await DaemonTestLock.run { - try await cleanSlate() - let tempDir = try CLIRunner.makeTempDir() - defer { try? FileManager.default.removeItem(at: tempDir) } - - let outputPath = tempDir.appendingPathComponent("snapshot.png").path - let result = try await CLIRunner.run( - "snapshot", - arguments: [fixture(relativePath), "-o", outputPath] + extraArguments - ) - #expect(result.exitCode == 0, "stderr: \(result.stderr)") - try CLIRunner.assertValidPNG(at: outputPath) - try CLIRunner.assertNonBlankPNG(at: outputPath) - try await thenWhileAlive() - return result - } - } - - private static func assertFails( - _ relativePath: String, - extraArguments: [String] = [], - containing expected: [String], - thenWhileAlive: @Sendable () async throws -> Void = {} - ) async throws { - try await DaemonTestLock.run { - try await cleanSlate() - let tempDir = try CLIRunner.makeTempDir() - defer { try? FileManager.default.removeItem(at: tempDir) } - - let outputPath = tempDir.appendingPathComponent("snapshot.png").path - let result = try await CLIRunner.run( - "snapshot", - arguments: [fixture(relativePath), "-o", outputPath] + extraArguments - ) - #expect(result.exitCode != 0, "expected a classified failure, got success") - let combined = result.stdout + result.stderr - for token in expected { - #expect( - combined.contains(token), - "diagnostic should contain '\(token)'; got: \(combined)" - ) - } - try await thenWhileAlive() - } - } - // MARK: - Artifact-generation rows /// P01: a cold build of a 2,000-file target ticks an elapsed-time @@ -124,7 +74,7 @@ struct RegressToolGuardTests { try? FileManager.default.removeItem( at: CLIRunner.regressRoot.appendingPathComponent("large-tier2/.build") ) - let result = try await Self.assertRenders( + let result = try await RegressRowAsserts.assertRenders( "large-tier2/Sources/LargeTier2/LargeTier2Preview.swift" ) #expect( @@ -164,8 +114,8 @@ struct RegressToolGuardTests { func b02CombinedXCFrameworks() async throws { guard let sim = try await Self.provisionSimulator() else { return } defer { sim.lock.release() } - try await Self.generate("binary-frameworks/generate-artifacts.sh") - try await Self.assertRenders( + try await Self.binaryFrameworkArtifacts.value + try await RegressRowAsserts.assertRenders( "binary-frameworks/combined/Sources/CombinedBinaryFixture/BinaryFrameworkPreview.swift", extraArguments: ["--platform", "ios", "--device", sim.udid] ) @@ -178,8 +128,8 @@ struct RegressToolGuardTests { func b03StaticXCFramework() async throws { guard let sim = try await Self.provisionSimulator() else { return } defer { sim.lock.release() } - try await Self.generate("binary-frameworks/generate-artifacts.sh") - try await Self.assertRenders( + try await Self.binaryFrameworkArtifacts.value + try await RegressRowAsserts.assertRenders( "binary-frameworks/static-only/Sources/StaticBinaryFixture/StaticBinaryPreview.swift", extraArguments: ["--platform", "ios", "--device", sim.udid] ) @@ -192,8 +142,8 @@ struct RegressToolGuardTests { func f01BadSliceClassified() async throws { guard let sim = try await Self.provisionSimulator() else { return } defer { sim.lock.release() } - try await Self.generate("binary-frameworks/generate-artifacts.sh") - try await Self.assertFails( + try await Self.binaryFrameworkArtifacts.value + try await RegressRowAsserts.assertFails( "binary-frameworks/bad-slice/Sources/BadSliceFixture/BadSlicePreview.swift", extraArguments: ["--platform", "ios", "--device", sim.udid], containing: ["has no iOS simulator slice", "ios-arm64"] @@ -215,11 +165,11 @@ struct RegressToolGuardTests { guard let sim = try await Self.provisionSimulator() else { return } defer { sim.lock.release() } try await DaemonTestLock.run { - try await Self.cleanSlate() + try await RegressRowAsserts.cleanSlate() let runResult = try await CLIRunner.run( "run", arguments: [ - Self.fixture("lifecycle-faults/Sources/AgentCrash/AgentCrashPreview.swift"), + RegressRowAsserts.fixture("lifecycle-faults/Sources/AgentCrash/AgentCrashPreview.swift"), "--platform", "ios", "--device", sim.udid, "--detach", "--headless", ] ) @@ -288,7 +238,7 @@ struct RegressToolGuardTests { @Test("B01: bzlmod fixture renders via aquery capture", .timeLimit(.minutes(10))) func b01BazelBzlmod() async throws { guard try await Self.requireTool("bazel") else { return } - try await Self.assertRenders("bazel-bzlmod/Sources/BzlmodPreview.swift") { + try await RegressRowAsserts.assertRenders("bazel-bzlmod/Sources/BzlmodPreview.swift") { let logs = try await CLIRunner.run("logs", arguments: ["-n", "300"]) #expect(logs.exitCode == 0, "logs stderr: \(logs.stderr)") let confirmed = logs.stdout.split(separator: "\n").contains { diff --git a/previewsmcp/Tests/TestSupport/SimulatorTestDevices.swift b/previewsmcp/Tests/TestSupport/SimulatorTestDevices.swift index 0f7f7ee0..23c8ecab 100644 --- a/previewsmcp/Tests/TestSupport/SimulatorTestDevices.swift +++ b/previewsmcp/Tests/TestSupport/SimulatorTestDevices.swift @@ -1,6 +1,20 @@ import Foundation import PreviewsCore +/// The required-gate enforcement policy: when the gate's coverage signal +/// is set, a missing test precondition (a dedicated simulator, a required +/// tool) must FAIL the run, never skip — a skip on the gate is a silent +/// coverage loss. Unset locally, so a dev host without the precondition +/// skips instead of blocking. Backed by the same three coupled sites +/// documented on `SimulatorTestDevices.requiresDedicatedSim`, which +/// delegates here; non-simulator preconditions (e.g. +/// `RegressToolGuardTests.requireTool`) read this general name directly. +public enum RequiredGateEnforcement { + public static var enforced: Bool { + ProcessInfo.processInfo.environment["PREVIEWSMCP_REQUIRE_DEDICATED_SIM"] != nil + } +} + /// Dedicated, harness-owned simulators for the sim-booting test suites (#337). /// /// The retired `IOSSimulatorPicker` copies assigned each test the index-th @@ -37,6 +51,8 @@ import PreviewsCore /// - index 8: `IOSCLIWorkflowTests.iosCLIWorkflow` (CLIIntegrationTests target; /// pins the CLI run + variants sessions so the auto-select can't boot a /// generic default device that nulls the agent CGSession, #391) +/// - index 9: `RegressToolGuardTests.provisionSimulator()` (RegressToolGuardTests +/// target; one device shared by its `.serialized` iOS rows B02/B03/F01/L04) public enum SimulatorTestDevices { public static let deviceType = "com.apple.CoreSimulator.SimDeviceType.iPhone-17" @@ -63,7 +79,7 @@ public enum SimulatorTestDevices { /// 3. `.github/workflows/ci.yml` sets it =1 at job env + an "Assert /// iOS-coverage signal" step that fails the gate if it's ever dropped. public static var requiresDedicatedSim: Bool { - ProcessInfo.processInfo.environment["PREVIEWSMCP_REQUIRE_DEDICATED_SIM"] != nil + RequiredGateEnforcement.enforced } /// Apply the fail-vs-skip policy to a resolved device. This seam keeps the From ec0365163a35f25bd93f6704960880dc6f3aab84 Mon Sep 17 00:00:00 2001 From: Jason Prasad Date: Tue, 21 Jul 2026 12:20:24 -0400 Subject: [PATCH 3/3] =?UTF-8?q?tests+ci:=20review=20fixes=20=E2=80=94=20fl?= =?UTF-8?q?ock=20release=20on=20gate=20throw,=20job=20timeout=20margin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review fold: provisionSimulator releases the host-wide sim flock on the RequiredDeviceUnavailable throw path (a leaked LOCK_EX self-deadlocks the remaining iOS rows for 10 minutes each); regress-tools timeout-minutes rises to 90 so a hung test hits bazel's 3600s timeout (step failure -> diagnostics step) before job cancellation would skip it; doc notes for the artifact task's .serialized dependency and CoreSimulatorHygiene's actual lock contract. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 6 ++++- .../CoreSimulatorHygiene.swift | 9 +++---- .../RegressToolGuardTests.swift | 24 ++++++++++++++----- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8625421a..e6d8755a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -208,7 +208,11 @@ jobs: github.event.label.name == 'ci' || github.event.label.name == 'merge' runs-on: [self-hosted, macOS, ARM64] - timeout-minutes: 60 + # Above the target's `eternal` (3600s) bazel test timeout, like `ci`'s + # 120 vs its heaviest target: a hung test must hit bazel's timeout (a + # step failure(), which runs the diagnostics step) before the job-level + # cancellation, which would skip it. + timeout-minutes: 90 permissions: contents: read checks: read diff --git a/previewsmcp/Tests/CLIIntegrationTests/CoreSimulatorHygiene.swift b/previewsmcp/Tests/CLIIntegrationTests/CoreSimulatorHygiene.swift index 5b46e566..552b027b 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/CoreSimulatorHygiene.swift +++ b/previewsmcp/Tests/CLIIntegrationTests/CoreSimulatorHygiene.swift @@ -16,10 +16,11 @@ import os /// shut every simulator down and bounce CoreSimulatorService so the next boot /// starts from a clean service. Call while holding `SimulatorTestLock` (which /// serializes sim-booting tests host-wide, so this reset cannot kill another -/// workspace's live run — #336) and `DaemonTestLock`, which serializes the -/// sim-touching suites WITHIN this target (its flock path is per-target); -/// across targets within one invocation the guard is this target's -/// `exclusive` tag. +/// workspace's live run — #336) and with no live daemon session in this +/// process — either inside `DaemonTestLock.run` (whose teardown fences the +/// daemon dead before releasing) or between run blocks, where that fence +/// has already run; across targets within one invocation the guard is this +/// target's `exclusive` tag. enum CoreSimulatorHygiene { private static let didReset = OSAllocatedUnfairLock(initialState: false) diff --git a/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift b/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift index 7066683f..a561a97d 100644 --- a/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift +++ b/previewsmcp/Tests/CLIIntegrationTests/RegressToolGuardTests.swift @@ -43,7 +43,9 @@ struct RegressToolGuardTests { /// every artifact from scratch, so a second run in the same process /// buys nothing but ~15s of xcodebuild on the serial runner. The Task /// runs `generate` lazily on first await and caches its outcome - /// (including a failure) for the rest. + /// (including a failure) for the rest. Depends on `.serialized`: the + /// creating row awaits it to completion, so the `#require` inside + /// records against the right test and first access never races. private static let binaryFrameworkArtifacts = Task { try await generate("binary-frameworks/generate-artifacts.sh") } @@ -96,13 +98,23 @@ struct RegressToolGuardTests { -> (udid: String, lock: SimulatorTestLock.Guard)? { let simLock = try await SimulatorTestLock.acquire() - guard let udid = try await SimulatorTestDevices.udid(index: 9) else { - print("Host cannot create \(SimulatorTestDevices.name(index: 9)) — skipping") + do { + guard let udid = try await SimulatorTestDevices.udid(index: 9) else { + print("Host cannot create \(SimulatorTestDevices.name(index: 9)) — skipping") + simLock.release() + return nil + } + await CoreSimulatorHygiene.resetOnce() + return (udid, simLock) + } catch { + // The gate's RequiredDeviceUnavailable throw must not leak the + // held flock: a leaked LOCK_EX self-deadlocks every later iOS + // row in this process (flock re-acquisition blocks even + // within one process), turning one clean failure into three + // ten-minute hangs. simLock.release() - return nil + throw error } - await CoreSimulatorHygiene.resetOnce() - return (udid, simLock) } // MARK: - Binary-framework rows (iOS)