Merge pull request #27 from GNMoseke/refactor/cleanup-and-sig-handle

Refactor/cleanup and sig handle

.github/workflows/test.yml

@@ -6,12 +6,11 @@ on:
 
 jobs:
   lint:
-    name: Check Formatting
-    runs-on: ubuntu-latest
+    runs-on: macos-latest
     steps:
       - uses: actions/checkout@v3
-      - name: SwiftFormat lint
-        run: swift package plugin --allow-writing-to-package-directory swiftformat --lint
+      - name: SwiftFormat
+        run: swiftformat --lint . --reporter github-actions-log
 
   test-macos:
     name: Test macOS using Xcode latest-stable

Package.resolved

@@ -62,15 +62,6 @@
         "revision" : "4a0ea7695a17c4e4053e15814110b5086c3c2ceb",
         "version" : "1.4.1"
       }
-    },
-    {
-      "identity" : "swiftformat",
-      "kind" : "remoteSourceControl",
-      "location" : "https://github.com/nicklockwood/SwiftFormat",
-      "state" : {
-        "revision" : "dd989a46d0c6f15c016484bab8afe5e7a67a4022",
-        "version" : "0.54.0"
-      }
     }
   ],
   "version" : 2

Package.swift

@@ -14,7 +14,6 @@ let package = Package(
             url: "https://github.com/Zollerboy1/SwiftCommand.git",
             from: "1.2.0"
         ),
