diff --git a/CHANGELOG.md b/CHANGELOG.md index f47e609a..5d157783 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- Fixed test progress and summaries counting each failed Swift Testing assertion as a failed test, which could report more failed tests than were run and zero passing tests ([#533](https://github.com/getsentry/MobileBuildMCP/issues/533), [#541](https://github.com/getsentry/MobileBuildMCP/pull/541) by [@breken-ai](https://github.com/breken-ai)). + ## [2.7.1] ### Changed diff --git a/src/utils/__tests__/xcodebuild-event-parser.test.ts b/src/utils/__tests__/xcodebuild-event-parser.test.ts index a9774083..a0d26dd4 100644 --- a/src/utils/__tests__/xcodebuild-event-parser.test.ts +++ b/src/utils/__tests__/xcodebuild-event-parser.test.ts @@ -678,6 +678,45 @@ describe('xcodebuild-event-parser', () => { expect(progress).toEqual([expect.objectContaining({ completed: 2, failed: 2, skipped: 0 })]); }); + it('counts a Swift Testing test with several issues as one failed test', () => { + const events = collectRunStateEvents([ + { source: 'stdout', text: '✔ Test "passes" passed after 0.001 seconds.\n' }, + { + source: 'stdout', + text: '✘ Test "fails" recorded an issue at CountsTests.swift:10:3: Expectation failed: first\n', + }, + { + source: 'stdout', + text: '✘ Test "fails" recorded an issue at CountsTests.swift:11:3: Expectation failed: second\n', + }, + { + source: 'stdout', + text: '✘ Test "fails" recorded an issue at CountsTests.swift:12:3: Expectation failed: third\n', + }, + { + source: 'stdout', + text: '✘ Test "fails" failed after 0.001 seconds with 3 issues.\n', + }, + { + source: 'stdout', + text: '✘ Test run with 2 tests in 1 suite failed after 0.002 seconds with 3 issues.\n', + }, + ]); + + expect(events.filter((event) => event.fragment === 'test-failure')).toHaveLength(3); + expect(events.filter((event) => event.fragment === 'test-progress').at(-1)).toMatchObject({ + completed: 2, + failed: 1, + skipped: 0, + }); + expect(events.filter((event) => event.fragment === 'build-summary').at(-1)).toMatchObject({ + totalTests: 2, + passedTests: 1, + failedTests: 1, + skippedTests: 0, + }); + }); + it('keeps parameterized Swift Testing result counts aligned with the run summary', () => { const events = collectRunStateEvents([ { diff --git a/src/utils/__tests__/xcodebuild-run-state.test.ts b/src/utils/__tests__/xcodebuild-run-state.test.ts index f29a32de..dd50882c 100644 --- a/src/utils/__tests__/xcodebuild-run-state.test.ts +++ b/src/utils/__tests__/xcodebuild-run-state.test.ts @@ -394,6 +394,42 @@ Actual mismatch`, expect(finalState.testFailures).toHaveLength(2); }); + it('counts several failure diagnostics from one test as one failed test', () => { + const forwarded: DomainFragment[] = []; + const state = createXcodebuildRunState({ + operation: 'TEST', + onEvent: (e) => forwarded.push(e), + }); + + state.push({ + kind: 'test-result', + fragment: 'test-progress', + operation: 'TEST', + completed: 2, + failed: 1, + skipped: 0, + }); + for (const line of [10, 11, 12]) { + state.push({ + kind: 'test-result', + fragment: 'test-failure', + operation: 'TEST', + test: 'fails', + message: `Expectation failed: ${line}`, + location: `CountsTests.swift:${line}:3`, + }); + } + + state.finalize(false); + expect(forwarded.at(-1)).toMatchObject({ + fragment: 'build-summary', + totalTests: 2, + passedTests: 1, + failedTests: 1, + skippedTests: 0, + }); + }); + it('highestStageRank returns correct rank for multi-phase handoff', () => { const state = createXcodebuildRunState({ operation: 'TEST' }); diff --git a/src/utils/swift-testing-line-parsers.ts b/src/utils/swift-testing-line-parsers.ts index dddf20ef..60188dc5 100644 --- a/src/utils/swift-testing-line-parsers.ts +++ b/src/utils/swift-testing-line-parsers.ts @@ -158,8 +158,8 @@ export function parseSwiftTestingRunSummary(line: string): ParsedTotals | null { // Swift Testing reports "issues" not "failed tests" -- a single test can produce // multiple issues (e.g. multiple #expect failures). This is the best available // approximation; the framework doesn't report a distinct failed-test count in its - // summary line. Downstream reconciliation via Math.max(failedTests, testFailures.length) - // partially mitigates overcounting. + // summary line. The event parser subtracts issues already attributed to + // individually reported failed tests before using this value. const issueMatch = line.match(/with (\d+) issues?/u); const failed = issueMatch ? Number(issueMatch[1]) : 0; diff --git a/src/utils/xcodebuild-domain-results.ts b/src/utils/xcodebuild-domain-results.ts index b53e0997..910d3181 100644 --- a/src/utils/xcodebuild-domain-results.ts +++ b/src/utils/xcodebuild-domain-results.ts @@ -25,6 +25,7 @@ import { finalizeInlineXcodebuild } from './xcodebuild-output.ts'; import type { StartedPipeline, XcodebuildPipeline } from './xcodebuild-pipeline.ts'; import { createXcodebuildPipeline } from './xcodebuild-pipeline.ts'; import type { XcodebuildRunState } from './xcodebuild-run-state.ts'; +import { countFailedTests } from './xcodebuild-run-state.ts'; import { collectResolvedTestSelectors, type TestPreflightResult } from './test-preflight.ts'; import { createStreamingExecutionContext } from './tool-execution-compat.ts'; import { isBuildErrorDiagnosticLine } from './xcodebuild-line-parsers.ts'; @@ -185,7 +186,7 @@ function createStateTestCounts(state: XcodebuildRunState): Counts | undefined { return undefined; } - const failed = Math.max(state.failedTests, state.testFailures.length); + const failed = countFailedTests(state); const skipped = state.skippedTests; const passed = Math.max(0, state.completedTests - failed - skipped); diff --git a/src/utils/xcodebuild-event-parser.ts b/src/utils/xcodebuild-event-parser.ts index a91ba22b..328517fc 100644 --- a/src/utils/xcodebuild-event-parser.ts +++ b/src/utils/xcodebuild-event-parser.ts @@ -135,7 +135,7 @@ export function createXcodebuildEventParser(options: EventParserOptions): Xcodeb let failedCount = 0; let skippedCount = 0; let testCasesCompletedSinceSwiftTestingSummary = 0; - let testCasesFailedSinceSwiftTestingSummary = 0; + let issuesAccountedSinceSwiftTestingSummary = 0; let detectedXcresultPath: string | null = null; let pendingError: { @@ -251,6 +251,7 @@ export function createXcodebuildEventParser(options: EventParserOptions): Xcodeb function recordTestCaseResult( testCase: ParsedTestCase, source: 'xcodebuild' | 'swift-testing' | 'swift-testing-native' = 'xcodebuild', + issueCount = 1, ): void { const increment = 1; completedCount += increment; @@ -258,17 +259,15 @@ export function createXcodebuildEventParser(options: EventParserOptions): Xcodeb if (testCase.status === 'failed') { applyFailureDuration(testCase.suiteName, testCase.testName, durationMs); - if (source !== 'swift-testing-native') { - failedCount += increment; - } + failedCount += increment; } else if (testCase.status === 'skipped') { skippedCount += increment; } if (source !== 'xcodebuild') { testCasesCompletedSinceSwiftTestingSummary += increment; - if (source === 'swift-testing' && testCase.status === 'failed') { - testCasesFailedSinceSwiftTestingSummary += increment; + if (testCase.status === 'failed') { + issuesAccountedSinceSwiftTestingSummary += issueCount; } } @@ -367,19 +366,32 @@ export function createXcodebuildEventParser(options: EventParserOptions): Xcodeb const stResult = parseSwiftTestingResultLine(line); if (stResult) { - recordTestCaseResult(stResult, 'swift-testing-native'); + const issueMatch = line.match(/with (\d+) issues?\.?$/u); + recordTestCaseResult( + stResult, + 'swift-testing-native', + issueMatch ? Number(issueMatch[1]) : 1, + ); return; } const stSummary = parseSwiftTestingRunSummary(line); if (stSummary) { - completedCount += Math.max( + // The summary reports issues, not failed tests, and one test can record + // several issues. Only issues not already attributed to a reported failed + // test can mark unreported tests as failed. + const unreportedTests = Math.max( 0, stSummary.executed - testCasesCompletedSinceSwiftTestingSummary, ); - failedCount += Math.max(0, stSummary.failed - testCasesFailedSinceSwiftTestingSummary); + const unaccountedIssues = Math.max( + 0, + stSummary.failed - issuesAccountedSinceSwiftTestingSummary, + ); + completedCount += unreportedTests; + failedCount += Math.min(unreportedTests, unaccountedIssues); testCasesCompletedSinceSwiftTestingSummary = 0; - testCasesFailedSinceSwiftTestingSummary = 0; + issuesAccountedSinceSwiftTestingSummary = 0; emitTestProgress(); return; } diff --git a/src/utils/xcodebuild-run-state.ts b/src/utils/xcodebuild-run-state.ts index b3ea9dee..4bef7507 100644 --- a/src/utils/xcodebuild-run-state.ts +++ b/src/utils/xcodebuild-run-state.ts @@ -83,6 +83,21 @@ function normalizeTestFailureKey(fragment: TestFailureFragment): string { return `${suite}|${test}|${normalizedMessage}`; } +/** + * Failed test count for a run. Failure diagnostics are counted per test, not per + * diagnostic, because one test can record several failed assertions. + */ +export function countFailedTests( + state: Pick, +): number { + const failedTestKeys = new Set(); + for (const [index, failure] of state.testFailures.entries()) { + const test = normalizeTestIdentifier(failure.test); + failedTestKeys.add(test ? `test:${test}` : `failure:${index}`); + } + return Math.max(state.failedTests, failedTestKeys.size); +} + export interface XcodebuildRunStateHandle { push(fragment: XcodebuildRunStateFragment): void; finalize(succeeded: boolean, durationMs?: number): XcodebuildRunState; @@ -95,7 +110,7 @@ function createTestSummaryFragment( kind: 'build-result' | 'build-run-result' | 'test-result', durationMs?: number, ): BuildSummaryFragment { - const failedTests = Math.max(state.failedTests, state.testFailures.length); + const failedTests = countFailedTests(state); const passedTests = Math.max(0, state.completedTests - failedTests - state.skippedTests); const totalTests = passedTests + failedTests + state.skippedTests;