ci(macos): run the darwin-only Vitest files on a lane that reaches them - #2944
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 325dd4d. This isn't green yet, and both failures trace to this diff. scripts/tests/apple-ci-impact.test.ts:63 pins the literal step-name substring
Coverage and one Smoke Tests job fail on the exact Once the step-name pin is fixed and the swiftc calls move to |
`.github/workflows/macos.yml:168` is the only Vitest invocation in the repository that
runs on macOS. It was a hand-written two-file list, so a test guarded on
`process.platform === 'darwin'` had no lane that could execute it: the sole job that
collects `packages/*/src/**/*.test.ts` is the `unit-ci` gate on `ubuntu-latest`, where
the whole file skips.
Two files were dying that way, six tests between them:
- `packages/platform-apple/src/foldable/simulator-hid-native.test.ts` — the only consumer
of the `samples` column of `contracts/fixtures/fold-keyframes.json`. The ObjC
interpolation in `apple/fold-helper/Fold.m` is the thing those samples check, and the
TypeScript twin (`device-rotation-fold-input.test.ts`) reads only `valid` and
`keyframesJson`. Proven: nudging one `angle` from 160 to 163 reddens this file
("opening reversal at 1667ms") while the TS legs that Linux CI actually runs report
14 passed. The column was unguarded on every lane.
- `packages/capture-kit/src/recording/__tests__/recording-scripts.test.ts` — `swiftc
-typecheck` over `apple/runner/AgentDeviceRunner/RecordingScripts`.
Both join the existing conditional step rather than a new unconditional one, because
`selectAppleBridgeProof` (`scripts/apple-ci-impact.ts:43`) already fires on any change
under `apple/` or `packages/platform-apple/src/` — which is exactly the input set of both
tests — so they run when their subject moved and cost nothing when it did not. The step is
renamed to say what it now covers. All four files pass together here: 58 tests.
One gap left on purpose: editing the recording-scripts test file alone does not trigger the
step, since `packages/capture-kit/` is outside the classifier's prefixes. Widening
`selectAppleBridgeProof` is affected-check model territory with its own tests, not a
workflow edit; noted rather than folded in.
…st wall time Both failures on this PR traced to my own diff. `scripts/__tests__/apple-ci-impact.test.ts` pinned the literal step title `- name: Verify clean-installed Simulator snapshot bridge preparation`, which I renamed, so `indexOf` returned -1 and the Coverage job died on "expected -1 to be greater than 6134". Patching the string would just re-pin the same prose, so the assertion moved to what the steps actually run: the replay step is found by `with.gate === 'replay-macos'`, the clean-install proof by the `--verify-snapshot-bridge-preparation` flag it passes (unique in the file, and named by `scripts/check-package.ts:42`). The lookup walks jobs in file order through the `yaml` parser this test already imports, so a reindented `run:` block or a multi-line `with:` no longer reads as a sequencing change. Proven non-vacuous: retagging the replay step so the proof no longer follows it fails with "expected -1 to be greater than -1". `recording-scripts.test.ts` called `assertSwiftScriptTypechecks` from inside `test()`, so the 7.97s `swiftc` launch landed on the per-case clock, blew through 2x the `packages/**` budget, and aborted the step before `pnpm check:package` ran. The three compiles now run in one `beforeAll`, following the note at `fold-helper-cache.test.ts:164` — one SDK probe, both shared-source compiles paid once instead of per case. The cases drop to 1ms/0ms/0ms while the `describe.skipIf` keeps them darwin-only, which also removes the per-test `t.skip` branches. Detection is unchanged and was checked three ways against real sources: a type error in `recording-inspect.swift` fails the inspect case alone; one in `recording-overlay.swift` fails the overlay case alone; one in the shared `RecordingExportSupport.swift` fails exactly the two cases that compile it. All fixtures reverted.
325dd4d to
115021a
Compare
|
Both findings were correct and both traced to my diff. Fixed in 1. The step-name pin. Rather than re-pin the renamed prose, the assertion now locates both steps 2. swiftc on the per-case clock. The three macOS lane observed green, job 107963122067 No slow-test gate output, and the step continued into the part that previously never got to run: 58 passed is the same count I measured locally across those four files, so nothing was silently On the |
|
Reviewed at 115021a. This is ready for human review. Both issues from the earlier pass (325dd4d, #2944 (comment)) are fixed at the root: the darwin-only Vitest files now run on a lane that reaches them, and the mechanism holds up under static review — Not blocking: could the recording-scripts typecheck proof's inputs ( |
thymikee
added
the
ready-for-human
|
1 participant
Summary
.github/workflows/macos.yml:168is the only Vitest invocation in the repo that runs on macOS,and it was a hand-written two-file list. Meanwhile the only job that collects
packages/*/src/**/*.test.tsis theunit-cigate onubuntu-latest(ci.yml:286). Anythingguarded on
process.platform === 'darwin'therefore had no lane that could execute it — the file wascollected and skipped, forever. Six tests across two files were dying this way.
packages/platform-apple/src/foldable/simulator-hid-native.test.ts(whole file,describe.skipIf(!darwin))samplescolumn ofcontracts/fixtures/fold-keyframes.json, checked against the ObjC interpolation inapple/fold-helper/Fold.m. Its TypeScript twindevice-rotation-fold-input.test.tsreads onlyvalidandkeyframesJsonpackages/capture-kit/src/recording/__tests__/recording-scripts.test.ts(t.skip×3)swiftc -typecheckoverapple/runner/AgentDeviceRunner/RecordingScriptsThe
samplescolumn was genuinely unguarded on every lane — measured. Nudging onesamples[2].anglefrom160to163:simulator-hid-native.test.ts→1 failed,AssertionError: opening reversal at 1667msdevice-rotation-fold-input.test.ts(what Linux CI actually runs) →14 passedSo a corrupted golden sample could merge green today and the native keyframe rule would never notice.
Fixture reverted before committing.
Why these join the existing step instead of a new one
I expected to add an unconditional step. I didn't, because
selectAppleBridgeProof(
scripts/apple-ci-impact.ts:43) already fires on any change underapple/orpackages/platform-apple/src/— which is precisely the input set of both tests (the.m, the Swiftscripts). They run when their subject moved and cost nothing when it did not, with no new macOS
minutes on unrelated PRs. The step is renamed to say what it now covers.
Validation
Tested at
325dd4d29.git statusshowed onlymacos.yml.github/workflows/macos.ymlparsed with a YAML parser to confirm the step, itsif:, and all fourcontinuation lines survived the edit
pnpm check:affected --run→ exit 0;oxlint . --deny-warnings→ exit 0One gap left open deliberately: editing the recording-scripts test file alone does not trigger
the step, because
packages/capture-kit/is outside the classifier's two prefixes. WideningselectAppleBridgeProofis affected-check-model territory with its own tests — a workflow change isthe wrong place to do it, so it is flagged here instead of folded in.