Fix Filehandler bugs & Swift 4 package comment. Groundwork for Async output

.travis.yml

@@ -1,8 +1,14 @@
-os: linux
+os:
+    - linux
+    - osx
+env:
+    - SWIFT_VERSION=4.0
+    - SWIFT_VERSION=4.1
+osx_image: xcode9
 language: generic
 sudo: required
 dist: trusty
 install:
-    - eval "$(curl -sL https://gist.githubusercontent.com/kylef/5c0475ff02b7c7671d2a/raw/9f442512a46d7a2af7b850d65a7e9bd31edfb09b/swiftenv-install.sh)"
+    - eval "$(curl -sL https://swiftenv.fuller.li/install.sh)"
 script:
     - swift test

Package.swift

@@ -1,4 +1,4 @@
-// swift-tools-version:4.1
+// swift-tools-version:4.0
 
 /**
  *  ShellOut

Sources/ShellOut.swift

@@ -15,9 +15,9 @@ import Dispatch
  *  - parameter command: The command to run
  *  - parameter arguments: The arguments to pass to the command
  *  - parameter path: The path to execute the commands at (defaults to current folder)
- *  - parameter outputHandle: Any `FileHandle` that any output (STDOUT) should be redirected to
+ *  - parameter outputHandle: Any `Handle` that any output (STDOUT) should be redirected to
  *              (at the moment this is only supported on macOS)
- *  - parameter errorHandle: Any `FileHandle` that any error output (STDERR) should be redirected to
+ *  - parameter errorHandle: Any `Handle` that any error output (STDERR) should be redirected to
  *              (at the moment this is only supported on macOS)
  *
  *  - returns: The output of running the command
@@ -29,8 +29,8 @@ import Dispatch
 @discardableResult public func shellOut(to command: String,
                                         arguments: [String] = [],
                                         at path: String = ".",
-                                        outputHandle: FileHandle? = nil,
-                                        errorHandle: FileHandle? = nil) throws -> String {
+                                        outputHandle: Handle? = nil,
+                                        errorHandle: Handle? = nil) throws -> String {
     let process = Process()
     let command = "cd \(path.escapingSpaces) && \(command) \(arguments.joined(separator: " "))"
     return try process.launchBash(with: command, outputHandle: outputHandle, errorHandle: errorHandle)
@@ -41,9 +41,9 @@ import Dispatch
  *
  *  - parameter commands: The commands to run
  *  - parameter path: The path to execute the commands at (defaults to current folder)
- *  - parameter outputHandle: Any `FileHandle` that any output (STDOUT) should be redirected to
+ *  - parameter outputHandle: Any `Handle` that any output (STDOUT) should be redirected to
  *              (at the moment this is only supported on macOS)
- *  - parameter errorHandle: Any `FileHandle` that any error output (STDERR) should be redirected to
+ *  - parameter errorHandle: Any `Handle` that any error output (STDERR) should be redirected to
  *              (at the moment this is only supported on macOS)
  *
  *  - returns: The output of running the command
@@ -54,8 +54,8 @@ import Dispatch
  */
 @discardableResult public func shellOut(to commands: [String],
                                         at path: String = ".",
-                                        outputHandle: FileHandle? = nil,
-                                        errorHandle: FileHandle? = nil) throws -> String {
+                                        outputHandle: Handle? = nil,
+                                        errorHandle: Handle? = nil) throws -> String {
     let command = commands.joined(separator: " && ")
     return try shellOut(to: command, at: path, outputHandle: outputHandle, errorHandle: errorHandle)
 }
@@ -65,8 +65,8 @@ import Dispatch
  *
  *  - parameter command: The command to run
  *  - parameter path: The path to execute the commands at (defaults to current folder)
- *  - parameter outputHandle: Any `FileHandle` that any output (STDOUT) should be redirected to
- *  - parameter errorHandle: Any `FileHandle` that any error output (STDERR) should be redirected to
+ *  - parameter outputHandle: Any `Handle` that any output (STDOUT) should be redirected to
+ *  - parameter errorHandle: Any `Handle` that any error output (STDERR) should be redirected to
  *
  *  - returns: The output of running the command
  *  - throws: `ShellOutError` in case the command couldn't be performed, or it returned an error
@@ -78,8 +78,8 @@ import Dispatch
  */
 @discardableResult public func shellOut(to command: ShellOutCommand,
                                         at path: String = ".",
-                                        outputHandle: FileHandle? = nil,
-                                        errorHandle: FileHandle? = nil) throws -> String {
+                                        outputHandle: Handle? = nil,
+                                        errorHandle: Handle? = nil) throws -> String {
     return try shellOut(to: command.string, at: path, outputHandle: outputHandle, errorHandle: errorHandle)
 }
 
@@ -343,10 +343,33 @@ extension ShellOutError: LocalizedError {
     }
 }
 