-        .package(url: "https://github.com/nicklockwood/SwiftFormat", from: "0.53.9"),
     ],
     targets: [
         .executableTarget(

README.md

@@ -9,7 +9,8 @@ test failures, quickly. It includes things like:
 - Listing the longest tests by execution time in your project, including outputting the results to a csv file for tracking purposes
 - Easily invoking tests with different toolchains
 
-The screenshot below shows a side-by-side comparison of the output of `swift test` and `peregrine run`, against the excellent [Units package](https://github.com/NeedleInAJayStack/Units):
+The screenshot below shows a side-by-side comparison of the output of `swift test` and `peregrine run`, against the excellent [Units
+package](https://github.com/NeedleInAJayStack/Units):
 
 ![output-comparison](./assets/test-screenshot.png)
 
@@ -24,8 +25,10 @@ peregrine --help
 > 2. Pass the `--plain` flag to `peregrine run` for standard ascii-only output
 
 > [!WARNING]
-> peregrine does a lot of string parsing, and is heavily reliant on the output format of [swift-package-manager](https://github.com/apple/swift-package-manager).
-> It is meant as a local development convenience tool and should be used as such. I'm sure there are bugs - contributions are welcome! See the "Why?" section below for more details.
+> peregrine does a lot of string parsing, and is heavily reliant on the output format of
+> [swift-package-manager](https://github.com/apple/swift-package-manager) and `XCTest`.
+> It is meant as a local development convenience tool and should be used as such. I'm sure there are bugs - contributions are welcome! See
+> the "Why?" section below for more details.
 
 ## Installing
 > It is assumed that you have [swift](https://www.swift.org/install/) installed, since peregrine is a tool for running swift tests.
@@ -34,16 +37,21 @@ peregrine --help
 3. If you wish to uninstall, simply run `./uninstall.sh`
 
 ## Contributing
-PRs are always welcome! There is a lot of work going on in the swift testing space, including the new [swift-testing library](https://github.com/apple/swift-testing), which looks to solve many of the same output issues that pushed me to create peregrine in the first place.
+PRs are always welcome! There is a lot of work going on in the swift testing space, including the new [swift-testing
+library](https://github.com/apple/swift-testing), which looks to solve many of the same output issues that pushed me to create peregrine in
+the first place.
 
 ## Known Issues
-- Passing through the spm `--filter` or `--skip` flags causes the progress bar to behave unexpectedly - this is due to these flags not being respected by `swift test list`. **Test success/failure output is unaffected, and will work with these flags.**
-- If peregrine crashes (or is terminated via something other than `SIGINT`, `SIGQUIT`, or `SIGSTOP`, the shell cursor may remain hidden. Run `tput cnorm` to fix
+- Passing through the spm `--filter` or `--skip` flags causes the progress bar to behave unexpectedly - this is due to these flags not being
+respected by `swift test list`. **Test success/failure output is unaffected, and will work with these flags.**
+- If peregrine crashes (or is terminated via something other than `SIGINT`, `SIGQUIT`, or `SIGSTOP`, the shell cursor may remain hidden. Run
+`tput cnorm` to fix
 
 ## Why?
 peregrine was born out of my personal need for some simple things in relation to swift testing - namely:
 1. Only showing failure output, since I was running a lot of large test suites and the output was incredibly noisy
 2. Being able to tell me which of my tests were the slowest, so that I could take a look at improving them.
 
-It is a tool meant for local developer convenience, as a prettified way of getting test output. Test infrastructure remains (in my opinion) one of the weakest
-parts of the swift insfrastructure, though as linked above the new [swift-testing library](https://github.com/apple/swift-testing) looks to solve many of these problems.
+It is a tool meant for local developer convenience, as a prettified way of getting test output. Test infrastructure remains (in my opinion)
+one of the weakest parts of the swift insfrastructure, though as linked above the new [swift-testing
+library](https://github.com/apple/swift-testing) looks to solve many of these problems.

Sources/OutputProcessing.swift

@@ -1,4 +1,3 @@
-
 /// This relies heavily on the output format from swift test remaining the same, I'd like to parse xunit here
 /// but spm's xunit output doesn't give valuable information: https://github.com/apple/swift-package-manager/issues/7622
 func processOutput(testOutput: TestRunOutput, symbolOutput: SymbolOutput) throws -> (output: String, color: TextColor) {

Sources/Peregrine.swift

@@ -15,11 +15,13 @@ struct Peregrine: AsyncParsableCommand {
         discussion: """
         peregrine is a tool intended to clean up the often noisy output of swift-package-manager's `swift test` command.
         It is meant as a development conveneince tool to more quickly and easily find failures and pull some simple test 
-        statistics for large test suites. It is **NOT** a drop-in replacement for `swift test` - when debugging, it is still
+        statistics for large test suites. 
+
+        It is **NOT** a drop-in replacement for `swift test` - when debugging, it is still
         generally favorable to `swift test --filter fooTest` where applicable. peregrine is meant to help you find that
         `fooTest` is having issues in the first place.
         """,
-        version: "1.0.1",
+        version: "1.0.3",
         subcommands: [Run.self, CountTests.self],
         defaultSubcommand: Run.self
     )
@@ -40,6 +42,11 @@ struct Peregrine: AsyncParsableCommand {
         @Flag(help: "Supress toolchain information & progress output")
         var quiet: Bool = false
 
+        @Flag(
+            help: "Retain log files even on successful runs. By deafult log files will be removed for successful runs."
+        )
+        var keepLogs: Bool = false
+
         @Option(
             help: "Control Peregrine's log level. Default is 'info'. Options: [trace, verbose, debug, info, warning, error, critical]"
         )
@@ -76,24 +83,18 @@ extension Peregrine {
         mutating func run() async throws {
             // NOTE: This feels potentially like the incorrect way to handle this - didn't want to adopt the full
             // swift-server/lifecycle package
-            let sigIntHandler = DispatchSource.makeSignalSource(signal: SIGINT, queue: .global())
-            sigIntHandler.setEventHandler {
+            signal(SIGINT) { _ in
                 tputCnorm()
                 Foundation.exit(SIGINT)
             }
-            sigIntHandler.resume()
-            let sigQuitHandler = DispatchSource.makeSignalSource(signal: SIGQUIT, queue: .global())
-            sigQuitHandler.setEventHandler {
+            signal(SIGQUIT) { _ in
                 tputCnorm()
                 Foundation.exit(SIGQUIT)
             }
-            sigQuitHandler.resume()
-            let sigStopHandler = DispatchSource.makeSignalSource(signal: SIGSTOP, queue: .global())
-            sigStopHandler.setEventHandler {
+            signal(SIGSTOP) { _ in
                 tputCnorm()
                 Foundation.exit(SIGSTOP)
             }
-            sigStopHandler.resume()
 
             let logger = try configureLogging(options.logLevel)
             logger.info("Executing Tests")
@@ -125,12 +126,12 @@ extension Peregrine {
             let testRunner = PeregrineRunner(options: testOptions, logger: logger)
             try await handle {
                 let tests = try await testRunner.listTests()
-                let testResults = try await testRunner.runTests(tests: tests)
+                let testResults = try await testRunner.runTests(testCount: tests.count)
                 try testRunner.output(results: testResults)
             }
 
             // only cleanup on fully successful run
-            try cleanupLogFile(logger: logger)
+            if !options.keepLogs { try cleanupLogFile(logger: logger) }
         }
 
         private func getSwiftVersion() throws -> String {

Sources/TestRunner.swift

@@ -74,14 +74,7 @@ struct TestResult {
     var duration: Duration
 }
 
-protocol TestRunner {
-    var options: TestOptions { get set }
-    func listTests() async throws -> [Test]
-    func runTests(tests: [Test]) async throws -> TestRunOutput
-    func output(results: TestRunOutput) throws
-}
-
-class PeregrineRunner: TestRunner {
+class PeregrineRunner {
     var options: TestOptions
     var testResults: [Test: TestResult] = [:]
 
@@ -150,7 +143,7 @@ class PeregrineRunner: TestRunner {
             if collectBuildFailure {
                 buildFailLines.append(line)
             }
-            if !collectBuildFailure && line.contains("error:") && line.contains(".swift") {
+            if !collectBuildFailure, line.contains("error:"), line.contains(".swift") {
                 logger.debug("Build failure found, collecting remaining stderr")
                 collectBuildFailure = true
             }
@@ -185,33 +178,25 @@ class PeregrineRunner: TestRunner {
         return tests
     }
 
-    func runTests(tests: [Test]) async throws -> TestRunOutput {
-        // TODO: the tests parameter here is somewhat confusing since it only gets used for couting the number being run
-        // The way to filter/skip is to pass the relevant flag via the passthrough option in peregrine, but that feels a
-        // little funky
-        // I'd like to refactor this to filter to the given test array in this function, but then there has to be some
-        // extra
-        // parsing done to see if --filter or --skip were included and respect them accordingly - `swift test list` does
-        // not use those flags
-        let testCount = tests.count == 0 ? 1 : tests.count
+    func runTests(testCount: Int) async throws -> TestRunOutput {
         guard
             let testProcess = try (
                 options.toolchainPath == nil ? Command
                     .findInPath(withName: "swift") : Command(executablePath: .init(options.toolchainPath!))
             )?
                 .addArguments(["test", "--package-path", options.packagePath])
                 .addArguments(options.additionalSwiftFlags)
-                .setStdout(.pipe) // swift build diagnostics go to stder
-                .setStderr(.pipe)
+                .setStdout(.pipe)
+                .setStderr(.pipe) // swift build diagnostics go to stder
                 .spawn()
         else {
             throw PeregrineError.couldNotFindSwiftExecutable
         }
 
         let progressBarCharacterLength = 45
-        let stepSize: Int = testCount < progressBarCharacterLength ? progressBarCharacterLength / testCount :
-            testCount /
-            progressBarCharacterLength
+        let stepSize: Int = testCount < progressBarCharacterLength ? progressBarCharacterLength /
+            ((testCount == 0) ? 1 : 0) :
+            testCount / progressBarCharacterLength
         var completeTests = 0
         var progressBar = String(
             repeating: options.symbolOutput.getSymbol(.LightlyShadedBlock),

Tests/PeregrineTests/PeregrineTests.swift

@@ -52,7 +52,7 @@ class PeregrineTests: XCTestCase {
             quietOutput: true,
             additionalSwiftFlags: ["--filter", "SuiteOne/testSuccess", "--filter", "SuiteTwo/testSuccess"]
         )
-        let output = try await runner.runTests(tests: [])
+        let output = try await runner.runTests(testCount: 0)
         XCTAssertTrue(output.success)
         XCTAssertNil(output.backtraceLines)
         XCTAssertTrue(output.results.map { $0.errors }.reduce([], +).isEmpty)
@@ -67,7 +67,7 @@ class PeregrineTests: XCTestCase {
             quietOutput: true,
             additionalSwiftFlags: ["--filter", "SuiteOne/testSingleFail"]
         )
-        var output = try await runner.runTests(tests: [])
+        var output = try await runner.runTests(testCount: 0)
         XCTAssertFalse(output.success)
         XCTAssertNil(output.backtraceLines)
         var expectedErrors = [#"XCTAssertEqual failed: ("Arthur Morgan") is not equal to ("Dutch Van Der Linde")"#]
@@ -83,7 +83,7 @@ class PeregrineTests: XCTestCase {
             additionalSwiftFlags: ["--filter", "SuiteOne/testCustomFailMessage"]
         )
         runner.testResults.removeAll()
-        output = try await runner.runTests(tests: [])
+        output = try await runner.runTests(testCount: 0)
         XCTAssertFalse(output.success)
         XCTAssertNil(output.backtraceLines)
         expectedErrors =
@@ -102,7 +102,7 @@ class PeregrineTests: XCTestCase {
             quietOutput: true,
             additionalSwiftFlags: ["--filter", "SuiteTwo"]
         )
-        let output = try await runner.runTests(tests: [])
+        let output = try await runner.runTests(testCount: 0)
         XCTAssertFalse(output.success)
         XCTAssertNil(output.backtraceLines)
         let expectedErrors = Set([
@@ -139,7 +139,7 @@ class PeregrineTests: XCTestCase {
                 "testSkippedWithReason",
             ]
         )
-        let output = try await runner.runTests(tests: [])
+        let output = try await runner.runTests(testCount: 0)
         XCTAssertFalse(output.success)
         XCTAssertNil(output.backtraceLines)
         let expectedErrors = Set([
@@ -173,7 +173,7 @@ class PeregrineTests: XCTestCase {
             quietOutput: true,
             additionalSwiftFlags: ["--filter", "SuiteThatCrashes"]
         )
-        let output = try await runner.runTests(tests: [])
+        let output = try await runner.runTests(testCount: 0)
         XCTAssertFalse(output.success)
         XCTAssertNotNil(output.backtraceLines)
     }
@@ -193,7 +193,7 @@ class PeregrineTests: XCTestCase {
                 "SuiteOne/testSkippedWithReason",
             ]
         )
-        let output = try await runner.runTests(tests: [])
+        let output = try await runner.runTests(testCount: 0)
         XCTAssertTrue(output.success)
         let expectedErrors = Set([
             "",