Skip to content

Commit f6ab9ea

Browse files
committed
ios: address PR55 review follow-ups
1 parent 230814a commit f6ab9ea

5 files changed

Lines changed: 85 additions & 42 deletions

File tree

src/core/dispatch.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ export async function dispatchCommand(
239239
case 'snapshot': {
240240
const backend = context?.snapshotBackend ?? 'xctest';
241241
if (device.platform === 'ios') {
242+
// Keep this guard for non-daemon callers that invoke dispatch directly.
242243
if (backend === 'ax' && device.kind !== 'simulator') {
243244
throw new AppError(
244245
'UNSUPPORTED_OPERATION',

src/platforms/ios/__tests__/runner-client.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { DeviceInfo } from '../../../utils/device.ts';
44
import {
55
resolveRunnerBuildDestination,
66
resolveRunnerDestination,
7+
resolveRunnerMaxConcurrentDestinationsFlag,
78
resolveRunnerSigningBuildSettings,
89
} from '../runner-client.ts';
910

@@ -38,6 +39,20 @@ test('resolveRunnerBuildDestination uses generic iOS destination for physical de
3839
assert.equal(resolveRunnerBuildDestination(iosDevice), 'generic/platform=iOS');
3940
});
4041

42+
test('resolveRunnerMaxConcurrentDestinationsFlag uses simulator flag for simulators', () => {
43+
assert.equal(
44+
resolveRunnerMaxConcurrentDestinationsFlag(iosSimulator),
45+
'-maximum-concurrent-test-simulator-destinations',
46+
);
47+
});
48+
49+
test('resolveRunnerMaxConcurrentDestinationsFlag uses device flag for physical devices', () => {
50+
assert.equal(
51+
resolveRunnerMaxConcurrentDestinationsFlag(iosDevice),
52+
'-maximum-concurrent-test-device-destinations',
53+
);
54+
});
55+
4156
test('resolveRunnerSigningBuildSettings returns empty args without env overrides', () => {
4257
assert.deepEqual(resolveRunnerSigningBuildSettings({}), []);
4358
});
@@ -48,6 +63,14 @@ test('resolveRunnerSigningBuildSettings enables automatic signing for device bui
4863
]);
4964
});
5065

66+
test('resolveRunnerSigningBuildSettings ignores device signing overrides for simulator builds', () => {
67+
assert.deepEqual(resolveRunnerSigningBuildSettings({
68+
AGENT_DEVICE_IOS_TEAM_ID: 'ABCDE12345',
69+
AGENT_DEVICE_IOS_SIGNING_IDENTITY: 'Apple Development',
70+
AGENT_DEVICE_IOS_PROVISIONING_PROFILE: 'My Profile',
71+
}, false), []);
72+
});
73+
5174
test('resolveRunnerSigningBuildSettings applies optional overrides when provided', () => {
5275
const settings = resolveRunnerSigningBuildSettings({
5376
AGENT_DEVICE_IOS_TEAM_ID: 'ABCDE12345',

src/platforms/ios/devices.ts

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -41,44 +41,45 @@ export async function listIosDevices(): Promise<DeviceInfo[]> {
4141
throw new AppError('COMMAND_FAILED', 'Failed to parse simctl devices JSON', undefined, err);
4242
}
4343

44-
const devicectlAvailable = await whichCmd('xcrun');
45-
if (devicectlAvailable) {
46-
try {
47-
const jsonPath = path.join(
48-
os.tmpdir(),
49-
`agent-device-devicectl-${process.pid}-${Date.now()}.json`,
50-
);
51-
await runCmd('xcrun', ['devicectl', 'list', 'devices', '--json-output', jsonPath]);
52-
const jsonText = await fs.readFile(jsonPath, 'utf8');
53-
await fs.rm(jsonPath, { force: true });
54-
const payload = JSON.parse(jsonText) as {
55-
result?: {
56-
devices?: Array<{
57-
identifier?: string;
58-
name?: string;
59-
hardwareProperties?: { platform?: string; udid?: string };
60-
deviceProperties?: { name?: string };
61-
connectionProperties?: { tunnelState?: string };
62-
}>;
63-
};
44+
let jsonPath: string | null = null;
45+
try {
46+
jsonPath = path.join(
47+
os.tmpdir(),
48+
`agent-device-devicectl-${process.pid}-${Date.now()}.json`,
49+
);
50+
await runCmd('xcrun', ['devicectl', 'list', 'devices', '--json-output', jsonPath]);
51+
const jsonText = await fs.readFile(jsonPath, 'utf8');
52+
const payload = JSON.parse(jsonText) as {
53+
result?: {
54+
devices?: Array<{
55+
identifier?: string;
56+
name?: string;
57+
hardwareProperties?: { platform?: string; udid?: string };
58+
deviceProperties?: { name?: string };
59+
connectionProperties?: { tunnelState?: string };
60+
}>;
6461
};
65-
for (const device of payload.result?.devices ?? []) {
66-
const platform = device.hardwareProperties?.platform ?? '';
67-
if (platform.toLowerCase().includes('ios')) {
68-
const id = device.hardwareProperties?.udid ?? device.identifier ?? '';
69-
const name = device.name ?? device.deviceProperties?.name ?? id;
70-
if (!id) continue;
71-
devices.push({
72-
platform: 'ios',
73-
id,
74-
name,
75-
kind: 'device',
76-
booted: true,
77-
});
78-
}
62+
};
63+
for (const device of payload.result?.devices ?? []) {
64+
const platform = device.hardwareProperties?.platform ?? '';
65+
if (platform.toLowerCase().includes('ios')) {
66+
const id = device.hardwareProperties?.udid ?? device.identifier ?? '';
67+
const name = device.name ?? device.deviceProperties?.name ?? id;
68+
if (!id) continue;
69+
devices.push({
70+
platform: 'ios',
71+
id,
72+
name,
73+
kind: 'device',
74+
booted: true,
75+
});
7976
}
80-
} catch {
81-
// Ignore devicectl failures; simulators are still supported.
77+
}
78+
} catch {
79+
// Ignore devicectl failures; simulators are still supported.
80+
} finally {
81+
if (jsonPath) {
82+
await fs.rm(jsonPath, { force: true }).catch(() => {});
8283
}
8384
}
8485

src/platforms/ios/runner-client.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ export async function runIosRunnerCommand(
8989
command: RunnerCommand,
9090
options: { verbose?: boolean; logPath?: string; traceLogPath?: string } = {},
9191
): Promise<Record<string, unknown>> {
92+
validateRunnerDevice(device);
9293
if (isReadOnlyRunnerCommand(command.command)) {
9394
return withRetry(
9495
() => executeRunnerCommand(device, command, options),
@@ -232,7 +233,7 @@ async function ensureRunnerSession(
232233
'NO',
233234
'-test-timeouts-enabled',
234235
'NO',
235-
'-maximum-concurrent-test-device-destinations',
236+
resolveRunnerMaxConcurrentDestinationsFlag(device),
236237
'1',
237238
'-xctestrun',
238239
xctestrunPath,
@@ -319,7 +320,7 @@ async function ensureXctestrun(
319320
'AgentDeviceRunner',
320321
'-parallel-testing-enabled',
321322
'NO',
322-
'-maximum-concurrent-test-device-destinations',
323+
resolveRunnerMaxConcurrentDestinationsFlag(device),
323324
'1',
324325
'-destination',
325326
resolveRunnerBuildDestination(device),
@@ -391,16 +392,31 @@ function ensureBootedIfNeeded(device: DeviceInfo): Promise<void> {
391392
return ensureBooted(device.id);
392393
}
393394

395+
function validateRunnerDevice(device: DeviceInfo): void {
396+
if (device.platform !== 'ios') {
397+
throw new AppError('UNSUPPORTED_PLATFORM', `Unsupported platform for iOS runner: ${device.platform}`);
398+
}
399+
if (device.kind !== 'simulator' && device.kind !== 'device') {
400+
throw new AppError('UNSUPPORTED_OPERATION', `Unsupported iOS device kind for runner: ${device.kind}`);
401+
}
402+
}
403+
404+
export function resolveRunnerMaxConcurrentDestinationsFlag(device: DeviceInfo): string {
405+
return device.kind === 'device'
406+
? '-maximum-concurrent-test-device-destinations'
407+
: '-maximum-concurrent-test-simulator-destinations';
408+
}
409+
394410
export function resolveRunnerSigningBuildSettings(
395411
env: NodeJS.ProcessEnv = process.env,
396412
forDevice = false,
397413
): string[] {
414+
if (!forDevice) {
415+
return [];
416+
}
398417
const teamId = env.AGENT_DEVICE_IOS_TEAM_ID?.trim() || '';
399418
const configuredIdentity = env.AGENT_DEVICE_IOS_SIGNING_IDENTITY?.trim() || '';
400419
const profile = env.AGENT_DEVICE_IOS_PROVISIONING_PROFILE?.trim() || '';
401-
if (!forDevice && !teamId && !configuredIdentity && !profile) {
402-
return [];
403-
}
404420
const args = ['CODE_SIGN_STYLE=Automatic'];
405421
if (teamId) {
406422
args.push(`DEVELOPMENT_TEAM=${teamId}`);

test/integration/ios.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@ import { createIntegrationTestContext, runCliJson } from './test-helpers.ts';
66
const session = ['--session', 'ios-test'];
77
const iosTarget = ['--platform', 'ios'];
88
const iosPhysicalUdid = process.env.IOS_UDID?.trim();
9+
let didRunIosPhysicalSession = false;
910

1011
test.after(() => {
1112
runCliJson(['close', ...iosTarget, '--json', ...session]);
12-
if (iosPhysicalUdid) {
13+
if (iosPhysicalUdid && didRunIosPhysicalSession) {
1314
runCliJson(['close', '--platform', 'ios', '--udid', iosPhysicalUdid, '--json', '--session', 'ios-device-test']);
1415
}
1516
});
@@ -90,6 +91,7 @@ test('ios physical device core lifecycle', { skip: shouldSkipIosPhysicalDevice()
9091
});
9192
const deviceSession = ['--session', 'ios-device-test'];
9293
const target = ['--platform', 'ios', '--udid', iosPhysicalUdid as string];
94+
didRunIosPhysicalSession = true;
9395

9496
const openArgs = ['open', 'com.apple.Preferences', ...target, '--json', ...deviceSession];
9597
integration.runStep('open settings (device)', openArgs);

0 commit comments

Comments
 (0)