test(ios-runner): pin production-built runner requests in a golden table TS and Swift both verify - #2900
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at e1f72f2. The golden table and the matching TS and Swift checks look right, and they follow the existing alert-command-traits pattern. One question: Small notes, not blocking: the aborted-start assertion uses a bare All checks pass, and there are no conflicts. |
thymikee
left a comment
thymikee
left a comment
There was a problem hiding this comment.
Reviewed at e1f72f2. The golden table itself is the right direction. The remaining structural issue is that the guard and the root helper duplicate the fixture contract by hand, so they can drift from the table they are supposed to verify.
| ); | ||
|
|
||
| test('runner request drives send only requests production builds', () => { | ||
| const sources = [ |
There was a problem hiding this comment.
This anti-literal guard scans a hand-maintained source list, but the fixture already carries the real producer paths in entry.producer. It omits the root screen-recording producers, so a raw request literal or direct send in those files would not be checked. Derive the scanned sources from the fixture producers, or otherwise close the list against the producer set the fixture itself declares.
| fs.readFileSync(path.join(REPO_ROOT, 'contracts/fixtures/runner-requests.json'), 'utf8'), | ||
| ) as RunnerRequestEntry[]; | ||
| const produced = captured | ||
| .map(([name, sent]) => ({ name, producer, request: JSON.parse(JSON.stringify(sent)) })) |
There was a problem hiding this comment.
This root helper copy has already diverged from the package helper: runner-requests-fixture.ts normalizes commandId and statusCommandId through wireRunnerRequest, while this compares the raw sent request. The current root entries happen to omit command IDs, but once a root producer carries one the comparison is against a different shape than the fixture. Use the shared normalization path if the package boundary allows, or move the normalization into a shared fixture utility.
…table Add contracts/fixtures/runner-requests.json: one entry per production request site, captured at its real entry point. Three producer tests own the entries: the platform-apple request sites, the runner-internal sites (real HTTP bodies from the fake runner), and the two recording modules. runner-contract.test.ts asserts every RunnerCommand has an entry, names are sorted and unique, and the drive code builds no request itself. Refs #2881
Every fixture request decodes as Command and re-encodes with the same key set, every CommandType case has a request, and every stored field of Command, SequenceStep and RunnerGesturePlan appears in some request. CommandType becomes CaseIterable for the case check. Refs #2881
… tests Export the fixture reader as @agent-device/platform-apple/runner/requests-fixtures so root producers pin requests through the same commandId normalization, and derive the request-literal guard's sources from the producers the fixture declares. Refs #2881
…lden file The request-literal guard scans every fixture producer, so root behavior tests that doubled as producers had to weaken their exact request assertions. Move the root recording drives into src/__tests__/screen-recording-runner-requests.test.ts, restore the original literals, and pin the recordStop sent without an app bundle. Producers must now be *runner-requests.test.ts files that call assertProducedRunnerRequests(import.meta.filename, ...), so a comment no longer satisfies the check. The runner-internal drive moves to runner/__tests__/runner-requests.test.ts to follow that rule. Refs #2881
e1f72f2 to
08f33d6
Compare
|
Addressed the review at e1f72f2. New head 08f33d6. Reuse point (root helper duplicating fixture read/normalize): Inline: guard scans a hand-kept source list: the request-literal guard now derives its scanned sources from Inline: root helper skipped commandId normalization: closed by the same fixture-module move above — there's no longer a second normalization path to drift. Follow-up finding while widening the guard: deriving sources from producers pulled the two root behavior tests (which doubled as producers) into the guard, which had weakened three request assertions to Rebase: onto Declined: fencing the new Gate on 08f33d6: |
|
The code at 08f33d6 looks good. The evidence gap from the earlier pass (e1f72f2) is closed: the golden table is now pinned and verified on both the TS and Swift sides. CI failure is unrelated to this diff. The PR's logical change does not touch Re-run Smoke Tests to clear the cold xcrun timeout; no code change is needed here. I did not re-run vitest, check:affected, or the Swift XCTest suite locally, so the author's gate report is unverified, and I can't confirm the ETIMEDOUT is transient without seeing a re-run. |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Adds
contracts/fixtures/runner-requests.json, a golden table of production-built Apple runner requests (67 entries, 34 commands). TS producer tests drive every real call site (platform-apple entry points, both screen-recording modules) and assert an exact match; Swift tests decode every entry and require everyCommandTypecase and everyCommand/SequenceStep/RunnerGesturePlanfield to have a producer. Step 1 of #2881: proves a production site can build each request, not that the daemon sends it. Step 2 (Swift unit tests onto the table) is a separate follow-up PR.Closes #2881 (step 1)
13 files touched. Net production lines: 0 TS, 0 Swift (one
CaseIterableconformance on an existing line). Gross diff 894 lines (issue estimate 800-950; under the 1,000-line budget).Validation
Tested at
08f33d6036.pnpm check:affected --run: green (59 selected checks — format, lint, typecheck, layering, fallow, build, vitest, xctest-selection, packaged-runner-swift, and the rest). Atmpdir-leaks-modelfailure seen on one run traced to contention from other gates sharing the host (passes 7/7 alone); reran clean with the host idle.pnpm check:xctest-selection: 322 declared methods, 0 unreachable by any lane.runner/__tests__/runner-requests,__tests__/runner-requests,interactor-runner-provider, the newsrc/__tests__/screen-recording-runner-requests, and both root screen-recording tests — 6 files / 27 tests pass.TEST BUILD SUCCEEDED(build:xcuitest:ios, unit tests included); 5/5 XCTest pass on a throwaway iOS 26.2 simulator (3 golden-table tests + 2ErrorPayloadwire tests).Remaining risk: macOS host lane and iOS 18.x/tvOS runtimes for the new Swift tests were not run locally; CI-verifiable, not blockers.