From 4f7be4fd638d7192131468b3df59b01adb2c4eb2 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 21 Nov 2024 16:45:23 +0900 Subject: [PATCH] Fix test output parser to be incremental and show failed tests (#505) --- Sources/CartonHelpers/Process+run.swift | 117 +++---------- .../CartonKit/Parsers/DiagnosticsParser.swift | 15 -- .../CartonFrontendTestCommand.swift | 7 +- .../TestRunners/CommandTestRunner.swift | 2 +- .../TestRunners/JavaScriptTestRunner.swift | 3 +- .../TestRunners/TestRunner.swift | 61 +++++++ .../TestRunners/TestsParser.swift | 163 +++++++++++------- 7 files changed, 189 insertions(+), 179 deletions(-) delete mode 100644 Sources/CartonKit/Parsers/DiagnosticsParser.swift diff --git a/Sources/CartonHelpers/Process+run.swift b/Sources/CartonHelpers/Process+run.swift index 2c073373..81bf4b8a 100644 --- a/Sources/CartonHelpers/Process+run.swift +++ b/Sources/CartonHelpers/Process+run.swift @@ -17,21 +17,12 @@ import Dispatch import Foundation struct ProcessError: Error { - let stderr: String? - let stdout: String? + let exitCode: Int32 } extension ProcessError: CustomStringConvertible { var description: String { - var result = "Process failed with non-zero exit status" - if let stdout = stdout { - result += " and following output:\n\(stdout)" - } - - if let stderr = stderr { - result += " and following error output:\n\(stderr)" - } - return result + return "Process failed with exit code \(exitCode)" } } @@ -41,11 +32,10 @@ extension Foundation.Process { _ arguments: [String], environment: [String: String] = [:], loadingMessage: String = "Running...", - parser: ProcessOutputParser? = nil, _ terminal: InteractiveWriter ) async throws { terminal.clearLine() - terminal.write("\(loadingMessage)\n", inColor: .yellow) + terminal.write("Running \(arguments.joined(separator: " "))\n") if !environment.isEmpty { terminal.write(environment.map { "\($0)=\($1)" }.joined(separator: " ") + " ") @@ -54,91 +44,26 @@ extension Foundation.Process { let processName = URL(fileURLWithPath: arguments[0]).lastPathComponent do { - try await withCheckedThrowingContinuation { - (continuation: CheckedContinuation<(), Swift.Error>) in - DispatchQueue.global().async { - var stdoutBuffer = "" - - let stdout: Process.OutputClosure = { - guard let string = String(data: Data($0), encoding: .utf8) else { return } - if parser != nil { - // Aggregate this for formatting later - stdoutBuffer += string - } else { - terminal.write(string) - } - } - - var stderrBuffer = [UInt8]() - - let stderr: Process.OutputClosure = { - stderrBuffer.append(contentsOf: $0) - } - - let process = Process( - arguments: arguments, - environmentBlock: ProcessEnvironmentBlock( - ProcessInfo.processInfo.environment.merging(environment) { _, new in new } - ), - outputRedirection: .stream(stdout: stdout, stderr: stderr), - startNewProcessGroup: true, - loggingHandler: { - terminal.write($0 + "\n") - } - ) - - let result = Result { - try process.launch() - return try process.waitUntilExit() - } - - switch result.map(\.exitStatus) { - case .success(.terminated(code: EXIT_SUCCESS)): - if let parser = parser { - if parser.parsingConditions.contains(.success) { - parser.parse(stdoutBuffer, terminal) - } - } else { - terminal.write(stdoutBuffer) - } - terminal.write( - "`\(processName)` process finished successfully\n", - inColor: .green, - bold: false - ) - continuation.resume() - - case let .failure(error): - continuation.resume(throwing: error) - default: - continuation.resume( - throwing: ProcessError( - stderr: String(data: Data(stderrBuffer), encoding: .utf8) ?? "", - stdout: stdoutBuffer - ) - ) - } + try await Process.checkNonZeroExit( + arguments: arguments, + environmentBlock: ProcessEnvironmentBlock( + ProcessInfo.processInfo.environment.merging(environment) { _, new in new } + ), + loggingHandler: { + terminal.write($0 + "\n") } - } + ) + terminal.write( + "`\(processName)` process finished successfully\n", + inColor: .green, + bold: false + ) } catch { - let errorString = String(describing: error) - if errorString.isEmpty { - terminal.clearLine() - terminal.write( - "\(processName) process failed.\n\n", - inColor: .red - ) - if let error = error as? ProcessError, let stdout = error.stdout { - if let parser = parser { - if parser.parsingConditions.contains(.failure) { - parser.parse(stdout, terminal) - } - } else { - terminal.write(stdout) - } - } - } - + terminal.clearLine() + terminal.write( + "\(processName) process failed.\n\n", + inColor: .red + ) throw error } } diff --git a/Sources/CartonKit/Parsers/DiagnosticsParser.swift b/Sources/CartonKit/Parsers/DiagnosticsParser.swift deleted file mode 100644 index a0097fc2..00000000 --- a/Sources/CartonKit/Parsers/DiagnosticsParser.swift +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2020 Carton contributors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import Foundation diff --git a/Sources/carton-frontend-slim/CartonFrontendTestCommand.swift b/Sources/carton-frontend-slim/CartonFrontendTestCommand.swift index 8f66bfc5..9a607950 100644 --- a/Sources/carton-frontend-slim/CartonFrontendTestCommand.swift +++ b/Sources/carton-frontend-slim/CartonFrontendTestCommand.swift @@ -75,6 +75,9 @@ struct CartonFrontendTestCommand: AsyncParsableCommand { @Flag(help: "When running browser tests, run the browser in headless mode") var headless: Bool = false + @Flag(help: "Enable verbose output") + var verbose: Bool = false + @Option(help: "Turn on runtime checks for various behavior.") private var sanitize: SanitizeVariant? @@ -195,6 +198,8 @@ struct CartonFrontendTestCommand: AsyncParsableCommand { env[key] = parentEnv[key] } } - return TestRunnerOptions(env: env, listTestCases: list, testCases: testCases) + return TestRunnerOptions( + env: env, listTestCases: list, testCases: testCases, + testsParser: verbose ? RawTestsParser() : FancyTestsParser()) } } diff --git a/Sources/carton-frontend-slim/TestRunners/CommandTestRunner.swift b/Sources/carton-frontend-slim/TestRunners/CommandTestRunner.swift index 6e4f953d..697cc1d9 100644 --- a/Sources/carton-frontend-slim/TestRunners/CommandTestRunner.swift +++ b/Sources/carton-frontend-slim/TestRunners/CommandTestRunner.swift @@ -49,7 +49,7 @@ struct CommandTestRunner: TestRunner { } arguments += [testFilePath.pathString] + xctestArgs - try await Process.run(arguments, parser: TestsParser(), terminal) + try await runTestProcess(arguments, parser: options.testsParser, terminal) } func defaultWASIRuntime() throws -> String { diff --git a/Sources/carton-frontend-slim/TestRunners/JavaScriptTestRunner.swift b/Sources/carton-frontend-slim/TestRunners/JavaScriptTestRunner.swift index ba41b67d..c5326a5b 100644 --- a/Sources/carton-frontend-slim/TestRunners/JavaScriptTestRunner.swift +++ b/Sources/carton-frontend-slim/TestRunners/JavaScriptTestRunner.swift @@ -58,6 +58,7 @@ struct JavaScriptTestRunner: TestRunner { var arguments = ["node"] + nodeArguments + [pluginWorkDirectory.appending(component: testHarness).pathString] options.applyXCTestArguments(to: &arguments) - try await Process.run(arguments, environment: options.env, parser: TestsParser(), terminal) + try await runTestProcess( + arguments, environment: options.env, parser: options.testsParser, terminal) } } diff --git a/Sources/carton-frontend-slim/TestRunners/TestRunner.swift b/Sources/carton-frontend-slim/TestRunners/TestRunner.swift index 1ac940ef..befcbc5b 100644 --- a/Sources/carton-frontend-slim/TestRunners/TestRunner.swift +++ b/Sources/carton-frontend-slim/TestRunners/TestRunner.swift @@ -12,6 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +import CartonCore +import CartonHelpers +import Foundation + struct TestRunnerOptions { /// The environment variables to pass to the test process. let env: [String: String] @@ -19,6 +23,8 @@ struct TestRunnerOptions { let listTestCases: Bool /// Filter the test cases to run. let testCases: [String] + /// The parser to use for the test output. + let testsParser: any TestsParser func applyXCTestArguments(to arguments: inout [String]) { if listTestCases { @@ -32,3 +38,58 @@ struct TestRunnerOptions { protocol TestRunner { func run(options: TestRunnerOptions) async throws } + +struct LineStream { + var buffer: String = "" + let onLine: (String) -> Void + + mutating func feed(_ bytes: [UInt8]) { + buffer += String(decoding: bytes, as: UTF8.self) + while let newlineIndex = buffer.firstIndex(of: "\n") { + let line = buffer[..( + _ lines: Lines, _ terminal: InteractiveWriter + ) async throws where Lines: AsyncSequence, Lines.Element == String +} + +struct RawTestsParser: TestsParser { + func parse( + _ lines: Lines, _ terminal: InteractiveWriter + ) async throws where Lines: AsyncSequence, Lines.Element == String { + for try await line in lines { + terminal.write(line + "\n") + } + } +} + /// Parses and re-formats diagnostics output by the Swift compiler. /// /// The compiler output often repeats itself, and the diagnostics can sometimes be @@ -44,7 +61,7 @@ struct DiagnosticsParser { } extension String.StringInterpolation { - fileprivate mutating func appendInterpolation(_ regexLabel: TestsParser.Regex.Label) { + fileprivate mutating func appendInterpolation(_ regexLabel: FancyTestsParser.Regex.Label) { appendInterpolation("<\(regexLabel.rawValue)>") } } @@ -52,12 +69,14 @@ extension String.StringInterpolation { extension StringProtocol { fileprivate func range( of regex: NSRegularExpression, - labelled label: TestsParser.Regex.Label + labelled label: FancyTestsParser.Regex.Label ) -> Range? { range(of: regex, named: label.rawValue) } - fileprivate func match(of regex: NSRegularExpression, labelled label: TestsParser.Regex.Label) + fileprivate func match( + of regex: NSRegularExpression, labelled label: FancyTestsParser.Regex.Label + ) -> String .SubSequence? { @@ -66,8 +85,8 @@ extension StringProtocol { fileprivate func match( of regex: NSRegularExpression, - labelled labelA: TestsParser.Regex.Label, - _ labelB: TestsParser.Regex.Label + labelled labelA: FancyTestsParser.Regex.Label, + _ labelB: FancyTestsParser.Regex.Label ) -> (String.SubSequence, String.SubSequence)? { guard let a = match(of: regex, named: labelA.rawValue), let b = match(of: regex, named: labelB.rawValue) @@ -78,7 +97,7 @@ extension StringProtocol { } } -public struct TestsParser: ProcessOutputParser { +public struct FancyTestsParser: TestsParser { public init() {} public let parsingConditions: ParsingCondition = [.success, .failure] @@ -199,9 +218,9 @@ public struct TestsParser: ProcessOutputParser { } } - public func parse(_ output: String, _ terminal: InteractiveWriter) { - let lines = output.split(separator: "\n") - + public func parse( + _ lines: Lines, _ terminal: InteractiveWriter + ) async throws where Lines: AsyncSequence, Lines.Element == String { var suites = [Suite]() var unmappedProblems = [ ( @@ -211,8 +230,11 @@ public struct TestsParser: ProcessOutputParser { ) ]() - for line in lines { + for try await line in lines { if let suite = line.match(of: Regex.suiteStarted, labelled: .suite) { + if let lastSuite = suites.last { + flushSingleSuite(lastSuite, terminal) + } suites.append(.init(name: suite, cases: [])) } else if let testCase = line.match(of: Regex.caseFinished, labelled: .testCase), let suite = line.match(of: Regex.caseFinished, labelled: .suite), @@ -255,75 +277,73 @@ public struct TestsParser: ProcessOutputParser { } } - flushSuites(suites, terminal) + if let lastSuite = suites.last { + flushSingleSuite(lastSuite, terminal) + } + terminal.write("\n") flushSummary(of: suites, terminal) } - func flushSuites(_ suites: [Suite], _ terminal: InteractiveWriter) { - let suitesWithCases = suites.filter { $0.cases.count > 0 } - + func flushSingleSuite(_ suite: Suite, _ terminal: InteractiveWriter) { // Keep track of files we already opened and store their contents struct FileBuf: Hashable { let path: String let contents: String } var fileBufs = Set() - - for suite in suitesWithCases { - // bold, white fg, green/red bg - terminal - .write( - """ - \n\(" \(suite.passed ? "PASSED" : "FAILED") ", + // bold, white fg, green/red bg + terminal + .write( + """ + \n\(" \(suite.passed ? "PASSED" : "FAILED") ", color: "[1m", "[97m", suite.passed ? "[42m" : "[101m" ) - """ - ) - terminal.write(" \(suite.name)\n") - for testCase in suite.cases { - if testCase.passed { - terminal.write(" \("✓", color: "[92m") ") // green - } else { - terminal.write(" \("✕", color: "[91m") ") // red + """ + ) + terminal.write(" \(suite.name)\n") + for testCase in suite.cases { + if testCase.passed { + terminal.write(" \("✓", color: "[92m") ") // green + } else { + terminal.write(" \("✕", color: "[91m") ") // red + } + terminal + .write( + "\(testCase.name) \("(\(Int(Double(testCase.duration)! * 1000))ms)", color: "[90m")\n" + ) // gray + for problem in testCase.problems { + terminal.write("\n \(problem.file, color: "[90m"):\(problem.line)\n") + terminal.write(" \(problem.message)\n\n") + // Format XCTAssert functions + for assertion in Regex.Assertion.allCases { + if let (expected, received) = problem.message.match( + of: Regex.xctAssert(assertion), + labelled: .expected, .received + ) { + terminal.write(" Expected: \("\(assertion.symbol)\(expected)", color: "[92m")\n") + terminal.write(" Received: \(received, color: "[91m")\n") + } } - terminal - .write( - "\(testCase.name) \("(\(Int(Double(testCase.duration)! * 1000))ms)", color: "[90m")\n" - ) // gray - for problem in testCase.problems { - terminal.write("\n \(problem.file, color: "[90m"):\(problem.line)\n") - terminal.write(" \(problem.message)\n\n") - // Format XCTAssert functions - for assertion in Regex.Assertion.allCases { - if let (expected, received) = problem.message.match( - of: Regex.xctAssert(assertion), - labelled: .expected, .received - ) { - terminal.write(" Expected: \("\(assertion.symbol)\(expected)", color: "[92m")\n") - terminal.write(" Received: \(received, color: "[91m")\n") - } + // Get the line of code from the file and output it for context. + if let lineNum = Int(problem.line), + lineNum > 0 + { + var fileContents: String? + if let fileBuf = fileBufs.first(where: { $0.path == problem.file })?.contents { + fileContents = fileBuf + } else if let fileBuf = try? String( + contentsOf: URL(fileURLWithPath: problem.file), + encoding: .utf8 + ) { + fileContents = fileBuf + fileBufs.insert(.init(path: problem.file, contents: fileBuf)) } - // Get the line of code from the file and output it for context. - if let lineNum = Int(problem.line), - lineNum > 0 - { - var fileContents: String? - if let fileBuf = fileBufs.first(where: { $0.path == problem.file })?.contents { - fileContents = fileBuf - } else if let fileBuf = try? String( - contentsOf: URL(fileURLWithPath: problem.file), - encoding: .utf8 - ) { - fileContents = fileBuf - fileBufs.insert(.init(path: problem.file, contents: fileBuf)) - } - if let fileContents = fileContents { - let fileLines = fileContents.components(separatedBy: .newlines) - guard fileLines.count >= lineNum else { break } - let codeLine = String(fileLines[lineNum - 1]) - terminal.write(" \("\(problem.line) | ", color: "[36m")\(codeLine)\n") - } + if let fileContents = fileContents { + let fileLines = fileContents.components(separatedBy: .newlines) + guard fileLines.count >= lineNum else { break } + let codeLine = String(fileLines[lineNum - 1]) + terminal.write(" \("\(problem.line) | ", color: "[36m")\(codeLine)\n") } } } @@ -361,5 +381,18 @@ public struct TestsParser: ProcessOutputParser { if suites.contains(where: { $0.name == "All tests" }) { terminal.write("\("Ran all test suites.", color: "[90m")\n") // gray } + + if suites.contains(where: { !$0.passed }) { + terminal.write("\n\("Failed test cases:", color: "[31m")\n") + for suite in suites.filter({ !$0.passed }) { + for testCase in suite.cases.filter({ !$0.passed }) { + terminal.write(" \("✕", color: "[91m") \(suite.name).\(testCase.name)\n") + } + } + + terminal.write( + "\n\("Some tests failed. Use --verbose for raw test output.", color: "[33m")\n" + ) + } } }