From 7a8a1437ca2ce28d70872441e13436a05e97c49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 27 Mar 2026 19:27:07 +0100 Subject: [PATCH] refactor: remove remaining test-only DI seams outside session handlers (#303) Replace optional dependency bags in 5 production modules with vi.mock in tests: - interactors.ts: remove InteractorDeps, mock runner-client - screenshot.ts (Android): remove ScreenshotAndroidDeps, mock adb sleep - screenshot.ts (iOS): remove SimulatorScreenshotFlowDeps, make fallback flow internal - runner-xctestrun.ts: remove EnsureXctestrunDeps, add AGENT_DEVICE_PROJECT_ROOT env override - dispatch-resolve.ts: remove ResolveAppleDeviceDeps, mock ios/devices Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/__tests__/dispatch-resolve.test.ts | 64 +++---- src/core/dispatch-resolve.ts | 15 +- .../android/__tests__/snapshot.test.ts | 38 +---- src/platforms/android/screenshot.ts | 28 +-- src/platforms/ios/__tests__/index.test.ts | 161 ------------------ .../ios/__tests__/runner-client.test.ts | 129 -------------- .../__tests__/runner-xctestrun-ensure.test.ts | 160 +++++++++++++++++ src/platforms/ios/runner-xctestrun.ts | 62 ++----- src/platforms/ios/screenshot.ts | 28 +-- src/utils/__tests__/interactors.test.ts | 127 +++++++------- src/utils/interactors.ts | 44 ++--- 11 files changed, 305 insertions(+), 551 deletions(-) create mode 100644 src/platforms/ios/__tests__/runner-xctestrun-ensure.test.ts diff --git a/src/core/__tests__/dispatch-resolve.test.ts b/src/core/__tests__/dispatch-resolve.test.ts index 7e502bbae..e5f0f17bc 100644 --- a/src/core/__tests__/dispatch-resolve.test.ts +++ b/src/core/__tests__/dispatch-resolve.test.ts @@ -1,9 +1,21 @@ -import { test } from 'vitest'; +import { test, vi, beforeEach } from 'vitest'; import assert from 'node:assert/strict'; import { resolveIosDevice } from '../dispatch-resolve.ts'; -import { resolveDevice, type DeviceInfo } from '../../utils/device.ts'; +import { type DeviceInfo } from '../../utils/device.ts'; import { AppError } from '../../utils/errors.ts'; +vi.mock('../../platforms/ios/devices.ts', () => ({ + findBootableIosSimulator: vi.fn(async () => null), + listAppleDevices: vi.fn(async () => []), +})); + +const { findBootableIosSimulator } = await import('../../platforms/ios/devices.ts'); +const mockedFindBootable = vi.mocked(findBootableIosSimulator); + +beforeEach(() => { + vi.clearAllMocks(); +}); + const physical: DeviceInfo = { platform: 'ios', id: 'phys-1', @@ -31,33 +43,18 @@ const bootedSimulator: DeviceInfo = { booted: true, }; -// Helper: creates deps with a controllable findBootableSimulator stub. -function makeDeps(fallbackSimulator: DeviceInfo | null = null) { - let findBootableCalled = false; - return { - deps: { - resolveDevice, - findBootableSimulator: async () => { - findBootableCalled = true; - return fallbackSimulator; - }, - }, - wasFindBootableCalled: () => findBootableCalled, - }; -} - // --- Physical device rejected in favour of simulator fallback --- test('resolveIosDevice prefers fallback simulator over auto-selected physical device', async () => { - const { deps } = makeDeps(simulator); - const result = await resolveIosDevice([physical], { platform: 'ios' }, {}, deps); + mockedFindBootable.mockResolvedValueOnce(simulator); + const result = await resolveIosDevice([physical], { platform: 'ios' }, {}); assert.equal(result.id, 'sim-1'); assert.equal(result.kind, 'simulator'); }); test('resolveIosDevice falls back to physical device when no simulator is found', async () => { - const { deps } = makeDeps(null); - const result = await resolveIosDevice([physical], { platform: 'ios' }, {}, deps); + mockedFindBootable.mockResolvedValueOnce(null); + const result = await resolveIosDevice([physical], { platform: 'ios' }, {}); assert.equal(result.id, 'phys-1'); assert.equal(result.kind, 'device'); }); @@ -65,43 +62,39 @@ test('resolveIosDevice falls back to physical device when no simulator is found' // --- Explicit selectors bypass the fallback --- test('resolveIosDevice keeps physical device when udid is explicit', async () => { - const { deps, wasFindBootableCalled } = makeDeps(simulator); - const result = await resolveIosDevice([physical], { platform: 'ios', udid: 'phys-1' }, {}, deps); + const result = await resolveIosDevice([physical], { platform: 'ios', udid: 'phys-1' }, {}); assert.equal(result.id, 'phys-1'); - assert.equal(wasFindBootableCalled(), false); + assert.equal(mockedFindBootable.mock.calls.length, 0); }); test('resolveIosDevice keeps physical device when deviceName is explicit', async () => { - const { deps, wasFindBootableCalled } = makeDeps(simulator); const result = await resolveIosDevice( [physical], { platform: 'ios', deviceName: 'My iPhone' }, {}, - deps, ); assert.equal(result.id, 'phys-1'); - assert.equal(wasFindBootableCalled(), false); + assert.equal(mockedFindBootable.mock.calls.length, 0); }); // --- Empty device list triggers fallback (P1-A: DEVICE_NOT_FOUND recovery) --- test('resolveIosDevice recovers from empty device list via simulator fallback', async () => { - const { deps } = makeDeps(simulator); - const result = await resolveIosDevice([], { platform: 'ios' }, {}, deps); + mockedFindBootable.mockResolvedValueOnce(simulator); + const result = await resolveIosDevice([], { platform: 'ios' }, {}); assert.equal(result.id, 'sim-1'); assert.equal(result.kind, 'simulator'); }); test('resolveIosDevice throws DEVICE_NOT_FOUND when empty list and no fallback simulator', async () => { - const { deps } = makeDeps(null); - const err = await resolveIosDevice([], { platform: 'ios' }, {}, deps).catch((e) => e); + mockedFindBootable.mockResolvedValueOnce(null); + const err = await resolveIosDevice([], { platform: 'ios' }, {}).catch((e) => e); assert.ok(err instanceof AppError); assert.equal(err.code, 'DEVICE_NOT_FOUND'); }); test('resolveIosDevice rethrows DEVICE_NOT_FOUND from resolveDevice when explicit selector used', async () => { - const { deps } = makeDeps(simulator); - const err = await resolveIosDevice([], { platform: 'ios', udid: 'nonexistent' }, {}, deps).catch( + const err = await resolveIosDevice([], { platform: 'ios', udid: 'nonexistent' }, {}).catch( (e) => e, ); assert.ok(err instanceof AppError); @@ -111,9 +104,8 @@ test('resolveIosDevice rethrows DEVICE_NOT_FOUND from resolveDevice when explici // --- Simulator already in the device list (normal path) --- test('resolveIosDevice returns simulator directly when present in device list', async () => { - const { deps, wasFindBootableCalled } = makeDeps(null); - const result = await resolveIosDevice([physical, bootedSimulator], { platform: 'ios' }, {}, deps); + const result = await resolveIosDevice([physical, bootedSimulator], { platform: 'ios' }, {}); assert.equal(result.id, 'sim-2'); assert.equal(result.kind, 'simulator'); - assert.equal(wasFindBootableCalled(), false); + assert.equal(mockedFindBootable.mock.calls.length, 0); }); diff --git a/src/core/dispatch-resolve.ts b/src/core/dispatch-resolve.ts index 0a8954014..061b9227d 100644 --- a/src/core/dispatch-resolve.ts +++ b/src/core/dispatch-resolve.ts @@ -36,29 +36,21 @@ type AppleDeviceSelector = { serial?: string; }; -type ResolveAppleDeviceDeps = { - resolveDevice: typeof resolveDevice; - findBootableSimulator: typeof findBootableIosSimulator; -}; - /** * Resolves the best iOS device given pre-fetched candidates. When no explicit * device selector was used, physical devices are rejected in favour of a - * bootable simulator discovered via `findBootableSimulator`. - * - * Exported for testing; production callers should use `resolveTargetDevice`. + * bootable simulator discovered via `findBootableIosSimulator`. */ export async function resolveAppleDevice( devices: DeviceInfo[], selector: AppleDeviceSelector, context: { simulatorSetPath?: string }, - deps: ResolveAppleDeviceDeps, ): Promise { const hasExplicitSelector = !!(selector.udid || selector.serial || selector.deviceName); let selected: DeviceInfo | undefined; try { - selected = await deps.resolveDevice(devices, selector, context); + selected = await resolveDevice(devices, selector, context); } catch (err) { // When resolveDevice throws DEVICE_NOT_FOUND and no explicit device // selector was used, attempt the simulator fallback before giving up. @@ -77,7 +69,7 @@ export async function resolveAppleDevice( selector.target !== 'desktop'; if (shouldUseSimulatorFallback && (!selected || selected.kind === 'device')) { - const simulator = await deps.findBootableSimulator({ + const simulator = await findBootableIosSimulator({ simulatorSetPath: context.simulatorSetPath, target: selector.target, }); @@ -127,7 +119,6 @@ export async function resolveTargetDevice(flags: ResolveDeviceFlags): Promise { + const original = await importOriginal(); + return { + ...original, + sleep: vi.fn(async () => {}), + }; +}); + const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); const VALID_PNG = Buffer.from( 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+b9xkAAAAASUVORK5CYII=', @@ -47,34 +55,6 @@ async function withMockedAdb( const catPayload = '#!/bin/bash\ncat "$(dirname "$0")/payload.bin"\n'; -test('screenshotAndroid waits for transient UI to settle before capture', async () => { - const device: DeviceInfo = { - platform: 'android', - id: 'emulator-5554', - name: 'Pixel', - kind: 'emulator', - booted: true, - }; - const events: string[] = []; - - await screenshotAndroid(device, '/tmp/out.png', { - enableDemoMode: async () => { - events.push('enable'); - }, - settle: async (ms) => { - events.push(`settle:${ms}`); - }, - capture: async () => { - events.push('capture'); - }, - disableDemoMode: async () => { - events.push('disable'); - }, - }); - - assert.deepEqual(events, ['enable', 'settle:1000', 'capture', 'disable']); -}); - test('screenshotAndroid writes a valid PNG when output is clean', async () => { await withMockedAdb( 'screenshot-clean-', diff --git a/src/platforms/android/screenshot.ts b/src/platforms/android/screenshot.ts index d807f29fa..d13937210 100644 --- a/src/platforms/android/screenshot.ts +++ b/src/platforms/android/screenshot.ts @@ -8,32 +8,14 @@ import { adbArgs, sleep } from './adb.ts'; const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); const ANDROID_SCREENSHOT_SETTLE_DELAY_MS = 1_000; -type ScreenshotAndroidDeps = { - enableDemoMode: (device: DeviceInfo) => Promise; - settle: (ms: number) => Promise; - capture: (device: DeviceInfo, outPath: string) => Promise; - disableDemoMode: (device: DeviceInfo) => Promise; -}; - -const defaultScreenshotAndroidDeps: ScreenshotAndroidDeps = { - enableDemoMode: enableAndroidDemoMode, - settle: sleep, - capture: captureAndroidScreenshot, - disableDemoMode: disableAndroidDemoMode, -}; - -export async function screenshotAndroid( - device: DeviceInfo, - outPath: string, - deps: ScreenshotAndroidDeps = defaultScreenshotAndroidDeps, -): Promise { - await deps.enableDemoMode(device); +export async function screenshotAndroid(device: DeviceInfo, outPath: string): Promise { + await enableAndroidDemoMode(device); try { // Allow transient UI affordances like scrollbars to fade before capture. - await deps.settle(ANDROID_SCREENSHOT_SETTLE_DELAY_MS); - await deps.capture(device, outPath); + await sleep(ANDROID_SCREENSHOT_SETTLE_DELAY_MS); + await captureAndroidScreenshot(device, outPath); } finally { - await deps.disableDemoMode(device).catch(() => {}); + await disableAndroidDemoMode(device).catch(() => {}); } } diff --git a/src/platforms/ios/__tests__/index.test.ts b/src/platforms/ios/__tests__/index.test.ts index a09154d36..863d692fb 100644 --- a/src/platforms/ios/__tests__/index.test.ts +++ b/src/platforms/ios/__tests__/index.test.ts @@ -24,13 +24,11 @@ import { import { withMockedMacOsHelper } from './macos-helper-test-utils.ts'; import { quitMacOsApp, resolveMacOsHelperPackageRootFrom } from '../macos-helper.ts'; import { - captureSimulatorScreenshotWithFallback, prepareSimulatorStatusBarForScreenshot, resolveSimulatorRunnerScreenshotCandidatePaths, } from '../screenshot.ts'; import { focusIosSimulatorWindow } from '../simulator.ts'; import type { DeviceInfo } from '../../../utils/device.ts'; -import { withDiagnosticsScope } from '../../../utils/diagnostics.ts'; import { AppError } from '../../../utils/errors.ts'; const IOS_TEST_DEVICE: DeviceInfo = { @@ -219,152 +217,6 @@ test('shouldRetryIosSimulatorScreenshot ignores unrelated screenshot failures', assert.equal(shouldRetryIosSimulatorScreenshot(error), false); }); -test('captureSimulatorScreenshotWithFallback falls back to runner after retry exhaustion', async () => { - let ensureBootedCalls = 0; - let retryCalls = 0; - let runnerCalls = 0; - await captureSimulatorScreenshotWithFallback( - IOS_TEST_SIMULATOR, - '/tmp/out.png', - 'com.example.app', - { - ensureBooted: async () => { - ensureBootedCalls += 1; - }, - prepareStatusBarForScreenshot: async () => async () => {}, - captureWithRetry: async () => { - retryCalls += 1; - throw new AppError('COMMAND_FAILED', 'Detected file type from extension: PNG', { - stderr: 'Timeout waiting for screen surfaces', - exitCode: 60, - }); - }, - captureWithRunner: async () => { - runnerCalls += 1; - }, - shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, - }, - ); - assert.equal(ensureBootedCalls, 1); - assert.equal(retryCalls, 1); - assert.equal(runnerCalls, 1); -}); - -test('captureSimulatorScreenshotWithFallback falls back to runner after simctl screenshot timeout', async () => { - let runnerCalls = 0; - await captureSimulatorScreenshotWithFallback( - IOS_TEST_SIMULATOR, - '/tmp/out.png', - 'com.example.app', - { - ensureBooted: async () => {}, - prepareStatusBarForScreenshot: async () => async () => {}, - captureWithRetry: async () => { - throw new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', { - args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'], - timeoutMs: 20_000, - }); - }, - captureWithRunner: async () => { - runnerCalls += 1; - }, - shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, - }, - ); - assert.equal(runnerCalls, 1); -}); - -test('captureSimulatorScreenshotWithFallback continues when status bar preparation fails', async () => { - let retryCalls = 0; - await captureSimulatorScreenshotWithFallback( - IOS_TEST_SIMULATOR, - '/tmp/out.png', - 'com.example.app', - { - ensureBooted: async () => {}, - prepareStatusBarForScreenshot: async () => { - throw new AppError('COMMAND_FAILED', 'status_bar override failed'); - }, - captureWithRetry: async () => { - retryCalls += 1; - }, - captureWithRunner: async () => { - throw new Error('runner should not be used when capture succeeds'); - }, - shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, - }, - ); - assert.equal(retryCalls, 1); -}); - -test('captureSimulatorScreenshotWithFallback ignores status bar restore failures', async () => { - let retryCalls = 0; - await captureSimulatorScreenshotWithFallback( - IOS_TEST_SIMULATOR, - '/tmp/out.png', - 'com.example.app', - { - ensureBooted: async () => {}, - prepareStatusBarForScreenshot: async () => async () => { - throw new AppError('COMMAND_FAILED', 'status_bar clear failed'); - }, - captureWithRetry: async () => { - retryCalls += 1; - }, - captureWithRunner: async () => { - throw new Error('runner should not be used when capture succeeds'); - }, - shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, - }, - ); - assert.equal(retryCalls, 1); -}); - -test('captureSimulatorScreenshotWithFallback emits fallback diagnostic before using runner', async () => { - const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'agent-device-ios-screenshot-diag-test-')); - const logPath = path.join(tmpDir, 'diag.ndjson'); - try { - await withDiagnosticsScope( - { - debug: true, - logPath, - session: 'ios-test', - requestId: 'req-1', - command: 'screenshot', - }, - async () => { - await captureSimulatorScreenshotWithFallback( - IOS_TEST_SIMULATOR, - '/tmp/out.png', - 'com.example.app', - { - ensureBooted: async () => {}, - prepareStatusBarForScreenshot: async () => async () => {}, - captureWithRetry: async () => { - throw new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', { - args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'], - timeoutMs: 20_000, - }); - }, - captureWithRunner: async () => {}, - shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, - }, - ); - }, - ); - - const log = await waitForFileText(logPath); - assert.match(log, /"phase":"ios_screenshot_fallback"/); - assert.match(log, /"deviceId":"sim-1"/); - assert.match(log, /"errorCode":"COMMAND_FAILED"/); - assert.match(log, /"from":"simctl_screenshot"/); - assert.match(log, /"to":"runner"/); - assert.match(log, /"commandArgs":"simctl io sim-1 screenshot \/tmp\/out\.png"/); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } -}); - test('focusIosSimulatorWindow times out instead of hanging indefinitely', { timeout: 15_000 }, async () => { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'agent-device-ios-focus-timeout-test-')); const openPath = path.join(tmpDir, 'open'); @@ -466,19 +318,6 @@ exit 1 ); }); -async function waitForFileText(filePath: string, attempts = 20): Promise { - let lastError: unknown; - for (let attempt = 0; attempt < attempts; attempt += 1) { - try { - return await fs.readFile(filePath, 'utf8'); - } catch (error) { - lastError = error; - await new Promise((resolve) => setTimeout(resolve, 10)); - } - } - throw lastError; -} - test('resolveSimulatorRunnerScreenshotCandidatePaths includes tmp-based and basename fallbacks', () => { const containerPath = '/tmp/container'; const candidates = resolveSimulatorRunnerScreenshotCandidatePaths( diff --git a/src/platforms/ios/__tests__/runner-client.test.ts b/src/platforms/ios/__tests__/runner-client.test.ts index 325101540..2c7517d84 100644 --- a/src/platforms/ios/__tests__/runner-client.test.ts +++ b/src/platforms/ios/__tests__/runner-client.test.ts @@ -19,7 +19,6 @@ import { } from '../runner-client.ts'; import { isReadOnlyRunnerCommand } from '../runner-errors.ts'; import { - ensureXctestrun, resolveRunnerPerformanceBuildSettings, shouldDeleteRunnerDerivedRootEntry, xctestrunReferencesExistingProducts, @@ -455,134 +454,6 @@ test('xctestrunReferencesExistingProducts accepts xctestruns when referenced pro assert.equal(xctestrunReferencesExistingProducts(xctestrunPath), true); }); -test('ensureXctestrun rebuilds after cached macOS runner repair failure', async () => { - // Cached runner artifacts can look reusable until ad-hoc repair fails; ensure we clean once, - // rebuild, and return the repaired rebuilt xctestrun instead of looping on stale cache state. - const tmpDir = await makeTmpDir(); - const projectRoot = path.join(tmpDir, 'project'); - const derivedPath = path.join(tmpDir, 'derived'); - const projectPath = path.join( - projectRoot, - 'ios-runner', - 'AgentDeviceRunner', - 'AgentDeviceRunner.xcodeproj', - ); - await fs.promises.mkdir(path.dirname(projectPath), { recursive: true }); - fs.writeFileSync(projectPath, '', 'utf8'); - - const existingXctestrunPath = path.join(derivedPath, 'existing.xctestrun'); - const rebuiltXctestrunPath = path.join(derivedPath, 'rebuilt.xctestrun'); - await fs.promises.mkdir(derivedPath, { recursive: true }); - fs.writeFileSync(existingXctestrunPath, 'existing', 'utf8'); - - const previousDerivedPath = process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; - process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = derivedPath; - onTestFinished(() => { - if (previousDerivedPath === undefined) { - delete process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; - return; - } - process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = previousDerivedPath; - }); - - let currentXctestrunPath = existingXctestrunPath; - let cleanCalls = 0; - let buildCalls = 0; - const repairedPaths: string[] = []; - - const result = await ensureXctestrun( - macOsDevice, - {}, - { - findProjectRoot: () => projectRoot, - findXctestrun: () => currentXctestrunPath, - xctestrunReferencesProjectRoot: () => true, - resolveExistingXctestrunProductPaths: () => ['/tmp/runner.app'], - repairRunnerProductsIfNeeded: async (_device, _productPaths, xctestrunPath) => { - repairedPaths.push(xctestrunPath); - if (xctestrunPath === existingXctestrunPath) { - throw new AppError('COMMAND_FAILED', 'cached runner is damaged', { - reason: 'RUNNER_PRODUCT_REPAIR_FAILED', - }); - } - }, - assertSafeDerivedCleanup: (targetPath) => { - assert.equal(targetPath, derivedPath); - }, - cleanRunnerDerivedArtifacts: (targetPath) => { - assert.equal(targetPath, derivedPath); - cleanCalls += 1; - }, - buildRunnerXctestrun: async (_device, targetProjectPath, targetDerivedPath) => { - assert.equal(targetProjectPath, projectPath); - assert.equal(targetDerivedPath, derivedPath); - buildCalls += 1; - currentXctestrunPath = rebuiltXctestrunPath; - fs.writeFileSync(rebuiltXctestrunPath, 'rebuilt', 'utf8'); - }, - }, - ); - - assert.equal(result, rebuiltXctestrunPath); - assert.equal(cleanCalls, 1); - assert.equal(buildCalls, 1); - assert.deepEqual(repairedPaths, [existingXctestrunPath, rebuiltXctestrunPath]); -}); - -test('ensureXctestrun rethrows unexpected cached macOS runner repair errors', async () => { - const tmpDir = await makeTmpDir(); - const projectRoot = path.join(tmpDir, 'project'); - const derivedPath = path.join(tmpDir, 'derived'); - const projectPath = path.join( - projectRoot, - 'ios-runner', - 'AgentDeviceRunner', - 'AgentDeviceRunner.xcodeproj', - ); - await fs.promises.mkdir(path.dirname(projectPath), { recursive: true }); - fs.writeFileSync(projectPath, '', 'utf8'); - - const existingXctestrunPath = path.join(derivedPath, 'existing.xctestrun'); - await fs.promises.mkdir(derivedPath, { recursive: true }); - fs.writeFileSync(existingXctestrunPath, 'existing', 'utf8'); - - const previousDerivedPath = process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; - process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = derivedPath; - onTestFinished(() => { - if (previousDerivedPath === undefined) { - delete process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH; - return; - } - process.env.AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH = previousDerivedPath; - }); - - await assert.rejects( - ensureXctestrun( - macOsDevice, - {}, - { - findProjectRoot: () => projectRoot, - findXctestrun: () => existingXctestrunPath, - xctestrunReferencesProjectRoot: () => true, - resolveExistingXctestrunProductPaths: () => ['/tmp/runner.app'], - repairRunnerProductsIfNeeded: async () => { - throw new Error('permission denied'); - }, - assertSafeDerivedCleanup: () => { - throw new Error('should not clean derived data for unexpected repair errors'); - }, - cleanRunnerDerivedArtifacts: () => { - throw new Error('should not rebuild for unexpected repair errors'); - }, - buildRunnerXctestrun: async () => { - throw new Error('should not rebuild for unexpected repair errors'); - }, - }, - ), - /permission denied/, - ); -}); - test('shouldDeleteRunnerDerivedRootEntry only removes known xcode transient entries', () => { assert.equal(shouldDeleteRunnerDerivedRootEntry('Build'), true); assert.equal(shouldDeleteRunnerDerivedRootEntry('Logs'), true); diff --git a/src/platforms/ios/__tests__/runner-xctestrun-ensure.test.ts b/src/platforms/ios/__tests__/runner-xctestrun-ensure.test.ts new file mode 100644 index 000000000..5e3dfbd85 --- /dev/null +++ b/src/platforms/ios/__tests__/runner-xctestrun-ensure.test.ts @@ -0,0 +1,160 @@ +import { test, vi, onTestFinished, beforeEach } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import type { DeviceInfo } from '../../../utils/device.ts'; +import { AppError } from '../../../utils/errors.ts'; + +vi.mock('../runner-macos-products.ts', async (importOriginal) => { + const original = + await importOriginal(); + return { + ...original, + repairMacOsRunnerProductsIfNeeded: vi.fn(async () => {}), + }; +}); + +vi.mock('../runner-xctestrun-products.ts', async (importOriginal) => { + const original = + await importOriginal(); + return { + ...original, + resolveExistingXctestrunProductPaths: vi.fn(() => ['/tmp/runner.app']), + }; +}); + +vi.mock('../../../utils/exec.ts', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + runCmdStreaming: vi.fn(async () => {}), + }; +}); + +const { ensureXctestrun } = await import('../runner-xctestrun.ts'); +const { repairMacOsRunnerProductsIfNeeded } = await import('../runner-macos-products.ts'); +const { runCmdStreaming } = await import('../../../utils/exec.ts'); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +const macOsDevice: DeviceInfo = { + platform: 'macos', + id: 'host-macos-local', + name: 'Mac', + kind: 'device', + target: 'desktop', + booted: true, +}; + +async function makeTmpDir(): Promise { + const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'runner-xctestrun-test-')); + onTestFinished(async () => { + await fs.promises.rm(tmpDir, { recursive: true, force: true }); + }); + return tmpDir; +} + +function setEnvForTest(env: Record) { + const previous: Record = {}; + for (const [key, value] of Object.entries(env)) { + previous[key] = process.env[key]; + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + onTestFinished(() => { + for (const [key, value] of Object.entries(previous)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); +} + +test('ensureXctestrun rebuilds after cached macOS runner repair failure', async () => { + const tmpDir = await makeTmpDir(); + const projectRoot = path.join(tmpDir, 'project'); + const derivedPath = path.join(tmpDir, 'derived'); + const projectPath = path.join( + projectRoot, + 'ios-runner', + 'AgentDeviceRunner', + 'AgentDeviceRunner.xcodeproj', + ); + await fs.promises.mkdir(path.dirname(projectPath), { recursive: true }); + fs.writeFileSync(projectPath, '', 'utf8'); + + const existingXctestrunPath = path.join(derivedPath, 'existing.xctestrun'); + const rebuiltXctestrunPath = path.join(derivedPath, 'rebuilt.xctestrun'); + await fs.promises.mkdir(derivedPath, { recursive: true }); + // Write the project root into the file so xctestrunReferencesProjectRoot returns true + fs.writeFileSync(existingXctestrunPath, projectRoot, 'utf8'); + + setEnvForTest({ + AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH: derivedPath, + AGENT_DEVICE_IOS_ALLOW_OVERRIDE_DERIVED_CLEAN: '1', + AGENT_DEVICE_PROJECT_ROOT: projectRoot, + }); + + const mockedRepair = vi.mocked(repairMacOsRunnerProductsIfNeeded); + const repairedPaths: string[] = []; + mockedRepair.mockImplementation(async (_device, _productPaths, xctestrunPath) => { + repairedPaths.push(xctestrunPath); + if (xctestrunPath === existingXctestrunPath) { + throw new AppError('COMMAND_FAILED', 'cached runner is damaged', { + reason: 'RUNNER_PRODUCT_REPAIR_FAILED', + }); + } + }); + + // Simulate the build creating the rebuilt xctestrun file + vi.mocked(runCmdStreaming).mockImplementation(async () => { + fs.writeFileSync(rebuiltXctestrunPath, projectRoot, 'utf8'); + return undefined as never; + }); + + const result = await ensureXctestrun(macOsDevice, {}); + + assert.equal(result, rebuiltXctestrunPath); + assert.deepEqual(repairedPaths, [existingXctestrunPath, rebuiltXctestrunPath]); + // Verify build was called + assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); +}); + +test('ensureXctestrun rethrows unexpected cached macOS runner repair errors', async () => { + const tmpDir = await makeTmpDir(); + const projectRoot = path.join(tmpDir, 'project'); + const derivedPath = path.join(tmpDir, 'derived'); + const projectPath = path.join( + projectRoot, + 'ios-runner', + 'AgentDeviceRunner', + 'AgentDeviceRunner.xcodeproj', + ); + await fs.promises.mkdir(path.dirname(projectPath), { recursive: true }); + fs.writeFileSync(projectPath, '', 'utf8'); + + const existingXctestrunPath = path.join(derivedPath, 'existing.xctestrun'); + await fs.promises.mkdir(derivedPath, { recursive: true }); + fs.writeFileSync(existingXctestrunPath, projectRoot, 'utf8'); + + setEnvForTest({ + AGENT_DEVICE_IOS_RUNNER_DERIVED_PATH: derivedPath, + AGENT_DEVICE_PROJECT_ROOT: projectRoot, + }); + + vi.mocked(repairMacOsRunnerProductsIfNeeded).mockImplementation(async () => { + throw new Error('permission denied'); + }); + + await assert.rejects(ensureXctestrun(macOsDevice, {}), /permission denied/); + // Verify build was NOT called (error was rethrown before reaching build) + assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 0); +}); diff --git a/src/platforms/ios/runner-xctestrun.ts b/src/platforms/ios/runner-xctestrun.ts index e18fac1ef..7c3016626 100644 --- a/src/platforms/ios/runner-xctestrun.ts +++ b/src/platforms/ios/runner-xctestrun.ts @@ -61,56 +61,24 @@ export const IOS_RUNNER_CONTAINER_BUNDLE_IDS: string[] = resolveRunnerContainerB process.env, ); -type EnsureXctestrunDeps = { - findProjectRoot: () => string; - findXctestrun: (root: string, device?: DeviceInfo) => string | null; - xctestrunReferencesProjectRoot: (xctestrunPath: string, projectRoot: string) => boolean; - resolveExistingXctestrunProductPaths: (xctestrunPath: string) => string[] | null; - repairRunnerProductsIfNeeded: ( - device: DeviceInfo, - productPaths: string[], - xctestrunPath: string, - ) => Promise; - assertSafeDerivedCleanup: (derivedPath: string) => void; - cleanRunnerDerivedArtifacts: (derivedPath: string) => void; - buildRunnerXctestrun: ( - device: DeviceInfo, - projectPath: string, - derived: string, - options: { verbose?: boolean; logPath?: string; traceLogPath?: string }, - ) => Promise; -}; - -const defaultEnsureXctestrunDeps: EnsureXctestrunDeps = { - findProjectRoot, - findXctestrun, - xctestrunReferencesProjectRoot, - resolveExistingXctestrunProductPaths, - repairRunnerProductsIfNeeded: repairMacOsRunnerProductsIfNeeded, - assertSafeDerivedCleanup, - cleanRunnerDerivedArtifacts, - buildRunnerXctestrun, -}; - export async function ensureXctestrun( device: DeviceInfo, options: { verbose?: boolean; logPath?: string; traceLogPath?: string }, - deps: EnsureXctestrunDeps = defaultEnsureXctestrunDeps, ): Promise { const derived = resolveRunnerDerivedPath(device); - const projectRoot = deps.findProjectRoot(); + const projectRoot = findProjectRoot(); return await withKeyedLock(runnerXctestrunBuildLocks, derived, async () => { if (shouldCleanDerived()) { emitRunnerXctestrunDecision('clean', 'forced_clean', { derived }); - deps.assertSafeDerivedCleanup(derived); - deps.cleanRunnerDerivedArtifacts(derived); + assertSafeDerivedCleanup(derived); + cleanRunnerDerivedArtifacts(derived); } const existing = evaluateExistingXctestrun({ derived, projectRoot, - findXctestrun: (root) => deps.findXctestrun(root, device), - xctestrunReferencesProjectRoot: deps.xctestrunReferencesProjectRoot, - resolveExistingXctestrunProductPaths: deps.resolveExistingXctestrunProductPaths, + findXctestrun: (root) => findXctestrun(root, device), + xctestrunReferencesProjectRoot, + resolveExistingXctestrunProductPaths, }); if (existing.reason !== 'reuse_ready') { emitRunnerXctestrunDecision('rebuild', existing.reason, { @@ -120,7 +88,7 @@ export async function ensureXctestrun( } if (existing.reason === 'reuse_ready') { try { - await deps.repairRunnerProductsIfNeeded( + await repairMacOsRunnerProductsIfNeeded( device, existing.productPaths, existing.xctestrunPath, @@ -142,8 +110,8 @@ export async function ensureXctestrun( } } if (existing.xctestrunPath) { - deps.assertSafeDerivedCleanup(derived); - deps.cleanRunnerDerivedArtifacts(derived); + assertSafeDerivedCleanup(derived); + cleanRunnerDerivedArtifacts(derived); } const projectPath = path.join( projectRoot, @@ -156,19 +124,19 @@ export async function ensureXctestrun( throw new AppError('COMMAND_FAILED', 'iOS runner project not found', { projectPath }); } - await deps.buildRunnerXctestrun(device, projectPath, derived, options); + await buildRunnerXctestrun(device, projectPath, derived, options); - const built = deps.findXctestrun(derived, device); + const built = findXctestrun(derived, device); if (!built) { throw new AppError('COMMAND_FAILED', 'Failed to locate .xctestrun after build'); } - const builtProductPaths = deps.resolveExistingXctestrunProductPaths(built); + const builtProductPaths = resolveExistingXctestrunProductPaths(built); if (!builtProductPaths) { throw new AppError('COMMAND_FAILED', 'Runner build is missing expected products', { xctestrunPath: built, }); } - await deps.repairRunnerProductsIfNeeded(device, builtProductPaths, built); + await repairMacOsRunnerProductsIfNeeded(device, builtProductPaths, built); emitRunnerXctestrunDecision('build', 'built_new', { derived, xctestrunPath: built, @@ -340,7 +308,9 @@ export function xctestrunReferencesProjectRoot( } } -function findProjectRoot(): string { +function findProjectRoot(env: NodeJS.ProcessEnv = process.env): string { + const override = env.AGENT_DEVICE_PROJECT_ROOT?.trim(); + if (override) return path.resolve(override); const start = path.dirname(fileURLToPath(import.meta.url)); let current = start; for (let i = 0; i < 6; i += 1) { diff --git a/src/platforms/ios/screenshot.ts b/src/platforms/ios/screenshot.ts index 5abacb102..2347e0a58 100644 --- a/src/platforms/ios/screenshot.ts +++ b/src/platforms/ios/screenshot.ts @@ -27,21 +27,6 @@ function runSimctl(device: DeviceInfo, args: string[], options?: Parameters Promise; - prepareStatusBarForScreenshot: (device: DeviceInfo) => Promise<() => Promise>; - captureWithRetry: (device: DeviceInfo, outPath: string) => Promise; - captureWithRunner: (device: DeviceInfo, outPath: string, appBundleId?: string) => Promise; - shouldFallbackToRunner: (error: unknown) => boolean; -}; - -const defaultSimulatorScreenshotFlowDeps: SimulatorScreenshotFlowDeps = { - ensureBooted: ensureBootedSimulator, - prepareStatusBarForScreenshot: prepareSimulatorStatusBarForScreenshot, - captureWithRetry: captureSimulatorScreenshotWithRetry, - captureWithRunner: captureScreenshotViaRunner, - shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, -}; export async function screenshotIos( device: DeviceInfo, @@ -73,11 +58,10 @@ export async function screenshotIos( await captureScreenshotViaRunner(device, outPath, appBundleId); } -export async function captureSimulatorScreenshotWithFallback( +async function captureSimulatorScreenshotWithFallback( device: DeviceInfo, outPath: string, appBundleId?: string, - deps: SimulatorScreenshotFlowDeps = defaultSimulatorScreenshotFlowDeps, ): Promise { if (device.kind !== 'simulator') { throw new AppError( @@ -86,24 +70,24 @@ export async function captureSimulatorScreenshotWithFallback( ); } - await deps.ensureBooted(device); + await ensureBootedSimulator(device); let restoreStatusBar = async () => {}; try { - restoreStatusBar = await deps.prepareStatusBarForScreenshot(device); + restoreStatusBar = await prepareSimulatorStatusBarForScreenshot(device); } catch (error) { emitStatusBarDiagnostic(device, 'prepare_failed', error); } try { try { - await deps.captureWithRetry(device, outPath); + await captureSimulatorScreenshotWithRetry(device, outPath); return; } catch (error) { - if (!deps.shouldFallbackToRunner(error)) { + if (!shouldRetryIosSimulatorScreenshot(error)) { throw error; } emitScreenshotFallbackDiagnostic(device, 'simctl_screenshot', error); } - await deps.captureWithRunner(device, outPath, appBundleId); + await captureScreenshotViaRunner(device, outPath, appBundleId); } finally { await restoreStatusBar().catch((error) => emitStatusBarDiagnostic(device, 'restore_failed', error), diff --git a/src/utils/__tests__/interactors.test.ts b/src/utils/__tests__/interactors.test.ts index 902caa714..be7e37ecd 100644 --- a/src/utils/__tests__/interactors.test.ts +++ b/src/utils/__tests__/interactors.test.ts @@ -3,11 +3,23 @@ import assert from 'node:assert/strict'; import type { RunnerCommand } from '../../platforms/ios/runner-client.ts'; import type { DeviceInfo } from '../device.ts'; import { AppError } from '../errors.ts'; -import { - getInteractor, - resolveAppleBackRunnerCommand, - scrollIntoViewIosRunnerText, -} from '../interactors.ts'; + +const mockRunIosRunnerCommand = vi.fn< + (device: DeviceInfo, command: RunnerCommand, opts?: unknown) => Promise> +>(); + +vi.mock('../../platforms/ios/runner-client.ts', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + runIosRunnerCommand: (...args: Parameters) => + mockRunIosRunnerCommand(...args), + }; +}); + +const { getInteractor, resolveAppleBackRunnerCommand, scrollIntoViewIosRunnerText } = await import( + '../interactors.ts' +); const iosSimulator: DeviceInfo = { platform: 'ios', @@ -35,28 +47,23 @@ test('ios scrollIntoView uses snapshot progress checks between swipes', async () const commands: string[] = []; let findTextCalls = 0; let snapshotCalls = 0; - const interactor = getInteractor( - iosSimulator, - { appBundleId: 'com.example.app' }, - { - runIosRunnerCommand: async (_device, command) => { - commands.push(command.command); - if (command.command === 'findText') { - findTextCalls += 1; - return { found: findTextCalls > 2 }; - } - if (command.command === 'snapshot') { - snapshotCalls += 1; - return { - nodes: [{ type: 'XCUIElementTypeStaticText', label: `frame-${snapshotCalls}` }], - }; - } - if (command.command === 'swipe') return {}; - throw new Error(`Unexpected runner command: ${command.command}`); - }, - sleepMs: async () => {}, - }, - ); + mockRunIosRunnerCommand.mockImplementation(async (_device, command) => { + commands.push(command.command); + if (command.command === 'findText') { + findTextCalls += 1; + return { found: findTextCalls > 2 }; + } + if (command.command === 'snapshot') { + snapshotCalls += 1; + return { + nodes: [{ type: 'XCUIElementTypeStaticText', label: `frame-${snapshotCalls}` }], + }; + } + if (command.command === 'swipe') return {}; + throw new Error(`Unexpected runner command: ${command.command}`); + }); + + const interactor = getInteractor(iosSimulator, { appBundleId: 'com.example.app' }); const result = await interactor.scrollIntoView('Target'); assert.deepEqual(result, { attempts: 2 }); @@ -73,33 +80,29 @@ test('ios scrollIntoView uses snapshot progress checks between swipes', async () }); test('ios scroll reports planned pixels without recomputing from runner coordinates', async () => { - const interactor = getInteractor( - iosSimulator, - { appBundleId: 'com.example.app' }, - { - runIosRunnerCommand: async (_device, command) => { - if (command.command === 'interactionFrame') { - return { - x: 5, - y: 10, - referenceWidth: 300, - referenceHeight: 600, - }; - } - if (command.command === 'drag') { - return { - x: 155, - y: 420, - x2: 155, - y2: 301, - referenceWidth: 300, - referenceHeight: 600, - }; - } - throw new Error(`Unexpected runner command: ${command.command}`); - }, - }, - ); + mockRunIosRunnerCommand.mockImplementation(async (_device, command) => { + if (command.command === 'interactionFrame') { + return { + x: 5, + y: 10, + referenceWidth: 300, + referenceHeight: 600, + }; + } + if (command.command === 'drag') { + return { + x: 155, + y: 420, + x2: 155, + y2: 301, + referenceWidth: 300, + referenceHeight: 600, + }; + } + throw new Error(`Unexpected runner command: ${command.command}`); + }); + + const interactor = getInteractor(iosSimulator, { appBundleId: 'com.example.app' }); const result = await interactor.scroll('down', { pixels: 120 }); const pixels = @@ -109,16 +112,12 @@ test('ios scroll reports planned pixels without recomputing from runner coordina test('ios fill preserves target coordinates for the follow-up type command', async () => { const commands: RunnerCommand[] = []; - const interactor = getInteractor( - iosSimulator, - { appBundleId: 'com.example.app' }, - { - runIosRunnerCommand: async (_device, command) => { - commands.push(command); - return {}; - }, - }, - ); + mockRunIosRunnerCommand.mockImplementation(async (_device, command) => { + commands.push(command); + return {}; + }); + + const interactor = getInteractor(iosSimulator, { appBundleId: 'com.example.app' }); await interactor.fill(120, 240, 'hunter2'); diff --git a/src/utils/interactors.ts b/src/utils/interactors.ts index 5e67d52e7..edffaf0c6 100644 --- a/src/utils/interactors.ts +++ b/src/utils/interactors.ts @@ -95,11 +95,7 @@ type Interactor = { ): Promise | void>; }; -export function getInteractor( - device: DeviceInfo, - runnerContext: RunnerContext, - deps: InteractorDeps = {}, -): Interactor { +export function getInteractor(device: DeviceInfo, runnerContext: RunnerContext): Interactor { switch (device.platform) { case 'android': return { @@ -129,8 +125,7 @@ export function getInteractor( }; case 'ios': case 'macos': { - const runRunnerCommand = deps.runIosRunnerCommand ?? runIosRunnerCommand; - const { overrides, runnerOpts } = iosRunnerOverrides(device, runnerContext, deps); + const { overrides, runnerOpts } = iosRunnerOverrides(device, runnerContext); return { open: (app, options) => openIosApp(device, app, { appBundleId: options?.appBundleId, url: options?.url }), @@ -138,7 +133,7 @@ export function getInteractor( close: (app) => closeIosApp(device, app), screenshot: (outPath, appBundleId) => screenshotIos(device, outPath, appBundleId), back: async (mode) => { - await runRunnerCommand( + await runIosRunnerCommand( device, { command: resolveAppleBackRunnerCommand(mode), @@ -148,14 +143,14 @@ export function getInteractor( ); }, home: async () => { - await runRunnerCommand( + await runIosRunnerCommand( device, { command: 'home', appBundleId: runnerContext.appBundleId }, runnerOpts, ); }, appSwitcher: async () => { - await runRunnerCommand( + await runIosRunnerCommand( device, { command: 'appSwitcher', appBundleId: runnerContext.appBundleId }, runnerOpts, @@ -254,19 +249,10 @@ type IoRunnerOverrides = Pick< | 'scrollIntoView' >; -type InteractorDeps = { - runIosRunnerCommand?: RunIosRunnerCommand; - sleepMs?: (ms: number) => Promise; -}; - function iosRunnerOverrides( device: DeviceInfo, ctx: RunnerContext, - deps: InteractorDeps, ): { overrides: IoRunnerOverrides; runnerOpts: RunnerOpts } { - const runRunnerCommand = deps.runIosRunnerCommand ?? runIosRunnerCommand; - const sleepMs = - deps.sleepMs ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))); const runnerOpts = { verbose: ctx.verbose, logPath: ctx.logPath, @@ -282,14 +268,14 @@ function iosRunnerOverrides( runnerOpts, overrides: { tap: async (x, y) => { - return await runRunnerCommand( + return await runIosRunnerCommand( device, { command: 'tap', x, y, appBundleId: ctx.appBundleId }, runnerOpts, ); }, doubleTap: async (x, y) => { - return await runRunnerCommand( + return await runIosRunnerCommand( device, { command: 'tapSeries', @@ -304,40 +290,40 @@ function iosRunnerOverrides( ); }, swipe: async (x1, y1, x2, y2, durationMs) => { - return await runRunnerCommand( + return await runIosRunnerCommand( device, { command: 'drag', x: x1, y: y1, x2, y2, durationMs, appBundleId: ctx.appBundleId }, runnerOpts, ); }, longPress: async (x, y, durationMs) => { - return await runRunnerCommand( + return await runIosRunnerCommand( device, { command: 'longPress', x, y, durationMs, appBundleId: ctx.appBundleId }, runnerOpts, ); }, focus: async (x, y) => { - return await runRunnerCommand( + return await runIosRunnerCommand( device, { command: 'tap', x, y, appBundleId: ctx.appBundleId }, runnerOpts, ); }, type: async (text, delayMs) => { - await runRunnerCommand( + await runIosRunnerCommand( device, { command: 'type', text, delayMs, appBundleId: ctx.appBundleId }, runnerOpts, ); }, fill: async (x, y, text, delayMs) => { - const tapResult = await runRunnerCommand( + const tapResult = await runIosRunnerCommand( device, { command: 'tap', x, y, appBundleId: ctx.appBundleId }, runnerOpts, ); - await runRunnerCommand( + await runIosRunnerCommand( device, { command: 'type', @@ -353,12 +339,12 @@ function iosRunnerOverrides( return tapResult; }, scroll: async (direction, options) => { - return await runAppleScroll(runRunnerCommand, device, ctx, runnerOpts, direction, options); + return await runAppleScroll(runIosRunnerCommand, device, ctx, runnerOpts, direction, options); }, scrollIntoView: async (text, options) => { return await scrollIntoViewIosRunnerText( (command) => - runRunnerCommand(device, { ...command, appBundleId: ctx.appBundleId }, runnerOpts), + runIosRunnerCommand(device, { ...command, appBundleId: ctx.appBundleId }, runnerOpts), throwIfCanceled, text, options,