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/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/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. 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..363ca1dc1 100644 --- a/src/utils/__tests__/xcresult-test-failures.test.ts +++ b/src/utils/__tests__/xcresult-test-failures.test.ts @@ -1,5 +1,38 @@ 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, + }); + }); + + it('returns null for malformed JSON summary output', () => { + expect(parseXcresultTestSummaryCounts('warning: no summary available')).toBeNull(); + expect(parseXcresultTestSummaryCounts('')).toBeNull(); + }); +}); 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/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 c23e1bf30..af7d88762 100644 --- a/src/utils/simulator-test-execution.ts +++ b/src/utils/simulator-test-execution.ts @@ -1,3 +1,4 @@ +import { parseResultBundlePathArgs } from './result-bundle-args.ts'; import type { TestPreflightResult } from './test-preflight.ts'; function parseTestSelectorArgs(extraArgs: string[] | undefined): { @@ -5,19 +6,25 @@ function parseTestSelectorArgs(extraArgs: string[] | undefined): { 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; @@ -25,15 +32,6 @@ function parseTestSelectorArgs(extraArgs: string[] | undefined): { continue; } - if (argument === '-resultBundlePath') { - const value = extraArgs[index + 1]; - if (value) { - resultBundlePath = value; - index += 1; - } - continue; - } - if (argument.startsWith('-only-testing:') || argument.startsWith('-skip-testing:')) { selectorArgs.push(argument); continue; @@ -42,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: { @@ -53,6 +57,7 @@ export function createSimulatorTwoPhaseExecutionPlan(params: { buildArgs: string[]; testArgs: string[]; usesExactSelectors: boolean; + resultBundlePath?: string; } { const parsedArgs = parseTestSelectorArgs(params.extraArgs); const selectedTestArgs = parsedArgs.selectorArgs; @@ -67,5 +72,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..50a1a2ccb 100644 --- a/src/utils/test-common.ts +++ b/src/utils/test-common.ts @@ -16,8 +16,13 @@ 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, 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 +60,18 @@ function getFallbackErrorMessages( return [...streamedLines, ...(responseContent ?? []).map((item) => item.text)]; } +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 +157,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 +191,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 +217,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 +237,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..1e8abfa39 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,62 @@ 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 { + 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 ( + !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(