Skip to content

Commit d16f01c

Browse files
authored
refactor: derive platform allow-lists from the canonical device tuples (#895)
The set of platforms was hand-restated in three places that must agree: the canonical tuples in src/utils/device.ts, the --platform flag's enumValues in cli-flags.ts, and the leaf-platform validation in client-normalizers.ts. The two non-canonical copies could drift. - Export PLATFORMS from device.ts and add an isPlatform() leaf-platform type guard derived from it (excludes the `apple` selector). - Derive the --platform enumValues and usageLabel from PLATFORM_SELECTORS. - Derive normalizeOpenDevice's leaf-platform check from isPlatform(); per-platform udid/serial identifier shaping is unchanged. Behaviorless: the derived sets equal the previous hardcoded sets.
1 parent 5a67623 commit d16f01c

6 files changed

Lines changed: 96 additions & 13 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import { normalizeOpenDevice } from '../client-normalizers.ts';
4+
import { PLATFORMS } from '../utils/device.ts';
5+
6+
test('normalizeOpenDevice accepts exactly the canonical leaf platforms', () => {
7+
for (const platform of PLATFORMS) {
8+
const result = normalizeOpenDevice({
9+
platform,
10+
id: 'device-1',
11+
device: 'Device One',
12+
});
13+
assert.ok(result, `expected platform "${platform}" to be accepted`);
14+
assert.equal(result.platform, platform);
15+
}
16+
// Lock the membership so the derived check cannot silently widen/narrow.
17+
assert.deepEqual([...PLATFORMS], ['ios', 'macos', 'android', 'linux', 'web']);
18+
});
19+
20+
test('normalizeOpenDevice rejects the apple selector and unknown platforms', () => {
21+
// `apple` is a selector, not a concrete device platform, so it must be rejected here.
22+
assert.equal(
23+
normalizeOpenDevice({ platform: 'apple', id: 'device-1', device: 'Device One' }),
24+
undefined,
25+
);
26+
assert.equal(
27+
normalizeOpenDevice({ platform: 'windows', id: 'device-1', device: 'Device One' }),
28+
undefined,
29+
);
30+
assert.equal(
31+
normalizeOpenDevice({ platform: undefined, id: 'device-1', device: 'Device One' }),
32+
undefined,
33+
);
34+
});
35+
36+
test('normalizeOpenDevice preserves per-platform identifier shaping', () => {
37+
const ios = normalizeOpenDevice({
38+
platform: 'ios',
39+
id: 'udid-1',
40+
device: 'iPhone',
41+
ios_simulator_device_set: '/tmp/set',
42+
});
43+
assert.deepEqual(ios?.ios, { udid: 'udid-1', simulatorSetPath: '/tmp/set' });
44+
assert.equal(ios?.android, undefined);
45+
46+
const android = normalizeOpenDevice({
47+
platform: 'android',
48+
id: 'serial-1',
49+
device: 'Pixel',
50+
serial: 'explicit-serial',
51+
});
52+
assert.deepEqual(android?.android, { serial: 'explicit-serial' });
53+
assert.equal(android?.ios, undefined);
54+
});

src/client-normalizers.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { DaemonRequest, SessionRuntimeHints } from './daemon/types.ts';
44
import { AppError, type NormalizedError } from './utils/errors.ts';
55
import type { SnapshotNode } from './utils/snapshot.ts';
66
import { buildAppIdentifiers, buildDeviceIdentifiers } from './client-shared.ts';
7+
import { isPlatform } from './utils/device.ts';
78
import {
89
leaseScopeFromOptions,
910
leaseScopeToCommandFlags,
@@ -183,15 +184,7 @@ export function normalizeOpenDevice(
183184
const platform = value.platform;
184185
const id = readOptionalString(value, 'id');
185186
const name = readOptionalString(value, 'device');
186-
if (
187-
(platform !== 'ios' &&
188-
platform !== 'macos' &&
189-
platform !== 'android' &&
190-
platform !== 'linux' &&
191-
platform !== 'web') ||
192-
!id ||
193-
!name
194-
) {
187+
if (!isPlatform(platform) || !id || !name) {
195188
return undefined;
196189
}
197190
const target = readDeviceTarget(value, 'target');
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import { getFlagDefinition } from '../cli-flags.ts';
4+
import { PLATFORM_SELECTORS } from '../device.ts';
5+
6+
test('--platform enumValues are derived from the canonical PLATFORM_SELECTORS tuple', () => {
7+
const platformFlag = getFlagDefinition('--platform');
8+
assert.ok(platformFlag, 'expected a --platform flag definition');
9+
assert.deepEqual(platformFlag.enumValues, [...PLATFORM_SELECTORS]);
10+
// Guard the exact membership that today's CLI accepts so the derivation cannot drift.
11+
assert.deepEqual(platformFlag.enumValues, ['ios', 'macos', 'android', 'linux', 'web', 'apple']);
12+
});
13+
14+
test('--platform usageLabel lists the same selectors as enumValues', () => {
15+
const platformFlag = getFlagDefinition('--platform');
16+
assert.ok(platformFlag);
17+
assert.equal(platformFlag.usageLabel, `--platform ${PLATFORM_SELECTORS.join('|')}`);
18+
});

src/utils/__tests__/device.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import {
4+
isPlatform,
45
matchesPlatformSelector,
6+
PLATFORMS,
57
resolveApplePlatformName,
68
resolveAppleSimulatorSetPathForSelector,
79
resolveDevice,
@@ -15,6 +17,16 @@ test('matchesPlatformSelector resolves apple selector across Apple platforms', (
1517
assert.equal(matchesPlatformSelector('android', 'apple'), false);
1618
});
1719

20+
test('isPlatform accepts exactly the canonical PLATFORMS tuple', () => {
21+
for (const platform of PLATFORMS) {
22+
assert.equal(isPlatform(platform), true);
23+
}
24+
// The `apple` selector is not a concrete leaf platform.
25+
assert.equal(isPlatform('apple'), false);
26+
assert.equal(isPlatform('windows'), false);
27+
assert.equal(isPlatform(undefined), false);
28+
});
29+
1830
test('resolveApplePlatformName resolves tv and desktop targets', () => {
1931
assert.equal(resolveApplePlatformName('tv'), 'tvOS');
2032
assert.equal(resolveApplePlatformName('mobile'), 'iOS');

src/utils/cli-flags.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { RecordingExportQuality } from '../core/recording-export-quality.ts
33
import type { BackMode } from '../core/back-mode.ts';
44
import type { ClickButton } from '../core/click-button.ts';
55
import type { SwipePattern } from '../core/scroll-gesture.ts';
6-
import type { DeviceTarget, PlatformSelector } from './device.ts';
6+
import { PLATFORM_SELECTORS, type DeviceTarget, type PlatformSelector } from './device.ts';
77
import type {
88
DaemonInstallSource,
99
DaemonServerMode,
@@ -344,8 +344,8 @@ const FLAG_DEFINITIONS: readonly FlagDefinition[] = [
344344
key: 'platform',
345345
names: ['--platform'],
346346
type: 'enum',
347-
enumValues: ['ios', 'macos', 'android', 'linux', 'web', 'apple'],
348-
usageLabel: '--platform ios|macos|android|linux|web|apple',
347+
enumValues: PLATFORM_SELECTORS,
348+
usageLabel: `--platform ${PLATFORM_SELECTORS.join('|')}`,
349349
usageDescription: 'Platform to target (`apple` aliases the Apple automation backend)',
350350
},
351351
{

src/utils/device.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { AppError } from './errors.ts';
22

33
export type ApplePlatform = 'ios' | 'macos';
4-
const PLATFORMS = ['ios', 'macos', 'android', 'linux', 'web'] as const;
4+
export const PLATFORMS = ['ios', 'macos', 'android', 'linux', 'web'] as const;
55
export type Platform = (typeof PLATFORMS)[number];
66
export const PLATFORM_SELECTORS = [...PLATFORMS, 'apple'] as const;
77
export type PlatformSelector = (typeof PLATFORM_SELECTORS)[number];
@@ -46,6 +46,12 @@ export function isApplePlatform(
4646
return platform === 'apple' || platform === 'ios' || platform === 'macos';
4747
}
4848

49+
export function isPlatform(value: unknown): value is Platform {
50+
// Leaf-platform membership derived from the canonical PLATFORMS tuple (excludes the
51+
// `apple` selector, which is not a concrete device platform).
52+
return (PLATFORMS as readonly unknown[]).includes(value);
53+
}
54+
4955
export function matchesPlatformSelector(
5056
platform: Platform,
5157
selector: PlatformSelector | undefined,

0 commit comments

Comments
 (0)