Skip to content

Commit df0a0f7

Browse files
authored
perf(package): strip comments from the Apple runner source the npm package ships (#2467)
* perf(package): strip comments from the Apple runner source the npm package ships The packager copies apple/runner/** into dist/ as Swift source, removing only its AGENT_DEVICE_RUNNER_UNIT_TESTS blocks, so doc comments and design notes were downloaded on every install: 71.9 kB of 441.2 kB of packaged runner Swift. Add a lexical scanner for the removal. A regex cannot do this: `//` and `/*` open a comment only in code position, raw literals move their own delimiter and escape with the `#` count, interpolation segments hold code and further literals, and Swift block comments nest. A construct the scanner cannot account for throws at packaging time instead of shipping Swift that does not compile. * fix(package): keep Swift regex literals out of the comment scanner `#/foo//bar/#` is a valid extended regex literal with no comment in it, but the scanner only knew the `#"` raw-string family, so it read the literal's `//` as a line comment and shipped `let pattern = #/foo` — Swift that does not compile. Add `#/…/#` and `##/…/##` as a literal context: matching `#` counts, the single- and multi-line forms, Swift's own-line rule for a multi-line closing delimiter, and the `\/` escape that keeps one from closing early. Bare `/…/` literals stay unresolvable, because the same `/` opens a comment, divides, and starts a regex literal, and only the parse separates them. Where one could begin — an expression position whose `/` is not followed by a space, a tab or `)` — packaging throws by file and line instead of rewriting bytes it cannot prove are code. Divisions (`width/2`, `Double(3)/Double(4)`), the recording scripts' shebang and `(/)` keep flowing through. * fix(package): keep the packaged runner source on the checkout's line numbers `dist/apple/runner/**` is the Swift a user's `xcodebuild` and the runner name a file and line in (it lands in runner.log), so those numbers are only worth reading if they point at the same line of `apple/runner/**`. Both rewriting passes now empty the lines they remove instead of deleting them: comment removal (889 lines, 889 B) and the pre-existing unit-test `#if` block strip, which was moving everything below a block by up to 883 lines (3,737 lines, 3,737 B). `dist/apple/runner/` 555,907 B -> 488,635 B (-67,272 B, -12.1%); its Swift alone 441,196 B -> 373,924 B (-15.2%). Parity costs 4,626 B of the 71,898 B the previous head saved. Nothing in the repo compiles the packaged source, so a mis-lex that failed to throw would ship Swift that does not build and no gate would see it. Add `pnpm check:packaged-runner-swift`: it packages into a throwaway root and asserts line-count parity plus the line of every declaration each packaged file still carries, then runs `swiftc -parse` over all 44 files. The parse half reports itself skipped where no Swift toolchain exists, so the gate is declared on the macOS lane, where both halves run.
1 parent 3d503a0 commit df0a0f7

12 files changed

Lines changed: 1229 additions & 4 deletions

.github/workflows/macos.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ jobs:
7070
uses: ./.github/actions/run-gate
7171
with: { gate: macos-coverage }
7272

73+
# The npm package ships apple/runner/** as rewritten Swift source, and nothing in the repo
74+
# compiles the result — the first consumer is a user's xcodebuild. This lane is the only one
75+
# with a Swift toolchain, so it is where `swiftc -parse` over the packaged files runs; the
76+
# line-parity half of the same gate needs no toolchain. Seconds, no simulator (#2461).
77+
- name: Check the packaged runner Swift parses and keeps its line numbering
78+
uses: ./.github/actions/run-gate
79+
with: { gate: packaged-runner-swift }
80+
7381
- name: Run iOS snapshot Swift/TypeScript differential
7482
uses: ./.github/actions/run-gate
7583
with: { gate: ios-snapshot-differential }

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,15 @@
156156
"check:daemon-wire-compat": "node --experimental-strip-types scripts/wire-compat/run.ts",
157157
"check:daemon-wire-compat:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/wire-compat/model.test.ts",
158158
"check:xctest-selection": "node --experimental-strip-types scripts/check-xctest-selection.ts",
159+
"check:packaged-runner-swift": "node --experimental-strip-types scripts/check-packaged-runner-swift.ts",
159160
"check:tmpdir-leaks": "node --experimental-strip-types scripts/check-tmpdir-leaks.ts",
160161
"check:tmpdir-leaks:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-tmpdir-leaks-model.test.ts scripts/vitest-tmpdir-global-setup.test.ts scripts/node-test-tmpdir.test.ts scripts/swift-toolchain-tmpdir.test.ts",
161162
"check:freerange": "fr",
162163
"check:quick": "pnpm lint && pnpm typecheck",
163164
"sync:mcp-metadata": "node scripts/sync-mcp-metadata.mjs",
164165
"check:mcp-metadata": "node scripts/sync-mcp-metadata.mjs --check",
165166
"version": "pnpm sync:mcp-metadata && git add server.json",
166-
"check:tooling": "pnpm format:check && pnpm lint && pnpm typecheck && pnpm check:layering && pnpm depgraph:test && pnpm check:gate-manifest:test && pnpm check:gate-manifest && pnpm check:production-exports && pnpm check:tmpdir-leaks:test && pnpm check:xctest-selection && pnpm check:mcp-metadata && pnpm build && pnpm check:bundle-owner-files && pnpm check:package",
167+
"check:tooling": "pnpm format:check && pnpm lint && pnpm typecheck && pnpm check:layering && pnpm depgraph:test && pnpm check:gate-manifest:test && pnpm check:gate-manifest && pnpm check:production-exports && pnpm check:tmpdir-leaks:test && pnpm check:xctest-selection && pnpm check:packaged-runner-swift && pnpm check:mcp-metadata && pnpm build && pnpm check:bundle-owner-files && pnpm check:package",
167168
"check:unit": "pnpm test:unit && pnpm check:tmpdir-leaks && pnpm test:smoke",
168169
"check": "pnpm check:tooling && pnpm check:fallow && pnpm check:unit",
169170
"prepack": "pnpm check:mcp-metadata && pnpm package:npm",
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { declarationsByLine, parityFailures } from '../check-packaged-runner-swift.ts';
4+
5+
function swift(...lines: string[]): string {
6+
return `${lines.join('\n')}\n`;
7+
}
8+
9+
const source = swift(
10+
'// a design note',
11+
'extension RunnerTests {',
12+
' /// What this does.',
13+
' func helper() -> Int { 1 }',
14+
'}',
15+
);
16+
17+
test('a packaged file that emptied its comment lines reports no failure', () => {
18+
const packaged = swift('', 'extension RunnerTests {', '', ' func helper() -> Int { 1 }', '}');
19+
20+
assert.deepEqual(parityFailures('Fixture.swift', source, packaged), []);
21+
});
22+
23+
test('a deleted comment line is named as a line-count failure', () => {
24+
const packaged = swift('extension RunnerTests {', ' func helper() -> Int { 1 }', '}');
25+
26+
const failures = parityFailures('Fixture.swift', source, packaged);
27+
assert.equal(failures.length, 1);
28+
assert.match(failures[0] ?? '', /Fixture\.swift: packaged source has 4 lines, checkout has 6\./);
29+
assert.match(failures[0] ?? '', /empty a removed line, not delete it/);
30+
});
31+
32+
// The line count alone cannot see a rewrite that moves a declaration and pads elsewhere, which is
33+
// the failure an `xcodebuild` line number would land on.
34+
test('a declaration that moved while the line count held is named with its line', () => {
35+
const packaged = swift('', 'extension RunnerTests {', ' func helper() -> Int { 1 }', '', '}');
36+
37+
const failures = parityFailures('Fixture.swift', source, packaged);
38+
assert.equal(failures.length, 1);
39+
assert.match(failures[0] ?? '', /Fixture\.swift:3: packaged `func helper`/);
40+
assert.match(failures[0] ?? '', /is `\(blank\)` at the same line of the checkout/);
41+
});
42+
43+
test('declarations are keyed by their one-based line', () => {
44+
assert.deepEqual(
45+
[
46+
...declarationsByLine(
47+
swift('import XCTest', 'final class RunnerTests: XCTestCase {', ' func testCommand() {}'),
48+
),
49+
],
50+
[
51+
[2, 'class RunnerTests'],
52+
[3, 'func testCommand'],
53+
],
54+
);
55+
});

0 commit comments

Comments
 (0)