diff --git a/CLAUDE.md b/CLAUDE.md index 351a16d..8ef10cd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -319,7 +319,7 @@ The codebase follows a modular architecture: - **Slowest Targets**: Top 5 targets sorted by duration (descending) in `slowest_targets` array - Supports xcodebuild phase detection from "(in target 'X' from project 'Y')" patterns - Supports SPM phase detection from "[N/M] Compiling/Linking TARGET" patterns - - Parses "Build target X (Ys)" and "** BUILD SUCCEEDED ** [Xs]" patterns + - Parses "Build target X (Ys)" and "** SUCCEEDED ** [Xs]" patterns - Build time in `summary.build_time`, test execution time in `summary.test_time` (not duplicated in build_info) - xcodebuild phases: `CompileSwiftSources`, `SwiftCompilation`, `CompileC`, `Link`, `CopySwiftLibs`, `PhaseScriptExecution`, `LinkAssetCatalog`, `ProcessInfoPlistFile` - SPM phases: `Compiling`, `Linking` diff --git a/Sources/XCSiftCore/LineParser.swift b/Sources/XCSiftCore/LineParser.swift index ce92669..78a310d 100644 --- a/Sources/XCSiftCore/LineParser.swift +++ b/Sources/XCSiftCore/LineParser.swift @@ -114,11 +114,12 @@ public struct LineParser: Sendable { public private(set) var didEmitXcbeautifyHint: Bool = false /// `true` if a positive terminal success marker was seen - /// (`** BUILD SUCCEEDED **`, `** TEST SUCCEEDED **`, `Build complete!`, `Build succeeded in …`). + /// (`** SUCCEEDED **`, `Build complete!`, `Build succeeded in …`). public private(set) var sawSuccessMarker: Bool = false - /// `true` if a terminal failure marker was seen - /// (`** BUILD FAILED **`, `** TEST FAILED **`, `Build failed after …`). + /// `true` if an authoritative terminal failure marker was seen + /// (`** FAILED **`, `Build failed after …`). This excludes `** TEST FAILED **`, which + /// xcodebuild also prints for a run that passes; that marker arrives as `.testRunFailed`. public private(set) var sawFailureMarker: Bool = false // MARK: - Event queue (events waiting to be delivered one per feed() call) @@ -370,8 +371,7 @@ public struct LineParser: Sendable { for marker in [ "Build succeeded", XcodebuildSymbols.succeededKeyword, - XcodebuildSymbols.buildFailedKeyword, - XcodebuildSymbols.testFailed, + XcodebuildSymbols.failedUppercaseKeyword, XcodebuildSymbols.buildComplete, ] { add(marker, candidates: .status) @@ -480,9 +480,9 @@ public struct LineParser: Sendable { } // xcbeautify rewrites the terminal `** … SUCCEEDED **` markers to title-case status lines. - if shouldParseXcbeautify - && (line.contains(XCBeautifySymbols.buildSucceeded) - || line.contains(XCBeautifySymbols.testSucceeded)) + // The suffix test is a cheap gate; the phase test then rejects run-script output. + if shouldParseXcbeautify, line.contains(XCBeautifySymbols.succeededSuffix), + XCBeautifySymbols.succeededMarkers.contains(where: { line.contains($0) }) { sawSuccessMarker = true } @@ -1411,23 +1411,21 @@ public struct LineParser: Sendable { } private mutating func parseBuildAndTestTime(_ line: String) -> ParseEvent? { - if line.contains(XcodebuildSymbols.buildSucceeded) - || line.contains(XcodebuildSymbols.testSucceeded) - || line.contains(XcodebuildSymbols.testExecuteSucceeded) - { - sawSuccessMarker = true - return bracketedTime(line) + if line.contains(XcodebuildSymbols.testFailed) { + sawTestRunFailed = true + return .testRunFailed } - if line.contains(XcodebuildSymbols.buildFailed) { - sawFailureMarker = true + if line.contains(XcodebuildSymbols.succeededMarkerSuffix) { + sawSuccessMarker = true return bracketedTime(line) } - if line.contains(XcodebuildSymbols.testFailed) { - sawTestRunFailed = true + if line.contains(XcodebuildSymbols.failedMarkerSuffix) { sawFailureMarker = true - return .testRunFailed + // A dead test executor also ends the test that was still in flight. + if line.contains(XcodebuildSymbols.testExecuteFailed) { sawTestRunFailed = true } + return bracketedTime(line) } if line.hasPrefix(XcodebuildSymbols.buildComplete) { diff --git a/Sources/XCSiftCore/OutputParser.swift b/Sources/XCSiftCore/OutputParser.swift index a402d02..4aa78f6 100644 --- a/Sources/XCSiftCore/OutputParser.swift +++ b/Sources/XCSiftCore/OutputParser.swift @@ -146,7 +146,8 @@ public struct StreamingOutputParser { } didEmitXcbeautifyHint = lineParser.didEmitXcbeautifyHint let sawSuccessMarker = lineParser.sawSuccessMarker - let sawFailureMarker = lineParser.sawFailureMarker || state.testRunFailed + let sawFailureMarker = lineParser.sawFailureMarker + let sawTestRunFailure = state.testRunFailed // If warnings-as-errors is enabled, convert warnings to errors var finalErrors = state.errors @@ -207,9 +208,13 @@ public struct StreamingOutputParser { let hasPassedTests = (computedPassedTests ?? 0) > 0 - // A terminal failure marker means the run failed even when no specific failure was - // attributed — unless tests actually passed (guards a stray "TEST FAILED" substring). - if sawFailureMarker { + // A terminal phase marker is authoritative even when no specific failure was + // attributed to a file or a test. + if sawFailureMarker { return "failed" } + + // `** TEST FAILED **` is not authoritative: xcodebuild also prints it for a run that + // passes under -skipMacroValidation (issue #52), so passed tests outrank it. + if sawTestRunFailure { return hasPassedTests ? "success" : "failed" } diff --git a/Sources/XCSiftCore/XCBeautifySymbols.swift b/Sources/XCSiftCore/XCBeautifySymbols.swift index 28a186c..9a83f4b 100644 --- a/Sources/XCSiftCore/XCBeautifySymbols.swift +++ b/Sources/XCSiftCore/XCBeautifySymbols.swift @@ -15,7 +15,25 @@ enum XCBeautifySymbols { static let measure = "◷" static let skipped = "⊘" - // Terminal status lines (xcbeautify rewrites `** BUILD/TEST SUCCEEDED **` to these). - static let buildSucceeded = "Build Succeeded" - static let testSucceeded = "Test Succeeded" + // Terminal status line (xcbeautify rewrites `** SUCCEEDED **` to " Succeeded"). + // The rewrite drops the `**` brackets, so the phase name is the only thing that separates a + // marker from ordinary run-script output. Match the known phases, not the bare suffix. + static let succeededSuffix = " Succeeded" + static let succeededMarkers = [ + "Build Succeeded", + "Build For Testing Succeeded", + "Test Succeeded", + "Test Execute Succeeded", + "Test Without Building Succeeded", + "Analyze Succeeded", + "Analyze For Testing Succeeded", + "Archive Succeeded", + "Export Succeeded", + "Clean Succeeded", + "Install Succeeded", + "Installsrc Succeeded", + "Installhdrs Succeeded", + "Installloc Succeeded", + "Docbuild Succeeded", + ] } diff --git a/Sources/XCSiftCore/XcodebuildSymbols.swift b/Sources/XCSiftCore/XcodebuildSymbols.swift index b35cdfa..1f477f9 100644 --- a/Sources/XCSiftCore/XcodebuildSymbols.swift +++ b/Sources/XCSiftCore/XcodebuildSymbols.swift @@ -46,14 +46,16 @@ enum XcodebuildSymbols { static let swiftTestingDetailsPrefix = "􀄵" static let swiftTestingDetailsPrefixFallback = "↳" - // Build status - static let buildSucceeded = "** BUILD SUCCEEDED **" - static let buildFailed = "** BUILD FAILED **" - static let buildFailedKeyword = "BUILD FAILED" + // Build status — xcodebuild ends every operation with `** SUCCEEDED/FAILED **` + // (BUILD, TEST, TEST EXECUTE, ARCHIVE, EXPORT, CLEAN, INSTALL, ANALYZE …) + static let succeededMarkerSuffix = " SUCCEEDED **" + static let failedMarkerSuffix = " FAILED **" + // The two keywords below route a line to the status parser. They are wider than the two + // markers above on purpose: the fast-path filter must not drop a line the parser still reads. static let succeededKeyword = "SUCCEEDED" + static let failedUppercaseKeyword = "FAILED" static let testFailed = "TEST FAILED" - static let testSucceeded = "** TEST SUCCEEDED **" - static let testExecuteSucceeded = "** TEST EXECUTE SUCCEEDED **" + static let testExecuteFailed = "TEST EXECUTE FAILED" static let buildComplete = "Build complete!" static let buildSucceededInPrefix = "Build succeeded in " static let buildFailedAfterPrefix = "Build failed after " diff --git a/Sources/xcsift/xcsift.docc/OutputFormats.md b/Sources/xcsift/xcsift.docc/OutputFormats.md index 2abea7b..f82841f 100644 --- a/Sources/xcsift/xcsift.docc/OutputFormats.md +++ b/Sources/xcsift/xcsift.docc/OutputFormats.md @@ -58,8 +58,8 @@ The default format outputs structured JSON with build status, summary, and detai ### Status Values -- `success` — the build/test run completed with no errors, no failed tests, and produced positive evidence of completion (a terminal `** BUILD SUCCEEDED **` / `** TEST SUCCEEDED **` / `Build complete!` marker, or passed tests). -- `failed` — errors, failed tests, linker errors, or a terminal `** … FAILED **` marker were detected. +- `success` — the build/test run completed with no errors, no failed tests, and produced positive evidence of completion (a terminal `** SUCCEEDED **` marker — `BUILD`, `TEST`, `ARCHIVE`, `EXPORT`, `CLEAN` … — a `Build complete!` marker, or passed tests). +- `failed` — errors, failed tests, linker errors, or a terminal `** … FAILED **` marker were detected. `** TEST FAILED **` is the one exception: xcodebuild also prints it for a run that passes under `-skipMacroValidation`, so passed tests outrank it. - `incomplete` — the stream ended without any terminal marker and without recognizable results. This typically means the build was truncated or killed (e.g. `Killed: 9` on memory pressure) before reporting an outcome. xcsift never reports a truncated run as `success`; combine with `--exit-on-failure` to fail the pipeline on `incomplete`. > **Migration note:** `incomplete` was introduced alongside the "success requires positive evidence" model. A successful stream lacking a recognizable terminal marker *and* passed tests now reports `incomplete` instead of `success`. Consumers that gate on status should treat anything other than `success` as non-success — checking only `status == "failed"` will miss `incomplete` runs. `--exit-on-failure` and `--quiet` already handle `incomplete` correctly. diff --git a/Tests/XCSiftCoreTests/LineParserTests.swift b/Tests/XCSiftCoreTests/LineParserTests.swift index db4eb4b..a9c41b7 100644 --- a/Tests/XCSiftCoreTests/LineParserTests.swift +++ b/Tests/XCSiftCoreTests/LineParserTests.swift @@ -315,7 +315,8 @@ final class LineParserTests: XCTestCase { } func testSawFailureMarker() { - for marker in ["** BUILD FAILED **", "** TEST FAILED **", "Build failed after 1.2s"] { + // `** TEST FAILED **` is absent on purpose: it arrives as .testRunFailed instead. + for marker in ["** BUILD FAILED **", "** ARCHIVE FAILED **", "Build failed after 1.2s"] { var parser = LineParser() _ = parser.feed(marker) XCTAssertTrue(parser.sawFailureMarker, "Expected failure marker for \(marker)") @@ -414,6 +415,19 @@ final class LineParserTests: XCTestCase { XCTAssertEqual(failedEvents[0].message, "Test did not complete (possible crash or timeout)") } + func testFlushEmitsCrashForInFlightTestOnTestExecuteFailed() { + var parser = LineParser() + _ = parser.feed("Test Case '-[MyModule.MyTests testCrashing]' started.") + _ = parser.feed("** TEST EXECUTE FAILED **") + let events = parser.flush() + let failedEvents = events.compactMap { event -> FailedTest? in + if case .testFailed(let failed) = event { return failed } + return nil + } + XCTAssertEqual(failedEvents.count, 1) + XCTAssertEqual(failedEvents.first?.test, "-[MyModule.MyTests testCrashing]") + } + // MARK: - No phantom testFailed after last test passes then TEST FAILED fires (issue #52 variant) func testNoPhantomFailureAfterPassedLastTest() { diff --git a/Tests/XCSiftCoreTests/ParsingTests.swift b/Tests/XCSiftCoreTests/ParsingTests.swift index f078903..73565da 100644 --- a/Tests/XCSiftCoreTests/ParsingTests.swift +++ b/Tests/XCSiftCoreTests/ParsingTests.swift @@ -1863,6 +1863,51 @@ final class ParsingTests: XCTestCase { XCTAssertEqual(result.summary.failedTests, 0) } + func testTerminalSuccessMarkersForEveryPhase() { + for marker in [ + "** BUILD SUCCEEDED **", + "** ARCHIVE SUCCEEDED **", + "** EXPORT SUCCEEDED **", + "** CLEAN SUCCEEDED **", + "** TEST EXECUTE SUCCEEDED **", + ] { + let parser = OutputParser() + let result = parser.parse(input: marker) + XCTAssertEqual(result.status, "success", marker) + } + } + + func testTerminalFailureMarkersForEveryPhase() { + for marker in ["** BUILD FAILED **", "** ARCHIVE FAILED **", "** EXPORT FAILED **"] { + let parser = OutputParser() + let result = parser.parse(input: marker) + XCTAssertEqual(result.status, "failed", marker) + } + } + + func testArchiveSuccessMarkerKeepsBuildTime() { + let parser = OutputParser() + let result = parser.parse(input: "** ARCHIVE SUCCEEDED ** [12.345 sec]") + + XCTAssertEqual(result.status, "success") + XCTAssertEqual(result.summary.buildTime, "12.345 sec") + } + + func testArchiveFailedMarkerOutranksPassedTests() { + // Only `** TEST FAILED **` is unreliable (issue #52). Every other phase marker is + // authoritative, so earlier passed tests must not turn the run green. + let parser = OutputParser() + let input = """ + Test Case 'MyTests.testExample' passed (0.001 seconds). + Executed 1 test, with 0 failures in 0.001 seconds + ** ARCHIVE FAILED ** + """ + + let result = parser.parse(input: input) + + XCTAssertEqual(result.status, "failed") + } + func testIncompleteOnMarkerlessStream() { let parser = OutputParser() let input = """ diff --git a/Tests/XCSiftCoreTests/XcbeautifyTests.swift b/Tests/XCSiftCoreTests/XcbeautifyTests.swift index ef77136..a2252f5 100644 --- a/Tests/XCSiftCoreTests/XcbeautifyTests.swift +++ b/Tests/XCSiftCoreTests/XcbeautifyTests.swift @@ -391,6 +391,53 @@ final class XcbeautifyIntegrationTests: XCTestCase { XCTAssertEqual(result.status, "success") } + func testArchiveAndExportSuccessMarkers() { + // xcbeautify rewrites every ** SUCCEEDED ** to " Succeeded". + for marker in ["Archive Succeeded", "Export Succeeded", "Test Execute Succeeded"] { + let parser = OutputParser() + let result = parser.parse(input: marker, xcbeautify: true) + XCTAssertEqual(result.status, "success", marker) + } + } + + func testScriptOutputContainingSucceededIsNotATerminalMarker() { + // A run-script line is not a phase marker. A killed build must stay incomplete. + let parser = OutputParser() + let input = """ + Compiling MyApp + [Upload] Upload Succeeded + Killed: 9 + """ + + let result = parser.parse(input: input, xcbeautify: true) + + XCTAssertEqual(result.status, "incomplete") + } + + func testEveryXcbeautifyPhaseSuccessMarker() { + // xcbeautify title-cases the phase word of `** SUCCEEDED **`. Verified against + // xcbeautify 3.2.1 for the whole xcodebuild action set. + for marker in [ + "Build Succeeded", "Build For Testing Succeeded", "Test Succeeded", + "Test Execute Succeeded", "Test Without Building Succeeded", "Analyze Succeeded", + "Analyze For Testing Succeeded", "Archive Succeeded", "Export Succeeded", + "Clean Succeeded", "Install Succeeded", "Installsrc Succeeded", + "Installhdrs Succeeded", "Installloc Succeeded", "Docbuild Succeeded", + ] { + let parser = OutputParser() + let result = parser.parse(input: marker, xcbeautify: true) + XCTAssertEqual(result.status, "success", marker) + } + } + + func testColoredSuccessMarker() { + // Terminal renderer wraps the marker in ANSI codes. + let parser = OutputParser() + let result = parser.parse(input: "\u{1B}[32;1mArchive Succeeded\u{1B}[0m", xcbeautify: true) + + XCTAssertEqual(result.status, "success") + } + func testDefaultModeUnaffected() { let parser = OutputParser() let input = """