diff --git a/src/commands/cloud.ts b/src/commands/cloud.ts index db1c0fb..081ea17 100644 --- a/src/commands/cloud.ts +++ b/src/commands/cloud.ts @@ -20,6 +20,7 @@ import { import { MoropoService } from '../services/moropo.service.js'; import { ReportDownloadService } from '../services/report-download.service.js'; import { + deviceFromResultRow, ResultsPollingService, RunFailedError, } from '../services/results-polling.service.js'; @@ -31,8 +32,10 @@ import { EAndroidDevices, EiOSDevices, EiOSVersions, + isIosMatrixConfig, } from '../types/domain/device.types.js'; import { resolveAuth } from '../utils/auth.js'; +import { matrixIsIos, parseDeviceMatrix } from '../utils/device-matrix.js'; import { detectCiContext, isCI } from '../utils/ci.js'; import { CliError, @@ -211,6 +214,10 @@ export const cloudCommand = defineCommand({ 'android-device', ); 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); const json = Boolean(args.json); const jsonFileFlag = Boolean(args['json-file']); const jsonFileName = args['json-file-name'] as string | undefined; @@ -502,6 +509,28 @@ export const cloudCommand = defineCommand({ { debug, logger: (m: string) => out(m) }, ); + // Validate every device-matrix cell up front — each on its own, against + // the same compatibility matrix — so an unsupported cell fails fast, + // naming it, before anything is uploaded. + for (const cfg of deviceMatrix) { + if (isIosMatrixConfig(cfg)) { + deviceValidationService.validateiOSDevice( + cfg.iOSVersion, + cfg.iOSDevice, + compatibilityData, + { debug, logger: (m: string) => out(m) }, + ); + } else { + deviceValidationService.validateAndroidDevice( + cfg.androidApiLevel, + cfg.androidDevice, + cfg.googlePlay ?? googlePlay, + compatibilityData, + { debug, logger: (m: string) => out(m) }, + ); + } + } + if (maestroChromeOnboarding && !androidApiLevel && !androidDevice) { warnOut( 'The --maestro-chrome-onboarding flag only applies to Android tests and will be ignored for iOS tests.', @@ -639,6 +668,8 @@ export const cloudCommand = defineCommand({ 'include-tags': includeTags, 'exclude-tags': excludeTags, 'exclude-flows': excludeFlows, + 'ios-config': iosConfigFlags, + 'android-config': androidConfigFlags, }; for (const [k, v] of Object.entries(args)) { if (!canonicalFlagKeys.has(k)) continue; @@ -762,6 +793,7 @@ export const cloudCommand = defineCommand({ continueOnFailure, debug, deviceLocale, + deviceMatrix, env, executionPlan, flowFile, @@ -784,6 +816,49 @@ export const cloudCommand = defineCommand({ disableAnimations, }); + // Device-matrix cost preview: the server prices the exact fan-out (quote + // == charge) so the user sees the cell count and estimated cost before the + // flow zip is uploaded. An unsupported cell fails fast here. Skipped when + // there is no matrix, and tolerant of older APIs that lack the endpoint. + if (deviceMatrix.length > 0) { + const estimate = await ApiGateway.estimateMatrix(apiUrl, auth, fields); + if (estimate) { + const osPrefix = matrixIsIos(deviceMatrix) ? 'iOS' : 'API'; + const rows = ui.fields([ + ['cells', colors.highlight(String(estimate.cellCount))], + ['est. cost', colors.highlight(`$${estimate.totalCost.toFixed(2)}`)], + ]); + for (const col of estimate.columns) { + const label = [ + col.deviceName, + col.osVersion && `${osPrefix} ${col.osVersion}`, + col.googlePlay && 'Play', + ] + .filter(Boolean) + .join(' · '); + rows.push( + ...ui.fields([ + [ + label, + colors.dim( + `${col.flowCount} flow${col.flowCount === 1 ? '' : 's'} · $${col.cost.toFixed(2)}`, + ), + ], + ]), + ); + } + if (estimate.excludedFlows.length > 0) { + rows.push( + colors.dim( + `${estimate.excludedFlows.length} flow${estimate.excludedFlows.length === 1 ? '' : 's'} target their own device (excluded from the matrix)`, + ), + ); + } + out(ui.section('Device matrix')); + out(ui.branch(rows)); + } + } + // New path: upload the zip directly to storage, then submit a JSON test // referencing it. Older API deployments lack these endpoints — a real API // 404s (route undefined), some proxies 405 (path/method not allowed); in @@ -865,6 +940,7 @@ export const cloudCommand = defineCommand({ consoleUrl: url, status: 'PENDING', tests: results.map((r) => ({ + device: deviceFromResultRow(r), fileName: r.test_file_name, flowName: testMetadataMap[r.test_file_name]?.flowName || diff --git a/src/config/flags/device.flags.ts b/src/config/flags/device.flags.ts index 37135d4..1fa81df 100644 --- a/src/config/flags/device.flags.ts +++ b/src/config/flags/device.flags.ts @@ -42,6 +42,14 @@ export const deviceFlags = { type: 'string', description: `[iOS only] iOS version to run your flow against (options: ${iosVersions})`, }, + 'ios-config': { + 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.`, + }, + 'android-config': { + 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.`, + }, orientation: { type: 'string', description: diff --git a/src/gateways/api-gateway.ts b/src/gateways/api-gateway.ts index 7ba37b4..48eeb18 100644 --- a/src/gateways/api-gateway.ts +++ b/src/gateways/api-gateway.ts @@ -588,6 +588,52 @@ export const ApiGateway = { } }, + /** + * Dry-run cost + cell-count estimate for a (possibly device-matrix) + * submission. Runs the same resolve → validate → fan-out → price core as the + * submit path server-side, without persisting. The preview is best-effort: + * an API that predates this endpoint 404s (route undefined) or 405s (route + * matched a sibling path, no POST) — in either case return null so the caller + * proceeds without a preview. A 400 (an invalid config) still surfaces as a + * normal API error so the CLI can fail fast before uploading the flow zip. + */ + async estimateMatrix(baseUrl: string, auth: AuthContext, body: Record) { + try { + const res = await fetch(`${baseUrl}/uploads/estimateMatrix`, { + body: JSON.stringify(body), + headers: { + 'content-type': 'application/json', + ...auth.headers, + }, + method: 'POST', + }); + if (res.status === 404 || res.status === 405) { + return null; + } + if (!res.ok) { + await this.handleApiError(res, 'Failed to estimate device matrix'); + } + return await parseJsonResponse<{ + cellCount: number; + totalCost: number; + excludedFlows: string[]; + columns: Array<{ + deviceName: string; + osVersion: string; + googlePlay: boolean; + flowCount: number; + cost: number; + }>; + }>(res, 'Failed to estimate device matrix'); + } catch (error) { + if (error instanceof TypeError && error.message === 'fetch failed') { + throw this.enhanceFetchError(error, `${baseUrl}/uploads/estimateMatrix`); + } + + throw error; + } + }, + /** * Generic report download method that handles both junit and allure reports diff --git a/src/services/results-polling.service.ts b/src/services/results-polling.service.ts index c1cf800..774fc66 100644 --- a/src/services/results-polling.service.ts +++ b/src/services/results-polling.service.ts @@ -47,10 +47,46 @@ export interface TestMetadata { tags: string[]; } +/** + * The device a result ran on. Additive: single-device runs are unchanged, and + * a device matrix disambiguates two `tests[]` entries that share a `name` (the + * same flow on two devices) by their device. + */ +export interface TestDevice { + googlePlay?: boolean; + name?: string; + osVersion?: string; +} + +/** + * Structured device for a result row. Prefers the friendly deviceName/osVersion + * the per-flow targeting / matrix fan-out stamps onto each result's config + * (#1097), falling back to the raw simulator_name for older rows. Returns + * undefined when neither is present, so single-device runs that predate the + * field simply omit `device`. Shared by the sync polling path and the async + * (--async --json) path so both emit an identical device shape. + */ +export function deviceFromResultRow(r: { + config?: unknown; + simulator_name?: string | null; +}): TestDevice | undefined { + const config = (r.config ?? {}) as { deviceName?: string; osVersion?: string }; + const sim = r.simulator_name ?? undefined; + const name = config.deviceName ?? sim; + if (!name && !config.osVersion) return undefined; + return { + name, + osVersion: config.osVersion, + googlePlay: sim ? /(_PLAY|-play)$/.test(sim) : undefined, + }; +} + export interface PollingResult { consoleUrl: string; status: 'FAILED' | 'PASSED'; tests: Array<{ + /** Device this result ran on (present when the API reports it). */ + device?: TestDevice; durationSeconds: null | number; failReason?: string; /** File path of the test (same as name, for clarity) */ @@ -311,6 +347,12 @@ export class ResultsPollingService { ? 'PASSED' : 'FAILED', tests: resultsWithoutEarlierTries.map((r) => ({ + // r carries config/simulator_name at runtime; the committed generated + // types lag the API (regenerated wholesale from dev's swagger), so read + // them through the helper's structural type. + device: deviceFromResultRow( + r as { config?: unknown; simulator_name?: string | null }, + ), durationSeconds: r.duration_seconds ?? null, failReason: r.status === 'FAILED' ? r.fail_reason || 'No reason provided' : undefined, diff --git a/src/services/test-submission.service.ts b/src/services/test-submission.service.ts index 5d4aea6..bbff215 100644 --- a/src/services/test-submission.service.ts +++ b/src/services/test-submission.service.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import * as path from 'node:path'; import { compressFilesFromRelativePath } from '../methods.js'; +import { DeviceMatrixConfig } from '../types/domain/device.types.js'; import { toPortableRelativePath } from '../utils/paths.js'; import { IExecutionPlan } from './execution-plan.service.js'; @@ -15,6 +16,7 @@ export interface TestSubmissionConfig { continueOnFailure?: boolean; debug?: boolean; deviceLocale?: string; + deviceMatrix?: DeviceMatrixConfig[]; disableAnimations?: boolean; env?: string[]; executionPlan: IExecutionPlan; @@ -85,6 +87,7 @@ export class TestSubmissionService { maestroChromeOnboarding, raw, disableAnimations, + deviceMatrix, debug = false, logger, } = config; @@ -183,7 +186,23 @@ export class TestSubmissionService { // Note: googlePlay is now included in configPayload below instead of as a separate field // to work around a FormData parsing issue in the API - const targetPlatform = iOSDevice || iOSVersion ? 'ios' : 'android'; + // Explicit device matrix (one upload, N cells). Only sent when present, so + // single-device submissions stay byte-identical. + if (deviceMatrix && deviceMatrix.length > 0) { + fields.deviceMatrix = JSON.stringify(deviceMatrix); + } + + // Platform used only to pick which workspace-config disableAnimations flag + // applies. A device matrix is single-platform; its first cell decides. Fall + // back to the scalar iOS flags for single-device submissions. + const matrixPlatform = + deviceMatrix && deviceMatrix.length > 0 + ? 'iOSDevice' in deviceMatrix[0] + ? 'ios' + : 'android' + : undefined; + const targetPlatform = + matrixPlatform ?? (iOSDevice || iOSVersion ? 'ios' : 'android'); const configYamlDisableAnimations = targetPlatform === 'ios' ? Boolean(workspaceConfig?.platform?.ios?.disableAnimations) diff --git a/src/types/domain/device.types.ts b/src/types/domain/device.types.ts index 064c23a..45e2632 100644 --- a/src/types/domain/device.types.ts +++ b/src/types/domain/device.types.ts @@ -40,3 +40,17 @@ export enum EAndroidApiLevels { 'thirtyTwo' = '32', 'twentyNine' = '29', } + +/** + * One explicit device-matrix cell. iOS entries carry {iOSDevice, iOSVersion}; + * Android entries carry {androidDevice, androidApiLevel} plus an optional Play + * channel. Sent to the API as the `deviceMatrix` array; every non-targeted flow + * runs once per cell. There is no cross-product — each entry is one cell. + */ +export type DeviceMatrixConfig = + | { iOSDevice: string; iOSVersion: string } + | { androidApiLevel: string; androidDevice: string; googlePlay?: boolean }; + +export const isIosMatrixConfig = ( + c: DeviceMatrixConfig, +): c is { iOSDevice: string; iOSVersion: string } => 'iOSDevice' in c; diff --git a/src/utils/device-matrix.ts b/src/utils/device-matrix.ts new file mode 100644 index 0000000..18e26cc --- /dev/null +++ b/src/utils/device-matrix.ts @@ -0,0 +1,68 @@ +import { + DeviceMatrixConfig, + isIosMatrixConfig, +} from '../types/domain/device.types.js'; +import { CliError } from './cli.js'; + +/** + * Parse repeated `--ios-config :` and + * `--android-config :[: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. + * + * A device matrix is single-platform (one upload, one binary), so mixing iOS + * and Android configs is rejected here before anything is uploaded. + * + * @throws CliError on malformed syntax or a mixed-platform matrix. + */ +export function parseDeviceMatrix( + iosConfigs: string[], + androidConfigs: string[], +): 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.', + ); + } + + const configs: DeviceMatrixConfig[] = []; + + for (const raw of iosConfigs) { + 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.`, + ); + } + configs.push({ iOSDevice: parts[0], iOSVersion: parts[1] }); + } + + for (const raw of androidConfigs) { + const parts = raw.split(':'); + // : with an optional trailing :play for a Play cell. + if ( + parts.length < 2 || + parts.length > 3 || + !parts[0] || + !parts[1] || + (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.`, + ); + } + configs.push({ + androidDevice: parts[0], + androidApiLevel: parts[1], + ...(parts.length === 3 ? { googlePlay: true } : {}), + }); + } + + return configs; +} + +/** True when the matrix targets iOS (used to pick the validation lookup). */ +export function matrixIsIos(configs: DeviceMatrixConfig[]): boolean { + return configs.length > 0 && isIosMatrixConfig(configs[0]); +} diff --git a/test/integration/cloud.integration.test.ts b/test/integration/cloud.integration.test.ts index 253057f..ab1de5d 100644 --- a/test/integration/cloud.integration.test.ts +++ b/test/integration/cloud.integration.test.ts @@ -163,6 +163,32 @@ appId: com.example.app }); }); + // #1105 device matrix: repeated --ios-config / --android-config 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`; + + const { stdout } = await exec(command, { timeout: 30_000 }); + // Still one upload with one uploadId — the matrix is a property of the + // upload, not N uploads. + 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`; + + 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`; + + const { output } = await runExpectingFailure(command); + expect(output).to.include('iphone-16'); + }); + }); + describe('device configuration options', () => { // Async non-JSON runs always reach submission against the mock API. // `.include` keeps these robust to incidental extra lines (e.g. the diff --git a/test/integration/helpers.ts b/test/integration/helpers.ts index d60eeb1..0c998f9 100644 --- a/test/integration/helpers.ts +++ b/test/integration/helpers.ts @@ -7,15 +7,54 @@ * assert the success path unconditionally: a dead or missing mock API must * fail the suite, never soften it. */ -import { exec as execCallback } from 'node:child_process'; +import { execFile as execFileCallback } from 'node:child_process'; import * as path from 'node:path'; import { promisify } from 'node:util'; -export const exec = promisify(execCallback); +const execFileAsync = promisify(execFileCallback); /** Absolute path to the built CLI so tests can run with any cwd. */ export const CLI = path.resolve('dist/index.js'); +export interface ExecResult { + stdout: string; + stderr: string; +} + +/** + * Split a `${CLI} …` command line into argv, honouring single/double quotes so + * a quoted multi-word value stays one token. These test commands contain no + * other shell metacharacters, so this is exact for our use. + */ +function tokenize(command: string): string[] { + const tokens: string[] = []; + const pattern = /"([^"]*)"|'([^']*)'|(\S+)/g; + let match: RegExpExecArray | null; + while ((match = pattern.exec(command)) !== null) { + tokens.push(match[1] ?? match[2] ?? match[3]); + } + return tokens; +} + +/** + * Run a test command **without a shell**. Every integration command is + * `${CLI} ` — an absolute script path plus arguments — so we tokenise it + * and invoke the current Node binary directly with the script and args as argv + * (`execFile`, never `exec`). Passing an argument list instead of a shell string + * is CodeQL's recommended fix for `js/shell-command-injection-from-environment`: + * with no shell there is nothing for the (uncontrolled, but trusted) absolute + * paths to inject into. Signature-compatible with the previous + * `promisify(child_process.exec)` so no call site changes. + */ +export async function exec( + command: string, + opts: { cwd?: string; env?: NodeJS.ProcessEnv; timeout?: number } = {}, +): Promise { + const argv = tokenize(command); + const { stdout, stderr } = await execFileAsync(process.execPath, argv, opts); + return { stdout: String(stdout), stderr: String(stderr) }; +} + export const MOCK_API_URL = process.env.MOCK_API_URL ?? 'http://localhost:3001'; /** One of the keys accepted by the mock API's auth shim. */ diff --git a/test/unit/device-matrix.test.ts b/test/unit/device-matrix.test.ts new file mode 100644 index 0000000..7e8c497 --- /dev/null +++ b/test/unit/device-matrix.test.ts @@ -0,0 +1,61 @@ +import { expect } from 'chai'; + +import { CliError } from '../../src/utils/cli.js'; +import { + matrixIsIos, + parseDeviceMatrix, +} 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 + * matrix is single-platform. These are pure and worth pinning precisely. + */ +describe('parseDeviceMatrix', () => { + it('returns an empty matrix when no config flags are passed', () => { + expect(parseDeviceMatrix([], [])).to.deep.equal([]); + expect(matrixIsIos([])).to.equal(false); + }); + + it('parses each --ios-config as exactly one cell (no cross-product)', () => { + const matrix = parseDeviceMatrix( + ['iphone-16:18', 'iphone-16-pro:26'], + [], + ); + expect(matrix).to.deep.equal([ + { iOSDevice: 'iphone-16', iOSVersion: '18' }, + { iOSDevice: 'iphone-16-pro', iOSVersion: '26' }, + ]); + expect(matrixIsIos(matrix)).to.equal(true); + }); + + it('parses --android-config, 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 }, + ]); + }); + + it('rejects a mixed-platform matrix', () => { + expect(() => + parseDeviceMatrix(['iphone-16:18'], ['pixel-7:34']), + ).to.throw(CliError, /cannot mix platforms/i); + }); + + it('rejects malformed iOS syntax, naming the offending value', () => { + expect(() => parseDeviceMatrix(['iphone-16'], [])).to.throw( + CliError, + /iphone-16/, + ); + expect(() => parseDeviceMatrix(['iphone-16:'], [])).to.throw(CliError); + expect(() => parseDeviceMatrix([':18'], [])).to.throw(CliError); + }); + + it('rejects malformed Android syntax and a bad third segment', () => { + expect(() => parseDeviceMatrix([], ['pixel-7'])).to.throw(CliError); + expect(() => parseDeviceMatrix([], ['pixel-7:34:store'])).to.throw( + CliError, + /pixel-7:34:store/, + ); + }); +});