From c96d96e9187805e91944a659fb3c50270c3d1e80 Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Wed, 6 May 2026 20:06:14 +0100 Subject: [PATCH 1/4] feat(test): Expose xcresult bundle paths Include xcresult bundle paths in test result artifacts when xcodebuild reports or receives a result bundle path. This lets MCP clients open the result bundle directly from structured output and text renderers. Fixes #392 Co-Authored-By: Codex --- CHANGELOG.md | 2 + .../1.schema.json | 3 + .../device/__tests__/test_device.test.ts | 28 +++++ .../__tests__/swift_package_test.test.ts | 18 +++ .../__tests__/text-render-parity.test.ts | 55 ++++++++- .../__tests__/json-normalize.test.ts | 18 +++ src/types/domain-results.ts | 1 + .../simulator-test-execution.test.ts | 28 +++++ .../xcodebuild-domain-results.test.ts | 112 +++++++++++++++++- .../__tests__/xcodebuild-pipeline.test.ts | 47 +++++++- .../__tests__/xcresult-test-failures.test.ts | 30 ++++- .../__tests__/cli-text-renderer.test.ts | 90 +++++++++++++- src/utils/renderers/cli-text-renderer.ts | 13 +- src/utils/renderers/domain-result-text.ts | 24 ++-- src/utils/renderers/event-formatting.ts | 2 +- src/utils/simulator-test-execution.ts | 16 ++- src/utils/test-common.ts | 73 +++++++++--- src/utils/xcodebuild-domain-results.ts | 45 +++++-- src/utils/xcodebuild-event-parser.ts | 28 ++++- src/utils/xcresult-test-failures.ts | 47 ++++++++ 20 files changed, 622 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a08843c93..ad79ab3f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ - Fixed CLI test summaries showing false-positive compiler errors from xcodebuild NSError dump lines, and added compiler-error snapshot coverage for simulator, device, and macOS build-style flows ([#383](https://github.com/getsentry/XcodeBuildMCP/issues/383)). - Fixed simulator OSLog helper cleanup so server and daemon startup reconcile same-workspace orphaned log streams without stopping helpers owned by live sessions in other workspaces ([#382](https://github.com/getsentry/XcodeBuildMCP/issues/382)). - Fixed Weather example test discovery and made CLI test progress visible while tests are running instead of leaving the last build phase displayed. +- Exposed xcresult bundle paths in test result structured output and text output when xcodebuild reports or is given a result bundle path, so agents can inspect test artifacts after simulator, device, and macOS test runs ([#392](https://github.com/getsentry/XcodeBuildMCP/issues/392)). +- Fixed final test summaries to use xcresult top-level test declaration counts when available, avoiding overcounting dynamic-parameter test runs ([#392](https://github.com/getsentry/XcodeBuildMCP/issues/392)). ### Changed diff --git a/schemas/structured-output/xcodebuildmcp.output.test-result/1.schema.json b/schemas/structured-output/xcodebuildmcp.output.test-result/1.schema.json index 2c187d173..e7516f2bd 100644 --- a/schemas/structured-output/xcodebuildmcp.output.test-result/1.schema.json +++ b/schemas/structured-output/xcodebuildmcp.output.test-result/1.schema.json @@ -73,6 +73,9 @@ }, "packagePath": { "type": "string" + }, + "xcresultPath": { + "type": "string" } }, "required": [], diff --git a/src/mcp/tools/device/__tests__/test_device.test.ts b/src/mcp/tools/device/__tests__/test_device.test.ts index ca2256dd1..82bc702e3 100644 --- a/src/mcp/tools/device/__tests__/test_device.test.ts +++ b/src/mcp/tools/device/__tests__/test_device.test.ts @@ -292,6 +292,34 @@ describe('test_device plugin', () => { expect(result.isError()).toBeFalsy(); }); + it('should expose user-provided result bundle paths in test output', async () => { + const mockExecutor = createMockExecutor({ + success: true, + output: 'Test Succeeded', + }); + + const { result } = await runTestDeviceLogic( + { + projectPath: '/path/to/project.xcodeproj', + scheme: 'MyScheme', + deviceId: 'test-device-123', + configuration: 'Debug', + extraArgs: [ + '-resultBundlePath', + '/tmp/Stale Device Tests.xcresult', + '-resultBundlePath=/tmp/Device Tests.xcresult', + ], + preferXcodebuild: false, + platform: 'iOS', + }, + mockExecutor, + mockFs(), + ); + + expectPendingBuildResponse(result); + expect(result.text()).toContain('Result Bundle: /tmp/Device Tests.xcresult'); + }); + it('should handle workspace testing successfully', async () => { const mockExecutor = createMockExecutor({ success: true, diff --git a/src/mcp/tools/swift-package/__tests__/swift_package_test.test.ts b/src/mcp/tools/swift-package/__tests__/swift_package_test.test.ts index e2620daac..11082a2d2 100644 --- a/src/mcp/tools/swift-package/__tests__/swift_package_test.test.ts +++ b/src/mcp/tools/swift-package/__tests__/swift_package_test.test.ts @@ -147,6 +147,24 @@ describe('swift_package_test plugin', () => { expect(result.isError()).toBeFalsy(); }); + it('should not expose xcresult paths from SwiftPM test output', async () => { + const mockExecutor: CommandExecutor = async (_args, _name, _hideOutput, opts) => { + opts?.onStdout?.('Result bundle written to: /tmp/SwiftPM.xcresult\n'); + return createMockCommandResponse({ + success: true, + output: 'All tests passed.', + }); + }; + + const { result } = await runSwiftPackageTestLogic( + { packagePath: '/test/package' }, + mockExecutor, + ); + + expect(result.isError()).toBeFalsy(); + expect(result.text()).not.toContain('Result Bundle: /tmp/SwiftPM.xcresult'); + }); + it('should return error response for test failure', async () => { const mockExecutor = createMockExecutor({ success: false, diff --git a/src/rendering/__tests__/text-render-parity.test.ts b/src/rendering/__tests__/text-render-parity.test.ts index fbe8dc7aa..aec051a03 100644 --- a/src/rendering/__tests__/text-render-parity.test.ts +++ b/src/rendering/__tests__/text-render-parity.test.ts @@ -249,7 +249,10 @@ describe('text render parity', () => { durationMs: 2200, counts: { passed: 1, failed: 1, skipped: 0 }, }, - artifacts: { buildLogPath: '/tmp/Test.log' }, + artifacts: { + buildLogPath: '/tmp/Test.log', + xcresultPath: '/tmp/App Tests.xcresult', + }, diagnostics: { warnings: [], errors: [], @@ -284,9 +287,59 @@ describe('text render parity', () => { expect(output.match(/Discovered 2 test\(s\):/g)).toHaveLength(1); expect(output.match(/MCPTestTests\n ✗ testTwo\(\):/g)).toHaveLength(1); expect(output.match(/1 test failed, 1 passed, 0 skipped/g)).toHaveLength(1); + expect(output).toContain('Result Bundle: /tmp/App Tests.xcresult'); expect(output).toContain('Build Logs: /tmp/Test.log'); }); + it('matches cli text and uses structured build summary when streamed build-summary disagrees', () => { + const fixture: TranscriptFixture = { + progressEvents: [ + { + kind: 'build-result', + fragment: 'invocation', + operation: 'BUILD', + request: { + scheme: 'MyApp', + projectPath: '/tmp/MyApp.xcodeproj', + configuration: 'Debug', + platform: 'iOS Simulator', + }, + }, + { + kind: 'build-result', + fragment: 'build-summary', + operation: 'BUILD', + status: 'FAILED', + durationMs: 9900, + }, + ], + structuredOutput: { + schema: 'xcodebuildmcp.output.build-result', + schemaVersion: '1.0.0', + result: { + kind: 'build-result', + didError: false, + error: null, + summary: { status: 'SUCCEEDED', durationMs: 3200 }, + artifacts: { scheme: 'MyApp', buildLogPath: '/tmp/build.log' }, + diagnostics: { warnings: [], errors: [] }, + }, + }, + }; + + const rendered = renderTranscript( + { + items: fixture.progressEvents, + structuredOutput: fixture.structuredOutput, + }, + 'text', + ); + + expect(rendered).toBe(captureCliText(fixture)); + expect(rendered).toContain('✅ Build succeeded. (⏱️ 3.2s)'); + expect(rendered).not.toContain('❌ Build failed. (⏱️ 9.9s)'); + }); + it('renders next steps in MCP tool-call syntax for MCP runtime text transcripts', () => { const fixture: TranscriptFixture = { progressEvents: [], diff --git a/src/snapshot-tests/__tests__/json-normalize.test.ts b/src/snapshot-tests/__tests__/json-normalize.test.ts index f49968ca8..4c5e9dc55 100644 --- a/src/snapshot-tests/__tests__/json-normalize.test.ts +++ b/src/snapshot-tests/__tests__/json-normalize.test.ts @@ -35,6 +35,24 @@ describe('normalizeStructuredEnvelope', () => { }); }); + it('preserves xcresult paths in test result artifacts', () => { + const envelope: StructuredOutputEnvelope = { + schema: 'xcodebuildmcp.output.test-result', + schemaVersion: '1', + didError: false, + error: null, + data: { + summary: { target: 'simulator' }, + artifacts: { + buildLogPath: '/tmp/build.log', + xcresultPath: '/tmp/App Tests.xcresult', + }, + }, + }; + + expect(normalizeStructuredEnvelope(envelope)).toEqual(envelope); + }); + it('keeps suite-less passed test cases for non-simulator results', () => { const envelope: StructuredOutputEnvelope = { schema: 'xcodebuildmcp.output.test-result', diff --git a/src/types/domain-results.ts b/src/types/domain-results.ts index 5f440931c..236b7ca6f 100644 --- a/src/types/domain-results.ts +++ b/src/types/domain-results.ts @@ -168,6 +168,7 @@ export type TestResultArtifacts = AtLeastOne<{ deviceId: string; buildLogPath: string; packagePath: string; + xcresultPath: string; }>; export interface CoverageSummary extends StatusSummary { coveragePct?: number; diff --git a/src/utils/__tests__/simulator-test-execution.test.ts b/src/utils/__tests__/simulator-test-execution.test.ts index 6c39fd15f..f3170a499 100644 --- a/src/utils/__tests__/simulator-test-execution.test.ts +++ b/src/utils/__tests__/simulator-test-execution.test.ts @@ -62,6 +62,7 @@ describe('createSimulatorTwoPhaseExecutionPlan', () => { '/tmp/Calculator.xcresult', ]); expect(plan.usesExactSelectors).toBe(false); + expect(plan.resultBundlePath).toBe('/tmp/Calculator.xcresult'); }); it('preserves user-supplied selector arguments in both simulator test phases', () => { @@ -90,5 +91,32 @@ describe('createSimulatorTwoPhaseExecutionPlan', () => { expect(plan.buildArgs).toEqual([]); expect(plan.testArgs).toEqual(['-resultBundlePath', '/tmp/UserProvided.xcresult']); + expect(plan.resultBundlePath).toBe('/tmp/UserProvided.xcresult'); + }); + + it('supports equals-form resultBundlePath arguments', () => { + const plan = createSimulatorTwoPhaseExecutionPlan({ + extraArgs: ['-resultBundlePath=/tmp/EqualsProvided.xcresult'], + }); + + expect(plan.buildArgs).toEqual([]); + expect(plan.testArgs).toEqual(['-resultBundlePath', '/tmp/EqualsProvided.xcresult']); + expect(plan.resultBundlePath).toBe('/tmp/EqualsProvided.xcresult'); + }); + + it('uses the last valid resultBundlePath argument', () => { + const plan = createSimulatorTwoPhaseExecutionPlan({ + extraArgs: [ + '-resultBundlePath', + '-quiet', + '-resultBundlePath', + '/tmp/First.xcresult', + '-resultBundlePath=/tmp/Last.xcresult', + ], + }); + + expect(plan.buildArgs).toEqual(['-quiet']); + expect(plan.testArgs).toEqual(['-quiet', '-resultBundlePath', '/tmp/Last.xcresult']); + expect(plan.resultBundlePath).toBe('/tmp/Last.xcresult'); }); }); diff --git a/src/utils/__tests__/xcodebuild-domain-results.test.ts b/src/utils/__tests__/xcodebuild-domain-results.test.ts index 66a7d47c1..e1dd9bed4 100644 --- a/src/utils/__tests__/xcodebuild-domain-results.test.ts +++ b/src/utils/__tests__/xcodebuild-domain-results.test.ts @@ -1,9 +1,21 @@ -import { describe, expect, it } from 'vitest'; -import { createBuildDomainResult } from '../xcodebuild-domain-results.ts'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + extractTestSummaryCountsFromXcresult: vi.fn(), +})); + +vi.mock('../xcresult-test-failures.ts', () => ({ + extractTestSummaryCountsFromXcresult: mocks.extractTestSummaryCountsFromXcresult, +})); + +import { createBuildDomainResult, createTestDomainResult } from '../xcodebuild-domain-results.ts'; import { createXcodebuildRunState, type XcodebuildRunState } from '../xcodebuild-run-state.ts'; import type { StartedPipeline, XcodebuildPipeline } from '../xcodebuild-pipeline.ts'; -function createStartedPipelineWithState(state: XcodebuildRunState): StartedPipeline { +function createStartedPipelineWithState( + state: XcodebuildRunState, + xcresultPath: string | null = null, +): StartedPipeline { const pipeline: XcodebuildPipeline = { onStdout(): void {}, onStderr(): void {}, @@ -14,7 +26,7 @@ function createStartedPipelineWithState(state: XcodebuildRunState): StartedPipel highestStageRank() { return 0; }, - xcresultPath: null, + xcresultPath, logPath: '/tmp/build.log', }; @@ -22,6 +34,98 @@ function createStartedPipelineWithState(state: XcodebuildRunState): StartedPipel } describe('xcodebuild-domain-results', () => { + beforeEach(() => { + mocks.extractTestSummaryCountsFromXcresult.mockReturnValue(null); + }); + + it('includes detected xcresult paths in test result artifacts', () => { + const runState = createXcodebuildRunState({ operation: 'TEST' }); + + const result = createTestDomainResult({ + started: createStartedPipelineWithState( + runState.finalize(true, 1000), + '/tmp/App Tests.xcresult', + ), + succeeded: true, + target: 'simulator', + artifacts: { buildLogPath: '/tmp/build.log' }, + request: { scheme: 'App' }, + }); + + expect(result.artifacts).toMatchObject({ + buildLogPath: '/tmp/build.log', + xcresultPath: '/tmp/App Tests.xcresult', + }); + }); + + it('does not copy parser-detected xcresult paths into SwiftPM test results', () => { + const runState = createXcodebuildRunState({ operation: 'TEST' }); + + const result = createTestDomainResult({ + started: createStartedPipelineWithState( + runState.finalize(true, 1000), + '/tmp/NotFromSwiftPM.xcresult', + ), + succeeded: true, + target: 'swift-package', + artifacts: { buildLogPath: '/tmp/build.log' }, + request: { target: 'swift-package', packagePath: '/tmp/Package' }, + }); + + expect(result.artifacts).toEqual({ buildLogPath: '/tmp/build.log' }); + }); + + it('preserves provided xcresult paths when the pipeline does not detect one', () => { + const runState = createXcodebuildRunState({ operation: 'TEST' }); + + const result = createTestDomainResult({ + started: createStartedPipelineWithState(runState.finalize(true, 1000)), + succeeded: true, + target: 'macos', + artifacts: { + buildLogPath: '/tmp/build.log', + xcresultPath: '/tmp/User Provided.xcresult', + }, + request: { scheme: 'App' }, + }); + + expect(result.artifacts.xcresultPath).toBe('/tmp/User Provided.xcresult'); + }); + + it('uses xcresult top-level declaration counts instead of streamed run counts', () => { + mocks.extractTestSummaryCountsFromXcresult.mockReturnValue({ + passed: 16, + failed: 0, + skipped: 0, + }); + + const runState = createXcodebuildRunState({ operation: 'TEST' }); + runState.push({ + kind: 'test-result', + fragment: 'test-progress', + operation: 'TEST', + completed: 19, + failed: 0, + skipped: 0, + }); + + const result = createTestDomainResult({ + started: createStartedPipelineWithState( + runState.finalize(true, 1000), + '/tmp/Weather.xcresult', + ), + succeeded: true, + target: 'simulator', + artifacts: { buildLogPath: '/tmp/build.log' }, + request: { scheme: 'Weather' }, + }); + + expect(mocks.extractTestSummaryCountsFromXcresult).toHaveBeenCalledWith( + '/tmp/Weather.xcresult', + ); + expect(result.summary.counts).toEqual({ passed: 16, failed: 0, skipped: 0 }); + }); + it('does not duplicate fallback lines represented by multi-line parsed errors', () => { const runState = createXcodebuildRunState({ operation: 'BUILD' }); runState.push({ diff --git a/src/utils/__tests__/xcodebuild-pipeline.test.ts b/src/utils/__tests__/xcodebuild-pipeline.test.ts index af9632a85..c4bc290f1 100644 --- a/src/utils/__tests__/xcodebuild-pipeline.test.ts +++ b/src/utils/__tests__/xcodebuild-pipeline.test.ts @@ -91,6 +91,51 @@ describe('xcodebuild-pipeline', () => { expect(text.match(/1 test failed, 1 passed, 0 skipped/g)).toHaveLength(1); }); + it('detects xcresult paths from xcodebuild result bundle output', () => { + const pipeline = createXcodebuildPipeline({ + operation: 'TEST', + toolName: 'test_sim', + params: { scheme: 'MyApp' }, + emit: () => {}, + }); + + pipeline.onStderr( + '2026-05-06 10:00:00.000 xcodebuild[123:456] Writing error result bundle to /tmp/My App Tests.xcresult\n', + ); + + expect(pipeline.xcresultPath).toBe('/tmp/My App Tests.xcresult'); + }); + + it('detects result bundle written messages and standalone xcresult paths', () => { + const pipeline = createXcodebuildPipeline({ + operation: 'TEST', + toolName: 'test_sim', + params: { scheme: 'MyApp' }, + emit: () => {}, + }); + + pipeline.onStdout('Result bundle written to: /tmp/First Tests.xcresult\n'); + expect(pipeline.xcresultPath).toBe('/tmp/First Tests.xcresult'); + + pipeline.onStdout('/tmp/Second Tests.xcresult\n'); + expect(pipeline.xcresultPath).toBe('/tmp/Second Tests.xcresult'); + }); + + it('does not treat xcodebuild command invocations as standalone xcresult paths', () => { + const pipeline = createXcodebuildPipeline({ + operation: 'TEST', + toolName: 'test_sim', + params: { scheme: 'MyApp' }, + emit: () => {}, + }); + + pipeline.onStdout( + '/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -scheme MyApp -resultBundlePath /tmp/MyApp.xcresult\n', + ); + + expect(pipeline.xcresultPath).toBeNull(); + }); + it('handles build output with warnings and errors', () => { const emittedEvents: AnyFragment[] = []; const pipeline = createXcodebuildPipeline({ @@ -217,7 +262,7 @@ describe('xcodebuild-pipeline', () => { }, }); expect(text).toContain('Discovered 3 test(s):'); - expect(text).toContain('✅ Test succeeded.'); + expect(text).toContain('✅ 3 tests passed, 0 failed, 0 skipped'); }); it('renders test discovery in cli-text mode', () => { diff --git a/src/utils/__tests__/xcresult-test-failures.test.ts b/src/utils/__tests__/xcresult-test-failures.test.ts index ef62ad6bd..c21c5673b 100644 --- a/src/utils/__tests__/xcresult-test-failures.test.ts +++ b/src/utils/__tests__/xcresult-test-failures.test.ts @@ -1,5 +1,33 @@ import { describe, expect, it } from 'vitest'; -import { parseXcresultFailureMessage } from '../xcresult-test-failures.ts'; +import { + parseXcresultFailureMessage, + parseXcresultTestSummaryCounts, +} from '../xcresult-test-failures.ts'; + +describe('parseXcresultTestSummaryCounts', () => { + it('uses top-level declaration counts instead of device run counts', () => { + const summary = JSON.stringify({ + totalTestCount: 16, + passedTests: 16, + failedTests: 0, + skippedTests: 0, + devicesAndConfigurations: [ + { + totalTestCount: 19, + passedTests: 19, + failedTests: 0, + skippedTests: 0, + }, + ], + }); + + expect(parseXcresultTestSummaryCounts(summary)).toEqual({ + passed: 16, + failed: 0, + skipped: 0, + }); + }); +}); describe('parseXcresultFailureMessage', () => { it('preserves locations from multi-line Swift Testing failure messages', () => { diff --git a/src/utils/renderers/__tests__/cli-text-renderer.test.ts b/src/utils/renderers/__tests__/cli-text-renderer.test.ts index 2a645b0b3..d0dd647cb 100644 --- a/src/utils/renderers/__tests__/cli-text-renderer.test.ts +++ b/src/utils/renderers/__tests__/cli-text-renderer.test.ts @@ -275,7 +275,7 @@ describe('cli-text-renderer', () => { ], }); - expect(output).toContain('✅ 1 test passed, 0 skipped'); + expect(output).toContain('✅ 1 test passed, 0 failed, 0 skipped'); expect(output).not.toContain('Compiler Errors (1):'); expect(output).not.toContain('SimCallingSelector=launchApplicationWithID:options:pid:error:,'); }); @@ -714,10 +714,96 @@ describe('cli-text-renderer', () => { expect(output).toContain('🧪 Test'); expect(output).toContain('Scheme: MyApp'); - expect(output).toContain('5 tests passed, 1 skipped'); + expect(output).toContain('5 tests passed, 0 failed, 1 skipped'); expect(output).toContain('Build Logs: /tmp/test.log'); }); + it('uses finalized test-result counts instead of the streamed build-summary counts', () => { + const output = renderCliTextTranscript({ + items: [ + { + kind: 'test-result', + fragment: 'test-progress', + operation: 'TEST', + completed: 19, + failed: 0, + skipped: 0, + }, + { + kind: 'test-result', + fragment: 'build-summary', + operation: 'TEST', + status: 'SUCCEEDED', + totalTests: 19, + passedTests: 19, + failedTests: 0, + skippedTests: 0, + durationMs: 2100, + }, + ], + structuredOutput: { + schema: 'xcodebuildmcp.output.test-result', + schemaVersion: '1.0.0', + result: { + kind: 'test-result', + didError: false, + error: null, + summary: { + status: 'SUCCEEDED', + durationMs: 2100, + counts: { passed: 16, failed: 0, skipped: 0 }, + }, + artifacts: { + xcresultPath: '/tmp/Weather.xcresult', + buildLogPath: '/tmp/weather-test.log', + }, + diagnostics: { warnings: [], errors: [], testFailures: [] }, + }, + }, + }); + + expect(output).toContain('Running tests (19 completed, 0 failures, 0 skipped)'); + expect(output.match(/✅ 16 tests passed, 0 failed, 0 skipped/g)).toHaveLength(1); + expect(output).not.toContain('✅ 19 tests passed, 0 failed, 0 skipped'); + expect(output).toContain('Result Bundle: /tmp/Weather.xcresult'); + expect(output).toContain('Build Logs: /tmp/weather-test.log'); + }); + + it('uses finalized build summary from structured output when streamed build-summary disagrees', () => { + const output = renderCliTextTranscript({ + items: [ + { + kind: 'build-result', + fragment: 'invocation', + operation: 'BUILD', + request: { + scheme: 'MyApp', + projectPath: '/tmp/MyApp.xcodeproj', + configuration: 'Debug', + platform: 'iOS Simulator', + }, + }, + { + kind: 'build-result', + fragment: 'build-summary', + operation: 'BUILD', + status: 'FAILED', + durationMs: 9900, + }, + ], + structuredOutput: buildOutput({ + didError: false, + error: null, + summary: { status: 'SUCCEEDED', durationMs: 3200 }, + artifacts: { scheme: 'MyApp', buildLogPath: '/tmp/build.log' }, + }), + }); + + expect(output).toContain('✅ Build succeeded. (⏱️ 3.2s)'); + expect(output).not.toContain('❌ Build failed. (⏱️ 9.9s)'); + expect(output).toContain('Build Logs: /tmp/build.log'); + }); + it('omits per-test results by default and renders them when showTestTiming is true', () => { const fragments = [ { diff --git a/src/utils/renderers/cli-text-renderer.ts b/src/utils/renderers/cli-text-renderer.ts index d4c91cd57..96a3c44ee 100644 --- a/src/utils/renderers/cli-text-renderer.ts +++ b/src/utils/renderers/cli-text-renderer.ts @@ -15,7 +15,7 @@ import { createCliProgressReporter } from '../cli-progress-reporter.ts'; import { formatCliTextLine } from '../terminal-output.ts'; import { createNextStepsBlock, - createStreamingTailItems, + createStreamingFinalItems, renderDomainResultTextItems, type SummaryTextBlock, type TextRenderableItem, @@ -116,6 +116,7 @@ function createCliTextProcessor(options: CliTextProcessorOptions): TranscriptRen let nextStepsRuntime: 'cli' | 'daemon' | 'mcp' | undefined; let sawProgressNextSteps = false; let lastRenderedTestProgressKey: string | null = null; + let pendingStreamedSummary: SummaryTextBlock | null = null; function writeDurable(text: string): void { sink.clearTransient(); @@ -396,6 +397,8 @@ function createCliTextProcessor(options: CliTextProcessorOptions): TranscriptRen sawIncomingNonHeaderEvent = true; if (item.type === 'summary') { sawIncomingSummaryEvent = true; + pendingStreamedSummary = item as SummaryTextBlock; + return; } else { sawIncomingNonSummaryEvent = true; } @@ -434,18 +437,19 @@ function createCliTextProcessor(options: CliTextProcessorOptions): TranscriptRen ); const replayItems = structuredItems.filter((item) => { if (sawIncomingHeaderEvent && item.type === 'header') return false; - if (sawIncomingSummaryEvent && item.type === 'summary') return false; return true; }); for (const item of replayItems) { processItem(item); } } else { - const tailItems = createStreamingTailItems(structuredOutput.result); - for (const item of tailItems) { + const finalItems = createStreamingFinalItems(structuredOutput.result); + for (const item of finalItems) { processItem(item); } } + } else if (pendingStreamedSummary) { + processItem(pendingStreamedSummary); } flushGroupedDiagnostics(lastSummaryStatus !== 'SUCCEEDED'); groupedCompilerErrors.length = 0; @@ -473,6 +477,7 @@ function createCliTextProcessor(options: CliTextProcessorOptions): TranscriptRen sawProgressNextSteps = false; collectedTestCaseResults.length = 0; lastRenderedTestProgressKey = null; + pendingStreamedSummary = null; }, }; } diff --git a/src/utils/renderers/domain-result-text.ts b/src/utils/renderers/domain-result-text.ts index e0e2a9d9e..10c6f1fb9 100644 --- a/src/utils/renderers/domain-result-text.ts +++ b/src/utils/renderers/domain-result-text.ts @@ -1960,6 +1960,9 @@ export function createBuildLikeTailItems(result: ToolDomainResult): TextRenderab case 'test-result': { if (!('artifacts' in result) || !result.artifacts) return []; const items: DetailTreeTextBlock['items'] = []; + if ('xcresultPath' in result.artifacts && typeof result.artifacts.xcresultPath === 'string') { + items.push({ label: 'Result Bundle', value: displayPath(result.artifacts.xcresultPath) }); + } if ('buildLogPath' in result.artifacts && typeof result.artifacts.buildLogPath === 'string') { items.push({ label: 'Build Logs', value: displayPath(result.artifacts.buildLogPath) }); } @@ -1970,14 +1973,15 @@ export function createBuildLikeTailItems(result: ToolDomainResult): TextRenderab } } -export function createStreamingTailItems(result: ToolDomainResult): TextRenderableItem[] { - const items = createBuildLikeTailItems(result); - - if (!('diagnostics' in result) || !result.diagnostics) { - return items; - } +export function createStreamingFinalItems(result: ToolDomainResult): TextRenderableItem[] { + const items: TextRenderableItem[] = []; - if ('rawOutput' in result.diagnostics && Array.isArray(result.diagnostics.rawOutput)) { + if ( + 'diagnostics' in result && + result.diagnostics && + 'rawOutput' in result.diagnostics && + Array.isArray(result.diagnostics.rawOutput) + ) { items.push( ...createStandardDiagnosticSections({ warnings: [], @@ -1987,6 +1991,12 @@ export function createStreamingTailItems(result: ToolDomainResult): TextRenderab ); } + const summary = createSummaryBlock(result); + if (summary) { + items.push(summary); + } + + items.push(...createBuildLikeTailItems(result)); return items; } diff --git a/src/utils/renderers/event-formatting.ts b/src/utils/renderers/event-formatting.ts index 3c4e3b48d..41403c90f 100644 --- a/src/utils/renderers/event-formatting.ts +++ b/src/utils/renderers/event-formatting.ts @@ -511,7 +511,7 @@ export function formatSummaryEvent(event: SummaryTextBlock): string { const skipped = event.skippedTests ?? 0; if (succeeded) { - return `${statusEmoji} ${pluralize(passed, 'test', 'tests')} passed, ${skipped} skipped${durationPart}`; + return `${statusEmoji} ${pluralize(passed, 'test', 'tests')} passed, ${failed} failed, ${skipped} skipped${durationPart}`; } return `${statusEmoji} ${pluralize(failed, 'test', 'tests')} failed, ${passed} passed, ${skipped} skipped${durationPart}`; diff --git a/src/utils/simulator-test-execution.ts b/src/utils/simulator-test-execution.ts index c23e1bf30..057c34cc6 100644 --- a/src/utils/simulator-test-execution.ts +++ b/src/utils/simulator-test-execution.ts @@ -1,5 +1,9 @@ import type { TestPreflightResult } from './test-preflight.ts'; +function isResultBundlePathValue(value: string | undefined): value is string { + return value !== undefined && value.length > 0 && !value.startsWith('-'); +} + function parseTestSelectorArgs(extraArgs: string[] | undefined): { remainingArgs: string[]; selectorArgs: string[]; @@ -27,13 +31,21 @@ function parseTestSelectorArgs(extraArgs: string[] | undefined): { if (argument === '-resultBundlePath') { const value = extraArgs[index + 1]; - if (value) { + if (isResultBundlePathValue(value)) { resultBundlePath = value; index += 1; } continue; } + if (argument.startsWith('-resultBundlePath=')) { + const value = argument.slice('-resultBundlePath='.length); + if (isResultBundlePathValue(value)) { + resultBundlePath = value; + } + continue; + } + if (argument.startsWith('-only-testing:') || argument.startsWith('-skip-testing:')) { selectorArgs.push(argument); continue; @@ -53,6 +65,7 @@ export function createSimulatorTwoPhaseExecutionPlan(params: { buildArgs: string[]; testArgs: string[]; usesExactSelectors: boolean; + resultBundlePath?: string; } { const parsedArgs = parseTestSelectorArgs(params.extraArgs); const selectedTestArgs = parsedArgs.selectorArgs; @@ -67,5 +80,6 @@ export function createSimulatorTwoPhaseExecutionPlan(params: { ...(resultBundlePath ? ['-resultBundlePath', resultBundlePath] : []), ], usesExactSelectors, + ...(resultBundlePath ? { resultBundlePath } : {}), }; } diff --git a/src/utils/test-common.ts b/src/utils/test-common.ts index e22fe6b74..0f822aab8 100644 --- a/src/utils/test-common.ts +++ b/src/utils/test-common.ts @@ -17,7 +17,11 @@ import { type TestPreflightResult } from './test-preflight.ts'; import { createSimulatorTwoPhaseExecutionPlan } from './simulator-test-execution.ts'; -import type { BuildTarget, TestResultDomainResult } from '../types/domain-results.ts'; +import type { + BuildTarget, + TestResultArtifacts, + TestResultDomainResult, +} from '../types/domain-results.ts'; import type { BuildInvocationRequest } from '../types/domain-fragments.ts'; import type { StreamingExecutor } from '../types/tool-execution.ts'; import { @@ -55,6 +59,49 @@ function getFallbackErrorMessages( return [...streamedLines, ...(responseContent ?? []).map((item) => item.text)]; } +function isResultBundlePathValue(value: string | undefined): value is string { + return value !== undefined && value.length > 0 && !value.startsWith('-'); +} + +function findResultBundlePathArg(extraArgs?: readonly string[]): string | undefined { + if (!extraArgs) { + return undefined; + } + + let resultBundlePath: string | undefined; + for (let index = 0; index < extraArgs.length; index += 1) { + const argument = extraArgs[index]; + if (argument === '-resultBundlePath') { + const value = extraArgs[index + 1]; + if (isResultBundlePathValue(value)) { + resultBundlePath = value; + index += 1; + } + continue; + } + if (argument?.startsWith('-resultBundlePath=')) { + const value = argument.slice('-resultBundlePath='.length); + if (isResultBundlePathValue(value)) { + resultBundlePath = value; + } + } + } + + return resultBundlePath; +} + +function createXcodebuildTestArtifacts( + params: Pick, + started: ReturnType, + xcresultPath?: string, +): TestResultArtifacts { + return { + ...(params.deviceId ? { deviceId: params.deviceId } : {}), + buildLogPath: started.pipeline.logPath, + ...(xcresultPath ? { xcresultPath } : {}), + }; +} + export function resolveTestProgressEnabled(progress: boolean | undefined): boolean { return progress ?? process.env.XCODEBUILDMCP_RUNTIME === 'mcp'; } @@ -140,10 +187,7 @@ export function createTestExecutor( started, succeeded: false, target, - artifacts: { - ...(params.deviceId ? { deviceId: params.deviceId } : {}), - buildLogPath: started.pipeline.logPath, - }, + artifacts: createXcodebuildTestArtifacts(params, started), fallbackErrorMessages: getFallbackErrorMessages( started.stderrLines, buildForTestingResult.content, @@ -177,10 +221,7 @@ export function createTestExecutor( started, succeeded: !testWithoutBuildingResult.isError, target, - artifacts: { - ...(params.deviceId ? { deviceId: params.deviceId } : {}), - buildLogPath: started.pipeline.logPath, - }, + artifacts: createXcodebuildTestArtifacts(params, started, executionPlan.resultBundlePath), fallbackErrorMessages: getFallbackErrorMessages( started.stderrLines, testWithoutBuildingResult.content, @@ -206,10 +247,11 @@ export function createTestExecutor( started, succeeded: !singlePhaseResult.isError, target, - artifacts: { - ...(params.deviceId ? { deviceId: params.deviceId } : {}), - buildLogPath: started.pipeline.logPath, - }, + artifacts: createXcodebuildTestArtifacts( + params, + started, + findResultBundlePathArg(params.extraArgs), + ), fallbackErrorMessages: getFallbackErrorMessages( started.stderrLines, singlePhaseResult.content, @@ -225,10 +267,7 @@ export function createTestExecutor( started, succeeded: false, target, - artifacts: { - ...(params.deviceId ? { deviceId: params.deviceId } : {}), - buildLogPath: started.pipeline.logPath, - }, + artifacts: createXcodebuildTestArtifacts(params, started), fallbackErrorMessages: [...started.stderrLines, errorMessage], preflight: options.preflight, request: options.request, diff --git a/src/utils/xcodebuild-domain-results.ts b/src/utils/xcodebuild-domain-results.ts index 65e94f733..44abdb364 100644 --- a/src/utils/xcodebuild-domain-results.ts +++ b/src/utils/xcodebuild-domain-results.ts @@ -7,6 +7,7 @@ import type { BuildRunResultArtifacts, BuildRunResultDomainResult, BuildTarget, + Counts, TestDiagnostics, TestResultArtifacts, TestResultDomainResult, @@ -27,6 +28,7 @@ import type { XcodebuildRunState } from './xcodebuild-run-state.js'; import { collectResolvedTestSelectors, type TestPreflightResult } from './test-preflight.js'; import { createStreamingExecutionContext } from './tool-execution-compat.js'; import { isBuildErrorDiagnosticLine } from './xcodebuild-line-parsers.js'; +import { extractTestSummaryCountsFromXcresult } from './xcresult-test-failures.ts'; const MAX_DISCOVERED_TESTS = 6; @@ -172,6 +174,22 @@ function hasTestCounts(state: XcodebuildRunState): boolean { ); } +function createStateTestCounts(state: XcodebuildRunState): Counts | undefined { + if (!hasTestCounts(state)) { + return undefined; + } + + const failed = Math.max(state.failedTests, state.testFailures.length); + const skipped = state.skippedTests; + const passed = Math.max(0, state.completedTests - failed - skipped); + + return { + passed, + failed, + skipped, + }; +} + export function createTestDiscoveryFragment( preflight?: TestPreflightResult, ): TestDiscoveryFragment | null { @@ -372,9 +390,6 @@ export function createTestDomainResult(options: { }): TestResultDomainResult { const { durationMs, pipelineResult } = finalizePipelineResult(options); const state = pipelineResult.state; - const failed = Math.max(state.failedTests, state.testFailures.length); - const skipped = state.skippedTests; - const passed = Math.max(0, state.completedTests - failed - skipped); const testSelectionInfo = createTestSelectionInfo(options.preflight); const testCases = state.testCaseResults.map((fragment) => ({ ...(fragment.suite !== undefined ? { suite: fragment.suite } : {}), @@ -382,6 +397,18 @@ export function createTestDomainResult(options: { status: fragment.status, ...(fragment.durationMs !== undefined ? { durationMs: fragment.durationMs } : {}), })); + const detectedXcresultPath = + options.target === 'swift-package' ? null : options.started.pipeline.xcresultPath; + const providedXcresultPath = + 'xcresultPath' in options.artifacts ? options.artifacts.xcresultPath : undefined; + const xcresultPath = detectedXcresultPath ?? providedXcresultPath; + const artifacts: TestResultArtifacts = { + ...options.artifacts, + ...(xcresultPath ? { xcresultPath } : {}), + }; + const counts = + (xcresultPath ? extractTestSummaryCountsFromXcresult(xcresultPath) : null) ?? + createStateTestCounts(state); const result: TestResultDomainResult = { kind: 'test-result', request: options.request, @@ -390,18 +417,10 @@ export function createTestDomainResult(options: { summary: { status: options.succeeded ? 'SUCCEEDED' : 'FAILED', durationMs, - ...(hasTestCounts(state) - ? { - counts: { - passed, - failed, - skipped, - }, - } - : {}), + ...(counts ? { counts } : {}), target: options.target, }, - artifacts: options.artifacts, + artifacts, ...(testSelectionInfo ? { tests: testSelectionInfo } : {}), diagnostics: createTestDiagnostics(state, !options.succeeded, options.fallbackErrorMessages), ...(testCases.length > 0 ? { testCases } : {}), diff --git a/src/utils/xcodebuild-event-parser.ts b/src/utils/xcodebuild-event-parser.ts index f6c19373a..a91ba22b8 100644 --- a/src/utils/xcodebuild-event-parser.ts +++ b/src/utils/xcodebuild-event-parser.ts @@ -94,6 +94,22 @@ function normalizeEventLine(rawLine: string): string { return rawLine.trim().replace(/^(?:\u200B|\u200C|\u200D|\uFEFF)+/u, ''); } +function parseXcresultPathLine(line: string): string | null { + const resultBundleMessage = line.match( + /(?:Writing error result bundle to|Writing result bundle to|Result bundle written to):?\s+(.+?\.xcresult)\s*$/u, + ); + if (resultBundleMessage) { + return resultBundleMessage[1]; + } + + const standalonePath = line.match(/^((?:\/|~\/|\.\.?\/)[^\n]*\.xcresult)\s*$/u); + if (standalonePath && !/\s-[A-Za-z]/u.test(standalonePath[1])) { + return standalonePath[1]; + } + + return null; +} + export interface EventParserOptions { operation: XcodebuildOperation; kind?: BuildLikeKind; @@ -313,6 +329,12 @@ export function createXcodebuildEventParser(options: EventParserOptions): Xcodeb flushPendingError(); + const xcresultPath = parseXcresultPathLine(line); + if (xcresultPath) { + detectedXcresultPath = xcresultPath; + return; + } + const testCase = parseTestCaseLine(line); if (testCase) { const source = @@ -418,12 +440,6 @@ export function createXcodebuildEventParser(options: EventParserOptions): Xcodeb return; } - const xcresultMatch = line.match(/^\s*(\S+\.xcresult)\s*$/u); - if (xcresultMatch) { - detectedXcresultPath = xcresultMatch[1]; - return; - } - if (onUnrecognizedLine) { onUnrecognizedLine(line); } diff --git a/src/utils/xcresult-test-failures.ts b/src/utils/xcresult-test-failures.ts index 8398bef62..0e3f25b38 100644 --- a/src/utils/xcresult-test-failures.ts +++ b/src/utils/xcresult-test-failures.ts @@ -1,6 +1,7 @@ import { execFileSync } from 'node:child_process'; import { log } from './logger.ts'; import type { TestFailureFragment } from '../types/domain-fragments.ts'; +import type { Counts } from '../types/domain-results.ts'; import { parseRawTestName } from './xcodebuild-line-parsers.ts'; interface XcresultTestNode { @@ -14,6 +15,52 @@ interface XcresultTestResults { testNodes: XcresultTestNode[]; } +interface XcresultTestSummary { + totalTestCount?: unknown; + passedTests?: unknown; + failedTests?: unknown; + skippedTests?: unknown; +} + +function isSummaryCount(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value >= 0; +} + +export function parseXcresultTestSummaryCounts(raw: string): Counts | null { + const summary = JSON.parse(raw) as XcresultTestSummary; + const { passedTests, failedTests, skippedTests } = summary; + + if ( + !isSummaryCount(passedTests) || + !isSummaryCount(failedTests) || + !isSummaryCount(skippedTests) + ) { + return null; + } + + return { + passed: passedTests, + failed: failedTests, + skipped: skippedTests, + }; +} + +export function extractTestSummaryCountsFromXcresult(xcresultPath: string): Counts | null { + try { + const output = execFileSync( + 'xcrun', + ['xcresulttool', 'get', 'test-results', 'summary', '--path', xcresultPath, '--compact'], + { encoding: 'utf8', timeout: 10_000, stdio: ['ignore', 'pipe', 'pipe'] }, + ); + + return parseXcresultTestSummaryCounts(output); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + log('debug', `Failed to extract test summary from xcresult: ${message}`); + return null; + } +} + export function extractTestFailuresFromXcresult(xcresultPath: string): TestFailureFragment[] { try { const output = execFileSync( From 271ba1ca11bc726672498243609fd6d171736e7c Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Wed, 6 May 2026 20:14:01 +0100 Subject: [PATCH 2/4] docs: Clarify streaming fragment output contract --- AGENTS.md | 6 +++--- CLAUDE.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5da35e997..3b7228dc4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,9 +56,9 @@ ESM TypeScript project (`type: module`). Key layers: ## Rendering and Streaming Contract -- Streaming fragments are transient output only. They MUST NOT be used as internal state, cached for final responses, or promoted into final MCP/JSON/CLI text output. -- Non-streaming runtimes/output modes, including MCP final responses, MUST render only from the final structured result and next-step metadata. If final output needs data, add it to the final result type instead of reading it from fragments. -- Only streaming-capable renderers may observe fragment callbacks, and only to print live progress. Their fragment handling must not affect final structured output or final rendered text. +- Streaming fragments are transient live-progress output only. They may be displayed while a tool is running, but MUST NOT provide final settled MCP/JSON/CLI text. +- Final settled output MUST render from the final structured/domain result and next-step metadata. If final output needs data, add it to the final result type instead of reading it from fragments. +- Streaming-capable renderers may observe fragment callbacks only for live progress. Fragment handling must not affect final structured output or final settled text. ## Test Conventions - Vitest with colocated `__tests__/` directories using `*.test.ts` diff --git a/CLAUDE.md b/CLAUDE.md index 21932d9d0..531c6c7d6 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,9 +66,9 @@ Use these sections under `## [Unreleased]`: ## Rendering and Streaming Contract -- Streaming fragments are transient output only. They MUST NOT be used as internal state, cached for final responses, or promoted into final MCP/JSON/CLI text output. -- Non-streaming runtimes/output modes, including MCP final responses, MUST render only from the final structured result and next-step metadata. If final output needs data, add it to the final result type instead of reading it from fragments. -- Only streaming-capable renderers may observe fragment callbacks, and only to print live progress. Their fragment handling must not affect final structured output or final rendered text. +- Streaming fragments are transient live-progress output only. They may be displayed while a tool is running, but MUST NOT provide final settled MCP/JSON/CLI text. +- Final settled output MUST render from the final structured/domain result and next-step metadata. If final output needs data, add it to the final result type instead of reading it from fragments. +- Streaming-capable renderers may observe fragment callbacks only for live progress. Fragment handling must not affect final structured output or final settled text. ## Test Execution Rules - When running long test suites (snapshot tests, smoke tests), ALWAYS write full output to a log file and read it afterwards. NEVER pipe through `tail` or `grep` directly — that loses output you may need to debug failures. From 899b5b75bd12f3fa8d324415a6ca2dc1edd0de9d Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Wed, 6 May 2026 20:17:50 +0100 Subject: [PATCH 3/4] fix(test): Handle malformed xcresult summaries Return null when xcresult summary parsing receives malformed JSON so the exported parser matches its nullable contract and callers can safely fall back to streamed counts. Refs #397 Co-Authored-By: Codex --- src/utils/__tests__/xcresult-test-failures.test.ts | 5 +++++ src/utils/xcresult-test-failures.ts | 12 +++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/utils/__tests__/xcresult-test-failures.test.ts b/src/utils/__tests__/xcresult-test-failures.test.ts index c21c5673b..363ca1dc1 100644 --- a/src/utils/__tests__/xcresult-test-failures.test.ts +++ b/src/utils/__tests__/xcresult-test-failures.test.ts @@ -27,6 +27,11 @@ describe('parseXcresultTestSummaryCounts', () => { skipped: 0, }); }); + + it('returns null for malformed JSON summary output', () => { + expect(parseXcresultTestSummaryCounts('warning: no summary available')).toBeNull(); + expect(parseXcresultTestSummaryCounts('')).toBeNull(); + }); }); describe('parseXcresultFailureMessage', () => { diff --git a/src/utils/xcresult-test-failures.ts b/src/utils/xcresult-test-failures.ts index 0e3f25b38..1e8abfa39 100644 --- a/src/utils/xcresult-test-failures.ts +++ b/src/utils/xcresult-test-failures.ts @@ -27,7 +27,17 @@ function isSummaryCount(value: unknown): value is number { } export function parseXcresultTestSummaryCounts(raw: string): Counts | null { - const summary = JSON.parse(raw) as XcresultTestSummary; + let summary: XcresultTestSummary; + try { + const parsed = JSON.parse(raw) as unknown; + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + return null; + } + summary = parsed as XcresultTestSummary; + } catch { + return null; + } + const { passedTests, failedTests, skippedTests } = summary; if ( From 66608a008691de2a2967d50751a3dba0dedc996b Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Wed, 6 May 2026 21:47:25 +0100 Subject: [PATCH 4/4] ref(test): Share result bundle argument parsing Move result bundle path parsing into a shared helper so single-phase and two-phase test execution paths use the same validation and precedence rules. Refs #397 Co-Authored-By: Codex --- src/utils/result-bundle-args.ts | 48 +++++++++++++++++++++++++++ src/utils/simulator-test-execution.ts | 48 +++++++++++---------------- src/utils/test-common.ts | 32 +----------------- 3 files changed, 69 insertions(+), 59 deletions(-) create mode 100644 src/utils/result-bundle-args.ts diff --git a/src/utils/result-bundle-args.ts b/src/utils/result-bundle-args.ts new file mode 100644 index 000000000..6dacce471 --- /dev/null +++ b/src/utils/result-bundle-args.ts @@ -0,0 +1,48 @@ +function isResultBundlePathValue(value: string | undefined): value is string { + return value !== undefined && value.length > 0 && !value.startsWith('-'); +} + +export function parseResultBundlePathArgs(extraArgs?: readonly string[]): { + remainingArgs: string[]; + resultBundlePath?: string; +} { + if (!extraArgs) { + return { remainingArgs: [] }; + } + + const remainingArgs: string[] = []; + let resultBundlePath: string | undefined; + + for (let index = 0; index < extraArgs.length; index += 1) { + const argument = extraArgs[index]; + if (argument === '-resultBundlePath') { + const value = extraArgs[index + 1]; + if (isResultBundlePathValue(value)) { + resultBundlePath = value; + index += 1; + } + continue; + } + + if (argument?.startsWith('-resultBundlePath=')) { + const value = argument.slice('-resultBundlePath='.length); + if (isResultBundlePathValue(value)) { + resultBundlePath = value; + } + continue; + } + + if (argument !== undefined) { + remainingArgs.push(argument); + } + } + + return { + remainingArgs, + ...(resultBundlePath ? { resultBundlePath } : {}), + }; +} + +export function findResultBundlePathArg(extraArgs?: readonly string[]): string | undefined { + return parseResultBundlePathArgs(extraArgs).resultBundlePath; +} diff --git a/src/utils/simulator-test-execution.ts b/src/utils/simulator-test-execution.ts index 057c34cc6..af7d88762 100644 --- a/src/utils/simulator-test-execution.ts +++ b/src/utils/simulator-test-execution.ts @@ -1,27 +1,30 @@ +import { parseResultBundlePathArgs } from './result-bundle-args.ts'; import type { TestPreflightResult } from './test-preflight.ts'; -function isResultBundlePathValue(value: string | undefined): value is string { - return value !== undefined && value.length > 0 && !value.startsWith('-'); -} - function parseTestSelectorArgs(extraArgs: string[] | undefined): { remainingArgs: string[]; selectorArgs: string[]; resultBundlePath?: string; } { - if (!extraArgs || extraArgs.length === 0) { - return { remainingArgs: [], selectorArgs: [] }; + const parsedResultBundleArgs = parseResultBundlePathArgs(extraArgs); + if (parsedResultBundleArgs.remainingArgs.length === 0) { + return { + remainingArgs: [], + selectorArgs: [], + ...(parsedResultBundleArgs.resultBundlePath + ? { resultBundlePath: parsedResultBundleArgs.resultBundlePath } + : {}), + }; } const remainingArgs: string[] = []; const selectorArgs: string[] = []; - let resultBundlePath: string | undefined; - for (let index = 0; index < extraArgs.length; index += 1) { - const argument = extraArgs[index]!; + for (let index = 0; index < parsedResultBundleArgs.remainingArgs.length; index += 1) { + const argument = parsedResultBundleArgs.remainingArgs[index]!; if (argument === '-only-testing' || argument === '-skip-testing') { - const value = extraArgs[index + 1]; + const value = parsedResultBundleArgs.remainingArgs[index + 1]; if (value) { selectorArgs.push(argument, value); index += 1; @@ -29,23 +32,6 @@ function parseTestSelectorArgs(extraArgs: string[] | undefined): { continue; } - if (argument === '-resultBundlePath') { - const value = extraArgs[index + 1]; - if (isResultBundlePathValue(value)) { - resultBundlePath = value; - index += 1; - } - continue; - } - - if (argument.startsWith('-resultBundlePath=')) { - const value = argument.slice('-resultBundlePath='.length); - if (isResultBundlePathValue(value)) { - resultBundlePath = value; - } - continue; - } - if (argument.startsWith('-only-testing:') || argument.startsWith('-skip-testing:')) { selectorArgs.push(argument); continue; @@ -54,7 +40,13 @@ function parseTestSelectorArgs(extraArgs: string[] | undefined): { remainingArgs.push(argument); } - return { remainingArgs, selectorArgs, resultBundlePath }; + return { + remainingArgs, + selectorArgs, + ...(parsedResultBundleArgs.resultBundlePath + ? { resultBundlePath: parsedResultBundleArgs.resultBundlePath } + : {}), + }; } export function createSimulatorTwoPhaseExecutionPlan(params: { diff --git a/src/utils/test-common.ts b/src/utils/test-common.ts index 0f822aab8..50a1a2ccb 100644 --- a/src/utils/test-common.ts +++ b/src/utils/test-common.ts @@ -16,6 +16,7 @@ import { getDefaultCommandExecutor } from './command.ts'; import { type TestPreflightResult } from './test-preflight.ts'; import { createSimulatorTwoPhaseExecutionPlan } from './simulator-test-execution.ts'; +import { findResultBundlePathArg } from './result-bundle-args.ts'; import type { BuildTarget, @@ -59,37 +60,6 @@ function getFallbackErrorMessages( return [...streamedLines, ...(responseContent ?? []).map((item) => item.text)]; } -function isResultBundlePathValue(value: string | undefined): value is string { - return value !== undefined && value.length > 0 && !value.startsWith('-'); -} - -function findResultBundlePathArg(extraArgs?: readonly string[]): string | undefined { - if (!extraArgs) { - return undefined; - } - - let resultBundlePath: string | undefined; - for (let index = 0; index < extraArgs.length; index += 1) { - const argument = extraArgs[index]; - if (argument === '-resultBundlePath') { - const value = extraArgs[index + 1]; - if (isResultBundlePathValue(value)) { - resultBundlePath = value; - index += 1; - } - continue; - } - if (argument?.startsWith('-resultBundlePath=')) { - const value = argument.slice('-resultBundlePath='.length); - if (isResultBundlePathValue(value)) { - resultBundlePath = value; - } - } - } - - return resultBundlePath; -} - function createXcodebuildTestArtifacts( params: Pick, started: ReturnType,