From bfa2cdd7bc8e2df4ed1dabfcce42cec9fca7029b Mon Sep 17 00:00:00 2001 From: Grzegorz Wikiera Date: Fri, 26 Jan 2018 09:37:31 +0100 Subject: [PATCH 1/6] Added `Handle` protocol to get asynchronous output --- Sources/ShellOut.swift | 76 +++++++++++++++++++------ Tests/ShellOutTests/ShellOutTests.swift | 30 +++++++++- 2 files changed, 87 insertions(+), 19 deletions(-) diff --git a/Sources/ShellOut.swift b/Sources/ShellOut.swift index 290b9d0..b2b6379 100644 --- a/Sources/ShellOut.swift +++ b/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,52 @@ 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() { + closeFile() + } +} + +/// Handle to get async output from the command. The `handlingClosure` will be called each time new output string appear. +public struct StringHandle: 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 output = data.shellOutput() + guard !output.isEmpty else { return } + handlingClosure(output) + } +} + // 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] @@ -370,7 +412,7 @@ private extension Process { outputQueue.async { let data = handler.availableData outputData.append(data) - outputHandle?.write(data) + outputHandle?.handle(data: data) } } @@ -378,7 +420,7 @@ private extension Process { outputQueue.async { let data = handler.availableData errorData.append(data) - errorHandle?.write(data) + errorHandle?.handle(data: data) } } #endif @@ -394,8 +436,8 @@ private extension Process { waitUntilExit() - outputHandle?.closeFile() - errorHandle?.closeFile() + outputHandle?.endHandling() + errorHandle?.endHandling() #if !os(Linux) outputPipe.fileHandleForReading.readabilityHandler = nil diff --git a/Tests/ShellOutTests/ShellOutTests.swift b/Tests/ShellOutTests/ShellOutTests.swift index 90a8fd3..7392f7d 100644 --- a/Tests/ShellOutTests/ShellOutTests.swift +++ b/Tests/ShellOutTests/ShellOutTests.swift @@ -99,7 +99,7 @@ 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() @@ -107,7 +107,15 @@ class ShellOutTests: XCTestCase { XCTAssertEqual(output + "\n", String(data: capturedData, encoding: .utf8)) } - func testCapturingErrorWithHandle() throws { + func testCapturingOutputWithStringHandle() throws { + var stringHandleOutput = "" + let stringHandle = StringHandle { 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 +132,24 @@ class ShellOutTests: XCTestCase { XCTFail("Invalid error type: \(error)") } } + + func testCapturingErrorWithStringHandle() throws { + var stringHandleOutput = "" + let stringHandle = StringHandle { 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 From bfc5ebb4aa03ee194ddcce5cf9e904d55e776167 Mon Sep 17 00:00:00 2001 From: Grzegorz Wikiera Date: Mon, 29 Jan 2018 11:29:01 +0100 Subject: [PATCH 2/6] Fixed bug with `xcodebuild test` --- Sources/ShellOut.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/ShellOut.swift b/Sources/ShellOut.swift index b2b6379..1e3d6b8 100644 --- a/Sources/ShellOut.swift +++ b/Sources/ShellOut.swift @@ -409,16 +409,16 @@ 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?.handle(data: data) } } errorPipe.fileHandleForReading.readabilityHandler = { handler in + let data = handler.availableData outputQueue.async { - let data = handler.availableData errorData.append(data) errorHandle?.handle(data: data) } From 295a25cc2d8fd54885de5a1e03bed9ec0e22413c Mon Sep 17 00:00:00 2001 From: Iain Date: Wed, 11 Apr 2018 23:29:21 +0100 Subject: [PATCH 3/6] Keep standardOuput, standardError & standardInput fileHandles open. This adds two tests with no assertions to verify that standardOut & standardError are not closed. I tried wrapping the shellOut calls in XCTAssertNoThrow but this causes the tests to always pass. Co-authored-by: SteveBarnegren --- Sources/ShellOut.swift | 10 +++++++++- Tests/ShellOutTests/ShellOutTests.swift | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Sources/ShellOut.swift b/Sources/ShellOut.swift index 1e3d6b8..c25b97f 100644 --- a/Sources/ShellOut.swift +++ b/Sources/ShellOut.swift @@ -362,7 +362,7 @@ extension FileHandle: Handle { } public func endHandling() { - closeFile() + if shouldBeClosed { closeFile() } } } @@ -460,6 +460,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 { diff --git a/Tests/ShellOutTests/ShellOutTests.swift b/Tests/ShellOutTests/ShellOutTests.swift index 7392f7d..4ab9fdb 100644 --- a/Tests/ShellOutTests/ShellOutTests.swift +++ b/Tests/ShellOutTests/ShellOutTests.swift @@ -107,6 +107,26 @@ class ShellOutTests: XCTestCase { XCTAssertEqual(output + "\n", String(data: capturedData, encoding: .utf8)) } + 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 = StringHandle { stringHandleOutput.append($0) } From 1091c9caa41016f36d545204e886be0f4c283d5d Mon Sep 17 00:00:00 2001 From: Iain Date: Wed, 11 Apr 2018 23:35:57 +0100 Subject: [PATCH 4/6] Run CI on Linux & OSX against Swift 4.0 & 4.1 --- .travis.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 06f52f3..2e419df 100644 --- a/.travis.yml +++ b/.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 From 46c6382a0531a448841c0fa6fc58fcf75f216c56 Mon Sep 17 00:00:00 2001 From: Iain Date: Wed, 11 Apr 2018 23:48:00 +0100 Subject: [PATCH 5/6] Knock swift version down to 4.0 --- Package.swift | 2 +- Tests/ShellOutTests/ShellOutTests.swift | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Package.swift b/Package.swift index 5d66919..97b4eae 100644 --- a/Package.swift +++ b/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version:4.1 +// swift-tools-version:4.0 /** * ShellOut diff --git a/Tests/ShellOutTests/ShellOutTests.swift b/Tests/ShellOutTests/ShellOutTests.swift index 4ab9fdb..9d07cc0 100644 --- a/Tests/ShellOutTests/ShellOutTests.swift +++ b/Tests/ShellOutTests/ShellOutTests.swift @@ -114,7 +114,6 @@ class ShellOutTests: XCTestCase { /// 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 { From 2aa0192a3dc1530153603e2e2a3fc4a79dc29724 Mon Sep 17 00:00:00 2001 From: Iain Date: Fri, 13 Apr 2018 10:33:31 +0100 Subject: [PATCH 6/6] Limit the proposed public API --- Sources/ShellOut.swift | 19 ------------------- Tests/ShellOutTests/ShellOutTests.swift | 23 +++++++++++++++++++++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/Sources/ShellOut.swift b/Sources/ShellOut.swift index c25b97f..8e6bf30 100644 --- a/Sources/ShellOut.swift +++ b/Sources/ShellOut.swift @@ -366,25 +366,6 @@ extension FileHandle: Handle { } } -/// Handle to get async output from the command. The `handlingClosure` will be called each time new output string appear. -public struct StringHandle: 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 output = data.shellOutput() - guard !output.isEmpty else { return } - handlingClosure(output) - } -} - // MARK: - Private private extension Process { diff --git a/Tests/ShellOutTests/ShellOutTests.swift b/Tests/ShellOutTests/ShellOutTests.swift index 9d07cc0..8b48bfd 100644 --- a/Tests/ShellOutTests/ShellOutTests.swift +++ b/Tests/ShellOutTests/ShellOutTests.swift @@ -128,7 +128,7 @@ class ShellOutTests: XCTestCase { func testCapturingOutputWithStringHandle() throws { var stringHandleOutput = "" - let stringHandle = StringHandle { stringHandleOutput.append($0) } + let stringHandle = TestStringHandle { stringHandleOutput.append($0) } let output = try shellOut(to: "echo", arguments: ["Hello"], outputHandle: stringHandle) XCTAssertEqual(output, "Hello") XCTAssertEqual(output, stringHandleOutput) @@ -154,7 +154,7 @@ class ShellOutTests: XCTestCase { func testCapturingErrorWithStringHandle() throws { var stringHandleOutput = "" - let stringHandle = StringHandle { stringHandleOutput.append($0) } + let stringHandle = TestStringHandle { stringHandleOutput.append($0) } do { try shellOut(to: "cd", arguments: ["notADirectory"], errorHandle: stringHandle) @@ -220,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) + } +}