Skip to content

Commit 3417a77

Browse files
authored
test(android,linux,apple): flake, dead-assertion, and fixture fixes from CRAP reviews (#3462)
* test(android,linux,apple): remove flake, dead assertions, and host-binary fixtures from CRAP follow-up tests - emulator-lifecycle: run the boot-timeout test on fake timers so the 80ms poll / 150ms deadline geometry cannot drift with host scheduling, and drop the notEqual(BOOT_COMMAND_FAILED) assertion that the reason equality already subsumes. - linux snapshot: give the degenerate-rect tie a negative control one pixel out, and pin the missing-depth default in both sort directions so only exactly 0 keeps the observed winner in both fixtures. - physical-device-files: pin details.step on the spent-deadline failure. - install-artifact: build the multi-payload IPA through host-kit's writeZipFixture instead of shelling out to the host zip binary. - host-provider: answer the mocked hostFileStat with real fs.Stats of backing tmp paths, make the user Applications root a real directory, and declare missing roots explicitly instead of an implicit ENOENT. - runner-xctestrun-products: seed a bundle only the surviving record target names so a target only needing to be a record is observable. * chore(gates): export host-kit's archive fixtures for cross-package tests Move the archive fixture module from src/internal/ to src/archive.fixtures.ts and export it as ./archive-fixtures (audio-probe-fixtures precedent) so platform-apple's IPA fixture can write real stored-zip archives through host-kit's writeZipFixture instead of the host zip binary. Replace the host-zip encrypted-entry helper with a fixture-written encrypted flag, and record the subpath in the R11 export-surface list.
1 parent e68f7f3 commit 3417a77

12 files changed

Lines changed: 185 additions & 94 deletions

File tree

‎packages/host-kit/package.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
"types": "./src/archive.ts",
2121
"default": "./src/archive.ts"
2222
},
23+
"./archive-fixtures": {
24+
"types": "./src/archive.fixtures.ts",
25+
"default": "./src/archive.fixtures.ts"
26+
},
2327
"./audio-probe-fixtures": {
2428
"types": "./src/audio-probe.fixtures.ts",
2529
"default": "./src/audio-probe.fixtures.ts"

packages/host-kit/src/internal/archive-extraction.fixtures.ts renamed to packages/host-kit/src/archive.fixtures.ts

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ import { promises as fs } from 'node:fs';
33
import path from 'node:path';
44
import { crc32, gzipSync } from 'node:zlib';
55
import * as tar from 'tar-stream';
6-
import { runCmdSync } from './exec.ts';
7-
import { mkdtempForTest } from './tmp-dir.fixtures.ts';
6+
import { mkdtempForTest } from './internal/tmp-dir.fixtures.ts';
87

98
export async function createArchiveWorkspace(): Promise<{
109
archivePath: string;
@@ -27,21 +26,30 @@ export async function createTruncatedTgz(archivePath: string): Promise<void> {
2726
await fs.writeFile(archivePath, gzipSync(headerOnly));
2827
}
2928

29+
/** Two stored entries whose second carries the encrypted general-purpose flag bit. */
3030
export async function createZipWithEncryptedSecondEntry(archivePath: string): Promise<void> {
31-
const staging = path.join(path.dirname(archivePath), 'zip-input');
32-
await fs.mkdir(staging);
33-
await fs.writeFile(path.join(staging, 'first.txt'), 'first');
34-
await fs.writeFile(path.join(staging, 'second.txt'), 'second');
35-
runCmdSync('zip', ['-q', archivePath, 'first.txt'], { cwd: staging });
36-
runCmdSync('zip', ['-q', '-P', 'secret', archivePath, 'second.txt'], { cwd: staging });
31+
await writeZipFixture(archivePath, [
32+
{ name: 'first.txt', mode: ZIP_FILE_MODE, data: 'first' },
33+
{ name: 'second.txt', mode: ZIP_FILE_MODE, data: 'second', encrypted: true },
34+
]);
3735
}
3836

37+
const ZIP_FILE_MODE = 0o100644;
38+
3939
/** One stored zip entry; `mode` is the full Unix `st_mode`, file-type bits included. */
40-
export type ZipFixtureEntry = { name: string; mode: number; data?: string };
40+
export type ZipFixtureEntry = {
41+
name: string;
42+
mode: number;
43+
data?: string;
44+
/** Declares the entry encrypted, which no archive reader may then extract. */
45+
encrypted?: boolean;
46+
};
4147

4248
/**
4349
* Writes a stored (uncompressed) zip whose names and Unix attributes are exactly as given, which
44-
* `zip` itself refuses to produce for links, special files, and escaping names.
50+
* `zip` itself refuses to produce for links, special files, and escaping names. An entry declared
51+
* `encrypted` carries the traditional encryption flag with a padded 12-byte encryption header and
52+
* a zero CRC, the shape readers must refuse before extracting anything.
4553
*/
4654
export async function writeZipFixture(
4755
archivePath: string,
@@ -52,24 +60,29 @@ export async function writeZipFixture(
5260
let offset = 0;
5361
for (const entry of entries) {
5462
const name = Buffer.from(entry.name, 'utf8');
55-
const data = Buffer.from(entry.data ?? '', 'utf8');
56-
const checksum = crc32(data);
63+
const payload = Buffer.from(entry.data ?? '', 'utf8');
64+
// A traditionally encrypted stored entry carries a 12-byte encryption header before the
65+
// ciphertext, and readers validate that against the declared sizes, so the record declares
66+
// it and the data section pads it; the payload itself never decrypts to anything.
67+
const data = entry.encrypted ? Buffer.concat([Buffer.alloc(12), payload]) : payload;
68+
const checksum = entry.encrypted ? 0 : crc32(payload);
69+
const flags = 0x0800 | (entry.encrypted ? 0x0001 : 0);
5770
const local = Buffer.alloc(30);
5871
local.writeUInt32LE(0x04034b50, 0);
5972
local.writeUInt16LE(20, 4);
60-
local.writeUInt16LE(0x0800, 6);
73+
local.writeUInt16LE(flags, 6);
6174
local.writeUInt32LE(checksum, 14);
6275
local.writeUInt32LE(data.length, 18);
63-
local.writeUInt32LE(data.length, 22);
76+
local.writeUInt32LE(payload.length, 22);
6477
local.writeUInt16LE(name.length, 26);
6578
const central = Buffer.alloc(46);
6679
central.writeUInt32LE(0x02014b50, 0);
6780
central.writeUInt16LE((3 << 8) | 20, 4);
6881
central.writeUInt16LE(20, 6);
69-
central.writeUInt16LE(0x0800, 8);
82+
central.writeUInt16LE(flags, 8);
7083
central.writeUInt32LE(checksum, 16);
7184
central.writeUInt32LE(data.length, 20);
72-
central.writeUInt32LE(data.length, 24);
85+
central.writeUInt32LE(payload.length, 24);
7386
central.writeUInt16LE(name.length, 28);
7487
central.writeUInt32LE((entry.mode << 16) >>> 0, 38);
7588
central.writeUInt32LE(offset, 42);

‎packages/host-kit/src/internal/archive-extraction-tar.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { test } from 'vitest';
44
import { AppError } from '@agent-device/kernel/errors';
55
import { extractTarArchive } from './archive-extraction-tar.ts';
66
import { ArchiveBudget } from './archive-safety.ts';
7-
import { createArchiveWorkspace, createTruncatedTgz } from './archive-extraction.fixtures.ts';
7+
import { createArchiveWorkspace, createTruncatedTgz } from '../archive.fixtures.ts';
88

99
test('tar inspection rejects declared bytes before draining an over-budget entry', async () => {
1010
const workspace = await createArchiveWorkspace();

‎packages/host-kit/src/internal/archive-extraction-zip.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@ import { test } from 'vitest';
44
import { AppError } from '@agent-device/kernel/errors';
55
import { extractZipArchive } from './archive-extraction-zip.ts';
66
import { ArchiveBudget } from './archive-safety.ts';
7-
import {
8-
createArchiveWorkspace,
9-
createZipWithEncryptedSecondEntry,
10-
} from './archive-extraction.fixtures.ts';
7+
import { createArchiveWorkspace, createZipWithEncryptedSecondEntry } from '../archive.fixtures.ts';
118

129
test('zip inspection stops at the entry budget before inspecting later entries', async () => {
1310
const workspace = await createArchiveWorkspace();

‎packages/host-kit/src/internal/archive-extraction.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
writeZipFixture,
1010
type TarFixtureEntry,
1111
type ZipFixtureEntry,
12-
} from './archive-extraction.fixtures.ts';
12+
} from '../archive.fixtures.ts';
1313
import { mkdtempForTest } from './tmp-dir.fixtures.ts';
1414

1515
const S_IFREG = 0o100000;

‎packages/platform-android/src/__tests__/emulator-lifecycle.test.ts‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -110,18 +110,22 @@ function bootPropFailure(stdout: string, stderr: string) {
110110
};
111111
}
112112

113-
/** A poll that keeps missing like a real booting device does, and actually burns wall-clock time. */
113+
/** A poll that keeps missing like a real booting device does and burns the given clock time. */
114114
function slowBootPropFailure(delayMs: number) {
115115
return async () => {
116116
await new Promise((resolve) => setTimeout(resolve, delayMs));
117117
return { stdout: '', stderr: '', exitCode: 1 };
118118
};
119119
}
120120

121-
async function expectBootFailure(
121+
/**
122+
* Starts the wait and resolves to the typed failure it must produce. The caller drives any timers
123+
* between starting and awaiting when the test runs on a fake clock.
124+
*/
125+
function startedBootFailure(
122126
timeoutMs: number,
123127
): Promise<{ code: string; message: string; details: Record<string, unknown> }> {
124-
return await waitForAndroidBoot('emulator-5554', timeoutMs).then(
128+
return waitForAndroidBoot('emulator-5554', timeoutMs).then(
125129
() => {
126130
throw new Error('expected waitForAndroidBoot to reject');
127131
},
@@ -133,6 +137,12 @@ async function expectBootFailure(
133137
);
134138
}
135139

140+
async function expectBootFailure(
141+
timeoutMs: number,
142+
): Promise<{ code: string; message: string; details: Record<string, unknown> }> {
143+
return await startedBootFailure(timeoutMs);
144+
}
145+
136146
test('a boot poll against an offline device fails as an adb transport loss without retrying', async () => {
137147
mockRunCmd.mockImplementation(bootPropFailure('', 'error: device offline'));
138148

@@ -149,21 +159,28 @@ test('a boot poll against an offline device fails as an adb transport loss witho
149159
});
150160

151161
test('a still-booting device past its deadline reports an Android boot timeout', async () => {
152-
// 80 ms per poll with a 50 ms retry cadence crosses the 150 ms deadline inside the second
153-
// poll, so the failure is a device that ran out of budget, not three fast poll misses.
154-
mockRunCmd.mockImplementation(slowBootPropFailure(80));
155-
156-
const failure = await expectBootFailure(150);
157-
158-
assert.equal(failure.code, 'COMMAND_FAILED');
159-
assert.equal(failure.message, 'Android device did not finish booting in time');
160-
assert.equal(failure.details.reason, 'ANDROID_BOOT_TIMEOUT');
161-
assert.equal(failure.details.timeoutMs, 150);
162-
assert.equal(Number(failure.details.elapsedMs) >= 150, true);
163-
assert.equal(mockRunCmd.mock.calls.length, 2);
164-
// The classifier sees BOOT_COMMAND_FAILED from the still-booting throw; the caller must
165-
// normalize it to the timeout, not trust the raw classification.
166-
assert.notEqual(failure.details.reason, 'BOOT_COMMAND_FAILED');
162+
// On a fake clock: each poll burns 80 ms, and the 150 ms deadline is crossed inside the
163+
// second poll, so the failure is a device that ran out of budget, not a run of fast poll
164+
// misses. Fake timers keep the 80 ms/150 ms geometry exact instead of racing the host.
165+
vi.useFakeTimers();
166+
try {
167+
mockRunCmd.mockImplementation(slowBootPropFailure(80));
168+
169+
const started = startedBootFailure(150);
170+
await vi.runAllTimersAsync();
171+
const failure = await started;
172+
173+
assert.equal(failure.code, 'COMMAND_FAILED');
174+
assert.equal(failure.message, 'Android device did not finish booting in time');
175+
// The classifier sees BOOT_COMMAND_FAILED from the still-booting throw; the caller must
176+
// normalize it to the timeout rather than trust the raw classification.
177+
assert.equal(failure.details.reason, 'ANDROID_BOOT_TIMEOUT');
178+
assert.equal(failure.details.timeoutMs, 150);
179+
assert.equal(Number(failure.details.elapsedMs) >= 150, true);
180+
assert.equal(mockRunCmd.mock.calls.length, 2);
181+
} finally {
182+
vi.useRealTimers();
183+
}
167184
});
168185

169186
test('a TOOL_MISSING adb inside the boot loop keeps its error code and hint', async () => {

‎packages/platform-apple/src/core/__tests__/install-artifact.test.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import fs from 'node:fs/promises';
44
import path from 'node:path';
5-
import { runCmdSync } from '@agent-device/host-kit/command';
5+
import { writeZipFixture } from '@agent-device/host-kit/archive-fixtures';
66
import { AppError } from '@agent-device/kernel/errors';
77
import { createLocalAppleToolProvider, withAppleToolProvider } from '../tool-provider.ts';
88
import { prepareIosInstallArtifact } from '../install-artifact.ts';
@@ -99,27 +99,29 @@ test('prepareIosInstallArtifact keeps a non-dotted hint a name-only match', asyn
9999

100100
type PayloadBundleFixture = { name: string; bundleId?: string };
101101

102+
const ZIP_DIR_MODE = 0o040000 | 0o755;
103+
const ZIP_FILE_MODE = 0o100644;
104+
102105
/**
103106
* Builds a real zip-format IPA carrying the given `Payload/*.app` bundles and runs `run` against
104-
* it. Info.plist reads go through a fake tool provider keyed by the bundle's own directory, so
107+
* it, written through host-kit's own zip fixture so the test needs no `zip` binary on the host.
108+
* Info.plist reads go through a fake tool provider keyed by the bundle's own directory, so
105109
* each payload reports an identity of its own without plist tooling on the host.
106110
*/
107111
async function withMultiPayloadIpa(
108112
bundles: PayloadBundleFixture[],
109113
run: (ipaPath: string) => Promise<void>,
110114
): Promise<void> {
111115
const root = await mkdtempForTest('agent-device-ios-multi-payload-');
112-
const staging = path.join(root, 'staging');
113-
for (const bundle of bundles) {
114-
const appDir = path.join(staging, 'Payload', `${bundle.name}.app`);
115-
await fs.mkdir(appDir, { recursive: true });
116-
await fs.writeFile(
117-
path.join(appDir, 'Info.plist'),
118-
'<?xml version="1.0" encoding="UTF-8"?><plist version="1.0"><dict/></plist>',
119-
);
120-
}
121116
const ipaPath = path.join(root, 'multi.ipa');
122-
runCmdSync('zip', ['-qr', ipaPath, '.'], { cwd: staging });
117+
await writeZipFixture(ipaPath, [
118+
{ name: 'Payload/', mode: ZIP_DIR_MODE },
119+
...bundles.map(({ name }) => ({
120+
name: `Payload/${name}.app/Info.plist`,
121+
mode: ZIP_FILE_MODE,
122+
data: '<?xml version="1.0" encoding="UTF-8"?><plist version="1.0"><dict/></plist>',
123+
})),
124+
]);
123125

124126
const provider = createLocalAppleToolProvider({
125127
plist: {

‎packages/platform-apple/src/core/__tests__/physical-device-files.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { test } from 'vitest';
22
import assert from 'node:assert/strict';
3+
import { AppError } from '@agent-device/kernel/errors';
34
import { copyCoreDeviceRunnerFile } from '../physical-device-files.ts';
45
import { IOS_DEVICE } from '../../__tests__/device-fixtures.ts';
56
import { withFakeAppleTool, type FakeAppleToolScript } from '../../__tests__/fake-apple-tool.ts';
@@ -83,7 +84,16 @@ test('copyCoreDeviceRunnerFile refuses to start once the deadline is spent', asy
8384
async () => await copyCoreDeviceRunnerFile(IOS_DEVICE, '/tmp/shot.png', '/out/shot.png', 0),
8485
{ device: IOS_DEVICE },
8586
),
86-
/iOS runner file copy timed out after 0ms/,
87+
(error: unknown) => {
88+
// The message alone says only "timed out"; the typed step is what tells the caller the
89+
// spent deadline was the file copy's, not another phase of the physical-device flow.
90+
assert.ok(error instanceof AppError);
91+
assert.equal(error.code, 'COMMAND_FAILED');
92+
assert.match(error.message, /iOS runner file copy timed out after 0ms/);
93+
assert.equal(error.details?.step, 'runner file copy');
94+
assert.equal(error.details?.timeoutMs, 0);
95+
return true;
96+
},
8797
);
8898
assert.deepEqual(calls, []);
8999
});

0 commit comments

Comments
 (0)