From 5aab4ac67e8f3c683cb4ec64c993b3baf74bb1ca Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:14:23 +0100 Subject: [PATCH 1/2] refactor(cloud)!: rename --ios-config/--android-config to --ios-device-matrix/--android-device-matrix (#1105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns the flag names with the API's deviceMatrix field and says what they actually are. Clean break, no alias: the old names only ever existed in 5.2.0-beta.1 on the beta tag. citty silently ignores unknown flags, so a stale --ios-config would have been dropped and the run would have gone ahead on a single default device while the user believed a whole matrix ran — the exact silent under-testing this feature exists to prevent. Both removed names are therefore rejected explicitly, naming their replacement, covered by unit tests (incl. the --flag=value form). --- src/commands/cloud.ts | 19 ++++++--- src/config/flags/device.flags.ts | 8 ++-- src/utils/device-matrix.ts | 38 +++++++++++++++--- test/integration/cloud.integration.test.ts | 14 +++---- test/unit/device-matrix.test.ts | 46 ++++++++++++++++++++-- 5 files changed, 99 insertions(+), 26 deletions(-) diff --git a/src/commands/cloud.ts b/src/commands/cloud.ts index 081ea17..054ae85 100644 --- a/src/commands/cloud.ts +++ b/src/commands/cloud.ts @@ -35,7 +35,11 @@ import { isIosMatrixConfig, } from '../types/domain/device.types.js'; import { resolveAuth } from '../utils/auth.js'; -import { matrixIsIos, parseDeviceMatrix } from '../utils/device-matrix.js'; +import { + matrixIsIos, + parseDeviceMatrix, + rejectRenamedMatrixFlags, +} from '../utils/device-matrix.js'; import { detectCiContext, isCI } from '../utils/ci.js'; import { CliError, @@ -215,9 +219,12 @@ export const cloudCommand = defineCommand({ ); const androidNoSnapshot = Boolean(args['android-no-snapshot']); // Repeatable device-matrix flags: one validated cell each, no cross-product. - const iosConfigFlags = collectRepeatedFlag(rawArgs, ['--ios-config']); - const androidConfigFlags = collectRepeatedFlag(rawArgs, ['--android-config']); - const deviceMatrix = parseDeviceMatrix(iosConfigFlags, androidConfigFlags); + // Reject the pre-rename names first — citty would otherwise drop them + // silently and run a single device while the user expected a matrix. + rejectRenamedMatrixFlags(rawArgs); + const iosMatrixFlags = collectRepeatedFlag(rawArgs, ['--ios-device-matrix']); + const androidMatrixFlags = collectRepeatedFlag(rawArgs, ['--android-device-matrix']); + const deviceMatrix = parseDeviceMatrix(iosMatrixFlags, androidMatrixFlags); const json = Boolean(args.json); const jsonFileFlag = Boolean(args['json-file']); const jsonFileName = args['json-file-name'] as string | undefined; @@ -668,8 +675,8 @@ export const cloudCommand = defineCommand({ 'include-tags': includeTags, 'exclude-tags': excludeTags, 'exclude-flows': excludeFlows, - 'ios-config': iosConfigFlags, - 'android-config': androidConfigFlags, + 'ios-device-matrix': iosMatrixFlags, + 'android-device-matrix': androidMatrixFlags, }; for (const [k, v] of Object.entries(args)) { if (!canonicalFlagKeys.has(k)) continue; diff --git a/src/config/flags/device.flags.ts b/src/config/flags/device.flags.ts index 1fa81df..62df3de 100644 --- a/src/config/flags/device.flags.ts +++ b/src/config/flags/device.flags.ts @@ -42,13 +42,13 @@ export const deviceFlags = { type: 'string', description: `[iOS only] iOS version to run your flow against (options: ${iosVersions})`, }, - 'ios-config': { + 'ios-device-matrix': { type: 'string', - description: `[iOS only] Device-matrix cell as :, e.g. iphone-16:18. Repeatable — every flow runs once per cell (no cross-product). Cannot be combined with --android-config.`, + description: `[iOS only] Device-matrix cell as :, e.g. iphone-16:18. Repeatable — every flow runs once per cell (no cross-product). Cannot be combined with --android-device-matrix.`, }, - 'android-config': { + 'android-device-matrix': { type: 'string', - description: `[Android only] Device-matrix cell as : (append :play for Google Play), e.g. pixel-7:34 or pixel-7:34:play. Repeatable — every flow runs once per cell (no cross-product). Cannot be combined with --ios-config.`, + description: `[Android only] Device-matrix cell as : (append :play for Google Play), e.g. pixel-7:34 or pixel-7:34:play. Repeatable — every flow runs once per cell (no cross-product). Cannot be combined with --ios-device-matrix.`, }, orientation: { type: 'string', diff --git a/src/utils/device-matrix.ts b/src/utils/device-matrix.ts index 18e26cc..eafc6f8 100644 --- a/src/utils/device-matrix.ts +++ b/src/utils/device-matrix.ts @@ -5,8 +5,36 @@ import { import { CliError } from './cli.js'; /** - * Parse repeated `--ios-config :` and - * `--android-config :[:play]` flags into an explicit device + * Flags renamed during the 5.2 beta line (--ios-config -> --ios-device-matrix). + * + * citty silently ignores unknown flags, so without this guard an old + * `--ios-config` would simply be dropped: the run would go ahead on a single + * default device while the user believed they had tested a whole matrix. That + * is exactly the "you think you tested it" failure the device matrix exists to + * prevent, so fail loudly and name the replacement. + */ +const RENAMED_FLAGS: Record = { + '--android-config': '--android-device-matrix', + '--ios-config': '--ios-device-matrix', +}; + +/** @throws CliError naming the replacement if a removed flag is still used. */ +export function rejectRenamedMatrixFlags(rawArgs: string[]): void { + for (const [removed, replacement] of Object.entries(RENAMED_FLAGS)) { + const used = rawArgs.some( + (arg) => arg === removed || arg.startsWith(`${removed}=`), + ); + if (used) { + throw new CliError( + `${removed} was renamed to ${replacement}. Use ${replacement} : (repeat it once per device).`, + ); + } + } +} + +/** + * Parse repeated `--ios-device-matrix :` and + * `--android-device-matrix :[:play]` flags into an explicit device * matrix. Each entry is one validated cell — there is NO cross-product, because * the compatibility matrix is ragged and a cross-product would invent cells the * user never asked for. @@ -22,7 +50,7 @@ export function parseDeviceMatrix( ): DeviceMatrixConfig[] { if (iosConfigs.length > 0 && androidConfigs.length > 0) { throw new CliError( - 'A device matrix cannot mix platforms: use either --ios-config or --android-config, not both. One upload runs one binary.', + 'A device matrix cannot mix platforms: use either --ios-device-matrix or --android-device-matrix, not both. One upload runs one binary.', ); } @@ -32,7 +60,7 @@ export function parseDeviceMatrix( const parts = raw.split(':'); if (parts.length !== 2 || !parts[0] || !parts[1]) { throw new CliError( - `Invalid --ios-config "${raw}". Expected :, e.g. iphone-16:18.`, + `Invalid --ios-device-matrix "${raw}". Expected :, e.g. iphone-16:18.`, ); } configs.push({ iOSDevice: parts[0], iOSVersion: parts[1] }); @@ -49,7 +77,7 @@ export function parseDeviceMatrix( (parts.length === 3 && parts[2] !== 'play') ) { throw new CliError( - `Invalid --android-config "${raw}". Expected : or ::play, e.g. pixel-7:34 or pixel-7:34:play.`, + `Invalid --android-device-matrix "${raw}". Expected : or ::play, e.g. pixel-7:34 or pixel-7:34:play.`, ); } configs.push({ diff --git a/test/integration/cloud.integration.test.ts b/test/integration/cloud.integration.test.ts index ab1de5d..7d052af 100644 --- a/test/integration/cloud.integration.test.ts +++ b/test/integration/cloud.integration.test.ts @@ -163,10 +163,10 @@ appId: com.example.app }); }); - // #1105 device matrix: repeated --ios-config / --android-config cells. + // #1105 device matrix: repeated --ios-device-matrix / --android-device-matrix cells. describe('device matrix', () => { - it('accepts a repeated --ios-config matrix and still yields one upload', async () => { - const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-config iphone-16:18 --ios-config iphone-16-pro:26 --async --json`; + it('accepts a repeated --ios-device-matrix matrix and still yields one upload', async () => { + const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-device-matrix iphone-16:18 --ios-device-matrix iphone-16-pro:26 --async --json`; const { stdout } = await exec(command, { timeout: 30_000 }); // Still one upload with one uploadId — the matrix is a property of the @@ -174,15 +174,15 @@ appId: com.example.app expectAsyncRunJson(stdout); }); - it('rejects mixing --ios-config and --android-config before any upload', async () => { - const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-config iphone-16:18 --android-config pixel-7:34`; + it('rejects mixing --ios-device-matrix and --android-device-matrix before any upload', async () => { + const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-device-matrix iphone-16:18 --android-device-matrix pixel-7:34`; const { output } = await runExpectingFailure(command); expect(output.toLowerCase()).to.include('cannot mix platforms'); }); - it('rejects a malformed --ios-config, naming the value', async () => { - const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-config iphone-16`; + it('rejects a malformed --ios-device-matrix, naming the value', async () => { + const command = `${CLI} cloud ${iosAppFile} ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --ios-device-matrix iphone-16`; const { output } = await runExpectingFailure(command); expect(output).to.include('iphone-16'); diff --git a/test/unit/device-matrix.test.ts b/test/unit/device-matrix.test.ts index 7e8c497..f187c40 100644 --- a/test/unit/device-matrix.test.ts +++ b/test/unit/device-matrix.test.ts @@ -4,11 +4,12 @@ import { CliError } from '../../src/utils/cli.js'; import { matrixIsIos, parseDeviceMatrix, + rejectRenamedMatrixFlags, } from '../../src/utils/device-matrix.js'; /** - * The device matrix is the load-bearing part of #1105: each --ios-config / - * --android-config names exactly one cell, there is no cross-product, and a + * The device matrix is the load-bearing part of #1105: each --ios-device-matrix / + * --android-device-matrix names exactly one cell, there is no cross-product, and a * matrix is single-platform. These are pure and worth pinning precisely. */ describe('parseDeviceMatrix', () => { @@ -17,7 +18,7 @@ describe('parseDeviceMatrix', () => { expect(matrixIsIos([])).to.equal(false); }); - it('parses each --ios-config as exactly one cell (no cross-product)', () => { + it('parses each --ios-device-matrix as exactly one cell (no cross-product)', () => { const matrix = parseDeviceMatrix( ['iphone-16:18', 'iphone-16-pro:26'], [], @@ -29,7 +30,7 @@ describe('parseDeviceMatrix', () => { expect(matrixIsIos(matrix)).to.equal(true); }); - it('parses --android-config, with :play marking a Google Play cell', () => { + it('parses --android-device-matrix, with :play marking a Google Play cell', () => { expect(parseDeviceMatrix([], ['pixel-7:34', 'pixel-7:34:play'])).to.deep.equal([ { androidDevice: 'pixel-7', androidApiLevel: '34' }, { androidDevice: 'pixel-7', androidApiLevel: '34', googlePlay: true }, @@ -59,3 +60,40 @@ describe('parseDeviceMatrix', () => { ); }); }); + +/** + * citty silently drops unknown flags, so a stale `--ios-config` would otherwise + * run on ONE default device while the user believed a whole matrix ran — the + * exact silent under-testing this feature exists to prevent. It must fail loudly. + */ +describe('rejectRenamedMatrixFlags', () => { + it('rejects the pre-rename flags, naming the replacement', () => { + expect(() => + rejectRenamedMatrixFlags(['cloud', '--ios-config', 'iphone-16:18']), + ).to.throw(CliError, /--ios-config was renamed to --ios-device-matrix/); + + expect(() => + rejectRenamedMatrixFlags(['cloud', '--android-config', 'pixel-7:34']), + ).to.throw( + CliError, + /--android-config was renamed to --android-device-matrix/, + ); + }); + + it('also catches the --flag=value form', () => { + expect(() => + rejectRenamedMatrixFlags(['cloud', '--ios-config=iphone-16:18']), + ).to.throw(CliError, /--ios-device-matrix/); + }); + + it('allows the new flags and unrelated args through', () => { + expect(() => + rejectRenamedMatrixFlags([ + 'cloud', + '--ios-device-matrix', + 'iphone-16:18', + '--async', + ]), + ).to.not.throw(); + }); +}); From 003833c9a552ee43a8af12b37eefee86a12fd96e Mon Sep 17 00:00:00 2001 From: finalerock44 <77282157+finalerock44@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:34:22 +0100 Subject: [PATCH 2/2] refactor(cloud): drop the renamed-flag guard The old --ios-config/--android-config names only ever existed in 5.2.0-beta.1, which nobody consumed, so there is no one to migrate. Guarding against them was dead code. --- src/commands/cloud.ts | 9 +------- src/utils/device-matrix.ts | 28 ------------------------ test/unit/device-matrix.test.ts | 38 --------------------------------- 3 files changed, 1 insertion(+), 74 deletions(-) diff --git a/src/commands/cloud.ts b/src/commands/cloud.ts index 054ae85..8eb2106 100644 --- a/src/commands/cloud.ts +++ b/src/commands/cloud.ts @@ -35,11 +35,7 @@ import { isIosMatrixConfig, } from '../types/domain/device.types.js'; import { resolveAuth } from '../utils/auth.js'; -import { - matrixIsIos, - parseDeviceMatrix, - rejectRenamedMatrixFlags, -} from '../utils/device-matrix.js'; +import { matrixIsIos, parseDeviceMatrix } from '../utils/device-matrix.js'; import { detectCiContext, isCI } from '../utils/ci.js'; import { CliError, @@ -219,9 +215,6 @@ export const cloudCommand = defineCommand({ ); const androidNoSnapshot = Boolean(args['android-no-snapshot']); // Repeatable device-matrix flags: one validated cell each, no cross-product. - // Reject the pre-rename names first — citty would otherwise drop them - // silently and run a single device while the user expected a matrix. - rejectRenamedMatrixFlags(rawArgs); const iosMatrixFlags = collectRepeatedFlag(rawArgs, ['--ios-device-matrix']); const androidMatrixFlags = collectRepeatedFlag(rawArgs, ['--android-device-matrix']); const deviceMatrix = parseDeviceMatrix(iosMatrixFlags, androidMatrixFlags); diff --git a/src/utils/device-matrix.ts b/src/utils/device-matrix.ts index eafc6f8..8e9e5f9 100644 --- a/src/utils/device-matrix.ts +++ b/src/utils/device-matrix.ts @@ -4,34 +4,6 @@ import { } from '../types/domain/device.types.js'; import { CliError } from './cli.js'; -/** - * Flags renamed during the 5.2 beta line (--ios-config -> --ios-device-matrix). - * - * citty silently ignores unknown flags, so without this guard an old - * `--ios-config` would simply be dropped: the run would go ahead on a single - * default device while the user believed they had tested a whole matrix. That - * is exactly the "you think you tested it" failure the device matrix exists to - * prevent, so fail loudly and name the replacement. - */ -const RENAMED_FLAGS: Record = { - '--android-config': '--android-device-matrix', - '--ios-config': '--ios-device-matrix', -}; - -/** @throws CliError naming the replacement if a removed flag is still used. */ -export function rejectRenamedMatrixFlags(rawArgs: string[]): void { - for (const [removed, replacement] of Object.entries(RENAMED_FLAGS)) { - const used = rawArgs.some( - (arg) => arg === removed || arg.startsWith(`${removed}=`), - ); - if (used) { - throw new CliError( - `${removed} was renamed to ${replacement}. Use ${replacement} : (repeat it once per device).`, - ); - } - } -} - /** * Parse repeated `--ios-device-matrix :` and * `--android-device-matrix :[:play]` flags into an explicit device diff --git a/test/unit/device-matrix.test.ts b/test/unit/device-matrix.test.ts index f187c40..8a4cb11 100644 --- a/test/unit/device-matrix.test.ts +++ b/test/unit/device-matrix.test.ts @@ -4,7 +4,6 @@ import { CliError } from '../../src/utils/cli.js'; import { matrixIsIos, parseDeviceMatrix, - rejectRenamedMatrixFlags, } from '../../src/utils/device-matrix.js'; /** @@ -60,40 +59,3 @@ describe('parseDeviceMatrix', () => { ); }); }); - -/** - * citty silently drops unknown flags, so a stale `--ios-config` would otherwise - * run on ONE default device while the user believed a whole matrix ran — the - * exact silent under-testing this feature exists to prevent. It must fail loudly. - */ -describe('rejectRenamedMatrixFlags', () => { - it('rejects the pre-rename flags, naming the replacement', () => { - expect(() => - rejectRenamedMatrixFlags(['cloud', '--ios-config', 'iphone-16:18']), - ).to.throw(CliError, /--ios-config was renamed to --ios-device-matrix/); - - expect(() => - rejectRenamedMatrixFlags(['cloud', '--android-config', 'pixel-7:34']), - ).to.throw( - CliError, - /--android-config was renamed to --android-device-matrix/, - ); - }); - - it('also catches the --flag=value form', () => { - expect(() => - rejectRenamedMatrixFlags(['cloud', '--ios-config=iphone-16:18']), - ).to.throw(CliError, /--ios-device-matrix/); - }); - - it('allows the new flags and unrelated args through', () => { - expect(() => - rejectRenamedMatrixFlags([ - 'cloud', - '--ios-device-matrix', - 'iphone-16:18', - '--async', - ]), - ).to.not.throw(); - }); -});