+/// Protocol adopted by objects that handles command output
+public protocol Handle {
+    /// Method called each time command provide new output data
+    func handle(data: Data)
+
+    /// Optional method called when command has finished to close the handle
+    func endHandling()
+}
+
+public extension Handle {
+    func endHandling() {}
+}
+
+extension FileHandle: Handle {
+    public func handle(data: Data) {
+        write(data)
+    }
+
+    public func endHandling() {
+        if shouldBeClosed { closeFile() }
+    }
+}
+
 // MARK: - Private
 
 private extension Process {
-    @discardableResult func launchBash(with command: String, outputHandle: FileHandle? = nil, errorHandle: FileHandle? = nil) throws -> String {
+    @discardableResult func launchBash(with command: String, outputHandle: Handle? = nil, errorHandle: Handle? = nil) throws -> String {
         launchPath = "/bin/bash"
         arguments = ["-c", command]
 
@@ -367,18 +390,18 @@ private extension Process {
 
         #if !os(Linux)
         outputPipe.fileHandleForReading.readabilityHandler = { handler in
+            let data = handler.availableData
             outputQueue.async {
-                let data = handler.availableData
                 outputData.append(data)
-                outputHandle?.write(data)
+                outputHandle?.handle(data: data)
             }
         }
 
         errorPipe.fileHandleForReading.readabilityHandler = { handler in
+            let data = handler.availableData
             outputQueue.async {
-                let data = handler.availableData
                 errorData.append(data)
-                errorHandle?.write(data)
+                errorHandle?.handle(data: data)
             }
         }
         #endif
@@ -394,8 +417,8 @@ private extension Process {
 
         waitUntilExit()
 
-        outputHandle?.closeFile()
-        errorHandle?.closeFile()
+        outputHandle?.endHandling()
+        errorHandle?.endHandling()
 
         #if !os(Linux)
         outputPipe.fileHandleForReading.readabilityHandler = nil
@@ -418,6 +441,14 @@ private extension Process {
     }
 }
 
+private extension FileHandle {
+    var shouldBeClosed: Bool {
+        return self !== FileHandle.standardOutput &&
+            self !== FileHandle.standardError &&
+            self !== FileHandle.standardInput
+    }
+}
+
 private extension Data {
     func shellOutput() -> String {
         guard let output = String(data: self, encoding: .utf8) else {

Tests/ShellOutTests/ShellOutTests.swift

@@ -99,15 +99,42 @@ class ShellOutTests: XCTestCase {
         XCTAssertEqual(error.localizedDescription, expectedErrorDescription)
     }
 
-    func testCapturingOutputWithHandle() throws {
+    func testCapturingOutputWithFileHandle() throws {
         let pipe = Pipe()
         let output = try shellOut(to: "echo", arguments: ["Hello"], outputHandle: pipe.fileHandleForWriting)
         let capturedData = pipe.fileHandleForReading.readDataToEndOfFile()
         XCTAssertEqual(output, "Hello")
         XCTAssertEqual(output + "\n", String(data: capturedData, encoding: .utf8))
     }
 
-    func testCapturingErrorWithHandle() throws {
+    func testMultipleCallsToStandardOut() throws {
+        let standardOut = FileHandle.standardOutput
+
+        /// Do not wrap these calls in XCTAssertNoThrow as it suppresses the error and the test will
+        /// always pass. These calls will trap if the standardOut FileHandle is closed.
+        try shellOut(to: "echo", arguments: ["Hello"], outputHandle: standardOut)
+        try shellOut(to: "echo", arguments: ["Hello"], outputHandle: standardOut)
+    }
+
+    func testMultipleCallsToStandardError() throws {
+        let standardError = FileHandle.standardError
+
+        /// Do not wrap these calls in XCTAssertNoThrow as it suppresses the error and the test will
+        /// always pass. These calls will trap if the standardError FileHandle is closed.
+        _ = try? shellOut(to: "bash 'exit 1'", arguments: [], errorHandle: standardError)
+        _ = try? shellOut(to: "bash 'exit 1'", arguments: [], errorHandle: standardError)
+    }
+
+
+    func testCapturingOutputWithStringHandle() throws {
+        var stringHandleOutput = ""
+        let stringHandle = TestStringHandle { stringHandleOutput.append($0) }
+        let output = try shellOut(to: "echo", arguments: ["Hello"], outputHandle: stringHandle)
+        XCTAssertEqual(output, "Hello")
+        XCTAssertEqual(output, stringHandleOutput)
+    }
+
+    func testCapturingErrorWithFileHandle() throws {
         let pipe = Pipe()
 
         do {
@@ -124,6 +151,24 @@ class ShellOutTests: XCTestCase {
             XCTFail("Invalid error type: \(error)")
         }
     }
+
+    func testCapturingErrorWithStringHandle() throws {
+        var stringHandleOutput = ""
+        let stringHandle = TestStringHandle { stringHandleOutput.append($0) }
+
+        do {
+            try shellOut(to: "cd", arguments: ["notADirectory"], errorHandle: stringHandle)
+            XCTFail("Expected expression to throw")
+        } catch let error as ShellOutError {
+            XCTAssertTrue(error.message.contains("notADirectory"))
+            XCTAssertTrue(error.output.isEmpty)
+            XCTAssertTrue(error.terminationStatus != 0)
+
+            XCTAssertEqual(error.message, stringHandleOutput)
+        } catch {
+            XCTFail("Invalid error type: \(error)")
+        }
+    }
 
     func testGitCommands() throws {
         // Setup & clear state
@@ -175,3 +220,22 @@ class ShellOutTests: XCTestCase {
         XCTAssertTrue(try shellOut(to: "ls -a", at: packagePath).contains("SwiftPackageManagerTest.xcodeproj"))
     }
 }
+
+/// Test Handle to get async output from the command. The `handlingClosure` will be called each time new output string appear.
+struct TestStringHandle: Handle {
+    private let handlingClosure: (String) -> Void
+
+    /// Default initializer
+    ///
+    /// - Parameter handlingClosure: closure called each time new output string is provided
+    public init(handlingClosure: @escaping (String) -> Void) {
+        self.handlingClosure = handlingClosure
+    }
+
+    public func handle(data: Data) {
+        guard !data.isEmpty else { return }
+        let trimmedOutput = String(data: data, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines)
+        guard let output = trimmedOutput, !output.isEmpty else { return }
+        handlingClosure(output)
+    }
+}