diff --git a/Sources/EvidenceCLIKit/CommandRunner.swift b/Sources/EvidenceCLIKit/CommandRunner.swift index 6a36d13..d3c02d1 100644 --- a/Sources/EvidenceCLIKit/CommandRunner.swift +++ b/Sources/EvidenceCLIKit/CommandRunner.swift @@ -65,13 +65,50 @@ public struct ProcessCommandRunner: CommandRunning { process.standardOutput = stdout process.standardError = stderr + var stdoutData = Data() + var stderrData = Data() + let stdoutLock = NSLock() + let stderrLock = NSLock() + + stdout.fileHandleForReading.readabilityHandler = { handle in + let data = handle.availableData + guard !data.isEmpty else { return } + stdoutLock.lock() + stdoutData.append(data) + stdoutLock.unlock() + } + stderr.fileHandleForReading.readabilityHandler = { handle in + let data = handle.availableData + guard !data.isEmpty else { return } + stderrLock.lock() + stderrData.append(data) + stderrLock.unlock() + } + defer { + stdout.fileHandleForReading.readabilityHandler = nil + stderr.fileHandleForReading.readabilityHandler = nil + } + try process.run() process.waitUntilExit() + stdout.fileHandleForReading.readabilityHandler = nil + stderr.fileHandleForReading.readabilityHandler = nil + let remainingStdout = stdout.fileHandleForReading.readDataToEndOfFile() + let remainingStderr = stderr.fileHandleForReading.readDataToEndOfFile() + stdoutLock.lock() + stdoutData.append(remainingStdout) + let finalStdout = stdoutData + stdoutLock.unlock() + stderrLock.lock() + stderrData.append(remainingStderr) + let finalStderr = stderrData + stderrLock.unlock() + return CommandResult( exitCode: process.terminationStatus, - stdout: String(data: stdout.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "", - stderr: String(data: stderr.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "" + stdout: String(data: finalStdout, encoding: .utf8) ?? "", + stderr: String(data: finalStderr, encoding: .utf8) ?? "" ) } } diff --git a/Sources/EvidenceCLIKit/EvidencePlanExecution.swift b/Sources/EvidenceCLIKit/EvidencePlanExecution.swift index 62f958d..3247c2d 100644 --- a/Sources/EvidenceCLIKit/EvidencePlanExecution.swift +++ b/Sources/EvidenceCLIKit/EvidencePlanExecution.swift @@ -236,17 +236,38 @@ public struct XcodeTestPlanExecutor: EvidencePlanExecuting { guard let session, let videoPlan else { return nil } try videoRecorder.stop(session) let timestamp = timestamp() - result.artifacts.append(artifactWriter.artifact( + let artifact = try videoArtifact( kind: .video, phase: phase, path: session.outputPath, stepName: videoPlan.artifactStepName, mediaType: mediaType(for: URL(fileURLWithPath: session.outputPath)), capturedAt: timestamp - )) + ) + result.artifacts.append(artifact) return session.outputPath } + private func videoArtifact( + kind: CapturedArtifact.Kind, + phase: PRChangeEvidencePhase, + path: String, + stepName: String, + mediaType: String, + capturedAt: String + ) throws -> CapturedArtifact { + let artifact = artifactWriter.artifact( + kind: kind, + phase: phase, + path: path, + stepName: stepName, + mediaType: mediaType, + capturedAt: capturedAt + ) + try validateVideoArtifact(artifact, phase: phase) + return artifact + } + private func artifactPath( for step: PRChangeEvidenceStep, defaultPath: String?, @@ -373,7 +394,7 @@ public final class SimctlVideoRecorder: VideoRecording { public func stop(_ session: VideoRecordingSession) throws { guard let process = session.process else { return } if process.isRunning { - process.terminate() + process.interrupt() } process.waitUntilExit() } @@ -440,25 +461,27 @@ public struct SimctlPlanExecutor: EvidencePlanExecuting { didLaunch = true } - func finishActiveVideo(recordingStepName: String? = nil, capturedAt: String? = nil) { + func finishActiveVideo(recordingStepName: String? = nil, capturedAt: String? = nil) throws { if let session = activeVideo { - try? videoRecorder.stop(session) + try videoRecorder.stop(session) activeVideo = nil if let recordingStepName, let capturedAt { - result.artifacts.append(artifactWriter.artifact( + let artifact = artifactWriter.artifact( kind: .video, phase: build.phase, path: session.outputPath, stepName: recordingStepName, mediaType: mediaType(for: URL(fileURLWithPath: session.outputPath)), capturedAt: capturedAt - )) + ) + try validateVideoArtifact(artifact, phase: build.phase) + result.artifacts.append(artifact) } } } defer { - finishActiveVideo() + try? finishActiveVideo() try? simulator.terminate(bundleID: bundleID, selection: selection) } @@ -487,14 +510,16 @@ public struct SimctlPlanExecutor: EvidencePlanExecuting { if shouldRecordWholeFlow, step == steps.last, let session = activeVideo { try videoRecorder.stop(session) activeVideo = nil - result.artifacts.append(artifactWriter.artifact( + let artifact = artifactWriter.artifact( kind: .video, phase: build.phase, path: session.outputPath, stepName: request.plan.video.name?.nonEmpty ?? "flow", mediaType: mediaType(for: URL(fileURLWithPath: session.outputPath)), capturedAt: completedAt - )) + ) + try validateVideoArtifact(artifact, phase: build.phase) + result.artifacts.append(artifact) } result.stepResults.append(CaptureStepResult( phase: build.phase, @@ -515,18 +540,20 @@ public struct SimctlPlanExecutor: EvidencePlanExecuting { capturedAt: completedAt )) } else if step.kind == .stopVideo, let artifactPath { - result.artifacts.append(artifactWriter.artifact( + let artifact = artifactWriter.artifact( kind: .video, phase: build.phase, path: artifactPath, stepName: step.name, mediaType: mediaType(for: URL(fileURLWithPath: artifactPath)), capturedAt: completedAt - )) + ) + try validateVideoArtifact(artifact, phase: build.phase) + result.artifacts.append(artifact) } } catch let error as CLIError { let completedAt = timestamp() - finishActiveVideo(recordingStepName: step.name, capturedAt: completedAt) + try? finishActiveVideo(recordingStepName: step.name, capturedAt: completedAt) let message = "\(build.phase.rawValue) step '\(step.name)' failed. \(error.errorDescription ?? String(describing: error))" result.failures.append(PRChangeEvidenceFailureSummary(stepName: step.name, message: message)) result.stepResults.append(CaptureStepResult( @@ -541,7 +568,7 @@ public struct SimctlPlanExecutor: EvidencePlanExecuting { return result } catch { let completedAt = timestamp() - finishActiveVideo(recordingStepName: step.name, capturedAt: completedAt) + try? finishActiveVideo(recordingStepName: step.name, capturedAt: completedAt) let message = "\(build.phase.rawValue) step '\(step.name)' failed. \(String(describing: error))" result.failures.append(PRChangeEvidenceFailureSummary(stepName: step.name, message: message)) result.stepResults.append(CaptureStepResult( @@ -769,6 +796,16 @@ private func mediaType(for url: URL) -> String { } } +private func validateVideoArtifact( + _ artifact: CapturedArtifact, + phase: PRChangeEvidencePhase +) throws { + guard artifact.kind == .video else { return } + guard let fileSize = artifact.fileSize, fileSize > 0 else { + throw CLIError.commandFailed("\(phase.rawValue) video artifact missing or empty: \(artifact.path)") + } +} + private func fileSafeName(for name: String) -> String { let allowed = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: "-_")) let scalars = name.unicodeScalars.map { scalar in diff --git a/Sources/EvidenceCLIKit/PullRequestEvidenceReport.swift b/Sources/EvidenceCLIKit/PullRequestEvidenceReport.swift index 7ffb451..2fce69c 100644 --- a/Sources/EvidenceCLIKit/PullRequestEvidenceReport.swift +++ b/Sources/EvidenceCLIKit/PullRequestEvidenceReport.swift @@ -547,7 +547,7 @@ public struct RenderPullRequestEvidenceReport: PullRequestEvidenceReporting { outputDirectory: URL ) -> [ReportFailureSection] { var sections: [ReportFailureSection] = [] - let mediaSteps = plan.steps.filter { $0.kind == .screenshot || $0.kind == .stopVideo } + let mediaSteps = expectedMediaSteps(in: plan) let missingBefore = missingArtifacts( phase: .before, @@ -740,7 +740,7 @@ public struct RenderPullRequestEvidenceReport: PullRequestEvidenceReporting { plan: PRChangeEvidencePlan, manifest: PRChangeEvidenceManifest ) -> Bool { - for step in plan.steps where step.kind == .screenshot || step.kind == .stopVideo { + for step in expectedMediaSteps(in: plan) { let kind: CapturedArtifact.Kind = step.kind == .screenshot ? .screenshot : .video if artifact(kind: kind, phase: .before, stepName: step.name, manifest: manifest) == nil || artifact(kind: kind, phase: .after, stepName: step.name, manifest: manifest) == nil { @@ -750,6 +750,16 @@ public struct RenderPullRequestEvidenceReport: PullRequestEvidenceReporting { return false } + private func expectedMediaSteps(in plan: PRChangeEvidencePlan) -> [PRChangeEvidenceStep] { + var mediaSteps = plan.steps.filter { $0.kind == .screenshot || $0.kind == .stopVideo } + if plan.video.enabled, + !plan.steps.contains(where: { $0.kind == .startVideo || $0.kind == .stopVideo }) { + let name = plan.video.name?.nonEmpty ?? "flow" + mediaSteps.append(PRChangeEvidenceStep(name: name, kind: .stopVideo, path: "\(name).mov")) + } + return mediaSteps + } + private func shortSHA(_ sha: String) -> String { String(sha.prefix(7)) } diff --git a/Tests/EvidenceCLIKitTests/EvidenceCLIKitTests.swift b/Tests/EvidenceCLIKitTests/EvidenceCLIKitTests.swift index ad6cb83..2aa4f75 100644 --- a/Tests/EvidenceCLIKitTests/EvidenceCLIKitTests.swift +++ b/Tests/EvidenceCLIKitTests/EvidenceCLIKitTests.swift @@ -4,6 +4,26 @@ import Foundation import XCTest final class EvidenceCLIKitTests: XCTestCase { + func testProcessCommandRunnerDrainsLargeStdoutAndStderrWhileProcessRuns() throws { + try XCTSkipUnless( + FileManager.default.isExecutableFile(atPath: "/usr/bin/python3"), + "python3 is not available" + ) + let script = """ + import sys + sys.stdout.write("out" * 100000) + sys.stdout.flush() + sys.stderr.write("err" * 100000) + sys.stderr.flush() + """ + + let result = try ProcessCommandRunner().run("/usr/bin/python3", ["-c", script]) + + XCTAssertEqual(result.exitCode, 0) + XCTAssertEqual(result.stdout.count, 300_000) + XCTAssertEqual(result.stderr.count, 300_000) + } + func testConfigParsingRequiresNamedFields() throws { let document = try TOMLDocument.parse(""" scheme = "Example" diff --git a/Tests/EvidenceCLIKitTests/IOSRevisionAdaptersTests.swift b/Tests/EvidenceCLIKitTests/IOSRevisionAdaptersTests.swift index 5f435f2..9fa5f78 100644 --- a/Tests/EvidenceCLIKitTests/IOSRevisionAdaptersTests.swift +++ b/Tests/EvidenceCLIKitTests/IOSRevisionAdaptersTests.swift @@ -314,6 +314,58 @@ final class IOSRevisionAdaptersTests: XCTestCase { ]) } + func testSimctlPlanExecutorFailsWhenDeclaredWholeFlowVideoIsMissing() throws { + let directory = try temporaryDirectory() + let output = directory.appendingPathComponent("proof/pr-55", isDirectory: true) + let plan = PRChangeEvidencePlan( + repo: "RiddimSoftware/example", + pr: 55, + platform: .ios, + runner: .simctl, + ios: PRChangeEvidenceIOSSettings( + scheme: "Example", + bundleID: "com.example.app", + simulatorUDID: "SIM-123" + ), + steps: [ + PRChangeEvidenceStep(name: "launch app", kind: .launch), + PRChangeEvidenceStep(name: "settle", kind: .wait, seconds: 0), + PRChangeEvidenceStep(name: "capture home", kind: .screenshot, path: "home.png") + ], + video: PRChangeEvidenceVideo(enabled: true, name: "home-flow") + ) + let simulator = FakeSimulatorController() + let videoRecorder = FakeVideoRecorder(missingOutputPhases: [.after]) + let executor = SimctlPlanExecutor( + simulator: simulator, + videoRecorder: videoRecorder, + artifactWriter: FileArtifactWriter(), + clock: IOSFixedEvidenceClock(date: Date(timeIntervalSince1970: 1_714_000_000)) + ) + + let result = try executor.execute(EvidencePlanExecutionRequest( + plan: plan, + planURL: directory.appendingPathComponent(".evidence/pr-home.json"), + outputDirectory: output, + worktrees: [ + ComparisonWorktree(label: .before, sha: "before", path: directory.appendingPathComponent("before-worktree").path), + ComparisonWorktree(label: .after, sha: "after", path: directory.appendingPathComponent("after-worktree").path) + ], + revisionBuilds: [ + revisionBuild(.before, output: output), + revisionBuild(.after, output: output) + ], + ios: plan.ios!, + launch: plan.launch + )) + + XCTAssertFalse(result.succeeded) + XCTAssertEqual(result.artifacts.filter { $0.kind == .video }.map(\.phase), [.before]) + XCTAssertTrue(result.failures.contains { $0.message.contains("after") && $0.message.contains("home-flow.mov") }) + XCTAssertEqual(result.stepResults.last?.phase, .after) + XCTAssertEqual(result.stepResults.last?.status, .failed) + } + func testCapturePRBuildsBothRevisionsWithIsolatedDerivedDataAndManifestRecords() throws { let directory = try temporaryDirectory() let output = directory.appendingPathComponent("proof/pr-44", isDirectory: true) @@ -860,6 +912,11 @@ private final class FakeSimulatorController: SimulatorControlling { private final class FakeVideoRecorder: VideoRecording { private(set) var startedPaths: [String] = [] private(set) var stoppedPaths: [String] = [] + var missingOutputPhases: Set + + init(missingOutputPhases: Set = []) { + self.missingOutputPhases = missingOutputPhases + } func start(udid: String, outputURL: URL) throws -> VideoRecordingSession { startedPaths.append(outputURL.path) @@ -868,8 +925,15 @@ private final class FakeVideoRecorder: VideoRecording { func stop(_ session: VideoRecordingSession) throws { stoppedPaths.append(session.outputPath) + if missingOutputPhases.contains(phase(forArtifactPath: session.outputPath)) { + return + } let url = URL(fileURLWithPath: session.outputPath) try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true) try Data("mov".utf8).write(to: url) } + + private func phase(forArtifactPath path: String) -> PRChangeEvidencePhase { + path.contains("/after/") ? .after : .before + } } diff --git a/Tests/EvidenceCLIKitTests/PullRequestEvidenceReportTests.swift b/Tests/EvidenceCLIKitTests/PullRequestEvidenceReportTests.swift index 82530d1..fe2d00c 100644 --- a/Tests/EvidenceCLIKitTests/PullRequestEvidenceReportTests.swift +++ b/Tests/EvidenceCLIKitTests/PullRequestEvidenceReportTests.swift @@ -113,6 +113,30 @@ final class PullRequestEvidenceReportTests: XCTestCase { XCTAssertTrue(report.markdown.contains("- Overall status: **failed**")) } + func testRendererNamesMissingWholeFlowVideoWhenPlanVideoIsEnabled() throws { + let output = try outputDirectory() + var manifest = successfulManifest(output: output) + manifest.artifacts = manifest.artifacts.filter { artifact in + artifact.kind != .video || artifact.phase == .before + } + + var plan = comparisonPlan() + plan.video = PRChangeEvidenceVideo(enabled: true, name: "home-flow") + plan.steps = [ + PRChangeEvidenceStep(name: "Launch app", kind: .launch), + PRChangeEvidenceStep(name: "Home screen", kind: .screenshot, path: "home.png") + ] + + let report = try RenderPullRequestEvidenceReport( + comparisonRenderer: RecordingComparisonImageRenderer(), + fileManager: .default + ).writeReport(manifest: manifest, plan: plan, outputDirectory: output) + + XCTAssertTrue(report.markdown.contains("### Missing After Artifact")) + XCTAssertTrue(report.markdown.contains("home-flow: expected `after/home-flow.mov`")) + XCTAssertTrue(report.markdown.contains("- Overall status: **partial**")) + } + func testRendererNamesBuildFailureAndReportOnlyPartialOutputWithoutRawLogs() throws { let output = try outputDirectory() var manifest = successfulManifest(output: output) diff --git a/docs/architecture/use-case-catalog.md b/docs/architecture/use-case-catalog.md index 51bfdf8..fa8def3 100644 --- a/docs/architecture/use-case-catalog.md +++ b/docs/architecture/use-case-catalog.md @@ -146,9 +146,11 @@ screenshot/openURL adapter, simctl video recorder, filesystem artifact writer. Current implementation: `Sources/EvidenceCLIKit/EvidencePlanExecution.swift` dispatches `runner = "xctest"` to revision-scoped `xcodebuild test` calls with Evidence environment values, and `runner = "simctl"` to launch/wait/screenshot -and video steps. `CapturePullRequestEvidence` records returned artifacts and -step results in `manifest.json`; `EvidenceCLI.swift` remains a thin command -router. +and video steps. Video artifacts are validated for presence and non-zero file +size before they are added to the manifest; missing or empty recordings are +returned as capture failures. `CapturePullRequestEvidence` records returned +artifacts and step results in `manifest.json`; `EvidenceCLI.swift` remains a +thin command router. ### RenderPullRequestEvidenceReport