Skip to content

Commit 115021a

Browse files
thymikeethymikee
committed
test(ci): order the macOS proof by command, and move swiftc out of test 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.
1 parent 037a424 commit 115021a

2 files changed

Lines changed: 80 additions & 68 deletions

File tree

‎packages/capture-kit/src/recording/__tests__/recording-scripts.test.ts‎

Lines changed: 58 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { beforeAll, test } from 'vitest';
1+
import { beforeAll, describe, test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import path from 'node:path';
44
import { fileURLToPath } from 'node:url';
@@ -15,78 +15,71 @@ const recordingScriptsDir = path.resolve(
1515
);
1616
const recordingTestSupportDir = path.resolve(__dirname, '../../../../../test/integration/support');
1717
const SWIFT_TYPECHECK_TIMEOUT_MS = 60_000;
18-
let swiftCompilerPath = 'swiftc';
19-
let swiftSdkPath = '';
2018

21-
async function assertSwiftScriptTypechecks(
22-
scriptPath: string,
23-
extraSourcePaths: string[] = [],
24-
): Promise<void> {
25-
const result = await runCmd(
26-
swiftCompilerPath,
27-
['-sdk', swiftSdkPath, '-typecheck', scriptPath, ...extraSourcePaths],
28-
{
29-
allowFailure: true,
30-
},
31-
);
32-
assert.equal(
33-
result.exitCode,
34-
0,
35-
`${path.basename(scriptPath)} should typecheck\n${result.stderr}`,
36-
);
37-
}
19+
type TypecheckOutcome = { exitCode: number; stderr: string; source: string };
3820

39-
beforeAll(async () => {
40-
if (process.platform !== 'darwin') return;
41-
const [compilerResult, sdkResult] = await Promise.all([
42-
runCmd('xcrun', ['--find', 'swiftc']),
43-
runCmd('xcrun', ['--show-sdk-path', '--sdk', 'macosx']),
44-
]);
45-
swiftCompilerPath = compilerResult.stdout.trim();
46-
swiftSdkPath = sdkResult.stdout.trim();
47-
});
21+
// The three `swiftc -typecheck` invocations run in beforeAll, not in the test bodies: each is a
22+
// real compiler launch, and the unit slow-test gate budgets `packages/**` test cases far below one
23+
// compile (see the budget note in docs/agents/testing.md). The snapshot-bridge and fold-helper
24+
// gates in platform-apple keep their native compiles out of test-case wall time the same way.
25+
// One beforeAll covers all three so the SDK probe and the two shared-source compiles are paid once
26+
// per file rather than once per case.
27+
describe.skipIf(process.platform !== 'darwin')('recording Swift scripts typecheck', () => {
28+
let outcomes: TypecheckOutcome[] = [];
4829

49-
test(
50-
'recording inspect Swift script typechecks',
51-
async (t) => {
52-
if (process.platform !== 'darwin') {
53-
t.skip('Swift recording scripts are only validated on macOS');
54-
}
30+
beforeAll(
31+
async () => {
32+
const [compiler, sdk] = await Promise.all([
33+
runCmd('xcrun', ['--find', 'swiftc']),
34+
runCmd('xcrun', ['--show-sdk-path', '--sdk', 'macosx']),
35+
]);
36+
const swiftCompilerPath = compiler.stdout.trim() || 'swiftc';
37+
const swiftSdkPath = sdk.stdout.trim();
38+
const sharedSupport = path.join(recordingScriptsDir, 'RecordingExportSupport.swift');
39+
const targets: Array<{ source: string; extraSources: string[] }> = [
40+
{ source: path.join(recordingTestSupportDir, 'recording-inspect.swift'), extraSources: [] },
41+
{
42+
source: path.join(recordingScriptsDir, 'recording-overlay.swift'),
43+
extraSources: [sharedSupport],
44+
},
45+
{
46+
source: path.join(recordingScriptsDir, 'recording-frames.swift'),
47+
extraSources: [sharedSupport],
48+
},
49+
];
5550

56-
await assertSwiftScriptTypechecks(
57-
path.join(recordingTestSupportDir, 'recording-inspect.swift'),
58-
);
59-
},
60-
SWIFT_TYPECHECK_TIMEOUT_MS,
61-
);
51+
outcomes = await Promise.all(
52+
targets.map(async ({ source, extraSources }): Promise<TypecheckOutcome> => {
53+
const result = await runCmd(
54+
swiftCompilerPath,
55+
['-sdk', swiftSdkPath, '-typecheck', source, ...extraSources],
56+
{ allowFailure: true, timeoutMs: SWIFT_TYPECHECK_TIMEOUT_MS },
57+
);
58+
return { source, exitCode: result.exitCode, stderr: result.stderr };
59+
}),
60+
);
61+
},
62+
SWIFT_TYPECHECK_TIMEOUT_MS * 3 + 30_000,
63+
);
6264

63-
test(
64-
'recording overlay Swift script typechecks',
65-
async (t) => {
66-
if (process.platform !== 'darwin') {
67-
t.skip('Swift recording scripts are only validated on macOS');
68-
}
65+
test('recording inspect Swift script typechecks', () => {
66+
assertTypechecked(outcomes, 'recording-inspect.swift');
67+
});
6968

70-
await assertSwiftScriptTypechecks(path.join(recordingScriptsDir, 'recording-overlay.swift'), [
71-
path.join(recordingScriptsDir, 'RecordingExportSupport.swift'),
72-
]);
73-
},
74-
SWIFT_TYPECHECK_TIMEOUT_MS,
75-
);
69+
test('recording overlay Swift script typechecks', () => {
70+
assertTypechecked(outcomes, 'recording-overlay.swift');
71+
});
7672

77-
test(
78-
'recording frames Swift script typechecks',
79-
async (t) => {
80-
if (process.platform !== 'darwin') {
81-
t.skip('Swift recording scripts are only validated on macOS');
82-
}
73+
test('recording frames Swift script typechecks', () => {
74+
assertTypechecked(outcomes, 'recording-frames.swift');
75+
});
76+
});
8377

84-
await assertSwiftScriptTypechecks(path.join(recordingScriptsDir, 'recording-frames.swift'), [
85-
path.join(recordingScriptsDir, 'RecordingExportSupport.swift'),
86-
]);
87-
},
88-
SWIFT_TYPECHECK_TIMEOUT_MS,
89-
);
78+
function assertTypechecked(outcomes: readonly TypecheckOutcome[], basename: string): void {
79+
const outcome = outcomes.find((entry) => path.basename(entry.source) === basename);
80+
assert.ok(outcome, `${basename} was never typechecked`);
81+
assert.equal(outcome.exitCode, 0, `${basename} should typecheck\n${outcome.stderr}`);
82+
}
9083

9184
test('recording overlays are explicitly unsupported on non-macOS hosts', () => {
9285
assert.equal(

‎scripts/__tests__/apple-ci-impact.test.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ function cacheInputs(action: string): string[] {
1919
);
2020
}
2121

22+
type WorkflowStep = { run?: string; with?: { gate?: string } };
23+
24+
/**
25+
* Index of the first workflow step matching a predicate, counted across every job in file order,
26+
* or -1. Steps are located by the command they run or the gate they invoke rather than by their
27+
* title, and through the same YAML parser the assertions below already use, so a step renamed for
28+
* scope reasons — or a `run:` block reindented — is not mistaken for a sequencing regression.
29+
*/
30+
function workflowStepIndex(workflow: string, matches: (step: WorkflowStep) => boolean): number {
31+
const doc = parse(workflow) as { jobs?: Record<string, { steps?: WorkflowStep[] }> };
32+
return Object.values(doc.jobs ?? {})
33+
.flatMap((job) => job.steps ?? [])
34+
.findIndex(matches);
35+
}
36+
2237
test('every runner build-cache input triggers the PR XCTest lane', () => {
2338
const action = fs.readFileSync(
2439
path.join(repoRoot, '.github/actions/setup-apple-runner-build/action.yml'),
@@ -58,10 +73,14 @@ test('the PR workflow applies the impact decision to the XCTest step', () => {
5873
});
5974

6075
test('macOS clean-install proof follows live UI replay', () => {
76+
// Ordered by what each step runs, not by its title: the clean-install proof must not fire
77+
// before the replay that can raise local-network permission UI, and a step renamed for scope
78+
// reasons is not a sequencing regression.
6179
const workflow = fs.readFileSync(path.join(repoRoot, '.github/workflows/macos.yml'), 'utf8');
62-
const replay = workflow.indexOf('- name: Run macOS integration test');
63-
const proof = workflow.indexOf(
64-
'- name: Verify clean-installed Simulator snapshot bridge preparation',
80+
const replay = workflowStepIndex(workflow, (step) => step.with?.gate === 'replay-macos');
81+
const proof = workflowStepIndex(
82+
workflow,
83+
(step) => step.run?.includes('--verify-snapshot-bridge-preparation') ?? false,
6584
);
6685
expect(replay).toBeGreaterThan(-1);
6786
expect(proof).toBeGreaterThan(replay);

0 commit comments

Comments
 (0)