Merged by Peril

Danger executor

CHANGELOG.md

@@ -13,7 +13,8 @@
 
 ## Master
 
-- Use if let instead of try catch to decode fileMap  [@f-meloni][] - [#223](https://github.com/danger/swift/pull/223)
+- Use danger shell executor instead of ShellOut [@f-meloni][] - [#224](https://github.com/danger/swift/pull/224)
+- Use if let instead of try catch to decode fileMap [@f-meloni][] - [#223](https://github.com/danger/swift/pull/223)
 
 ## 1.5.2
 

Package.swift

@@ -15,7 +15,6 @@ let package = Package(
     dependencies: [
         .package(url: "https://github.com/f-meloni/Logger", from: "0.1.0"),
         .package(url: "https://github.com/JohnSundell/Marathon", from: "3.1.0"),
-        .package(url: "https://github.com/JohnSundell/ShellOut", from: "2.1.0"),
         .package(url: "https://github.com/nerdishbynature/octokit.swift", from: "0.9.0"),
         // Danger Plugins
         // Dev dependencies
@@ -29,8 +28,9 @@ let package = Package(
     ],
     targets: [
         .target(name: "Danger-Swift", dependencies: ["Danger", "Yams"]), // dev
-        .target(name: "Danger", dependencies: ["ShellOut", "OctoKit", "Logger"]),
-        .target(name: "RunnerLib", dependencies: ["Logger", "ShellOut"]),
+        .target(name: "DangerShellExecutor"),
+        .target(name: "Danger", dependencies: ["OctoKit", "Logger", "DangerShellExecutor"]),
+        .target(name: "RunnerLib", dependencies: ["Logger", "DangerShellExecutor"]),
         .target(name: "Runner", dependencies: ["RunnerLib", "MarathonCore", "Logger"]),
         .target(name: "DangerFixtures", dependencies: ["Danger"]),
         .testTarget(name: "DangerTests", dependencies: ["Danger", "DangerFixtures", "SnapshotTesting"]), // dev

Sources/Danger/DangerUtils.swift

@@ -1,3 +1,4 @@
+import DangerShellExecutor
 import Foundation
 
 /// Utility functions that make Dangerfiles easier to write

Sources/Danger/Plugins/SwiftLint/SwiftLint.swift

@@ -1,5 +1,5 @@
+import DangerShellExecutor
 import Foundation
-import ShellOut
 
 /// The SwiftLint plugin has been embedded inside Danger, making
 /// it usable out of the box.
@@ -33,7 +33,7 @@ extension SwiftLint {
     // swiftlint:disable:next function_body_length
     static func lint(
         danger: DangerDSL,
-        shellExecutor: ShellExecutor,
+        shellExecutor: ShellExecuting,
         swiftlintPath: String,
         inline: Bool = false,
         directory: String? = nil,
@@ -111,7 +111,7 @@ extension SwiftLint {
 
     private static func lintAll(directory: String?,
                                 arguments: [String],
-                                shellExecutor: ShellExecutor,
+                                shellExecutor: ShellExecuting,
                                 swiftlintPath: String,
                                 failAction: (String) -> Void) -> [SwiftLintViolation] {
         var arguments = arguments
@@ -128,7 +128,7 @@ extension SwiftLint {
     private static func lintModifiedAndCreated(danger: DangerDSL,
                                                directory: String?,
                                                arguments: [String],
-                                               shellExecutor: ShellExecutor,
+                                               shellExecutor: ShellExecuting,
                                                swiftlintPath: String,
                                                failAction: (String) -> Void) -> [SwiftLintViolation] {
         // Gathers modified+created files, invokes SwiftLint on each, and posts collected errors+warnings to Danger.

Sources/Danger/Plugins/SwiftLint/ShellExecutor.swift → Sources/DangerShellExecutor/ShellExecutor.swift

@@ -1,16 +1,41 @@
 import Foundation
 
-// TODO: Get a logger into here this is real tricky
-// because of the decoding from JSON nature of DangerDSL
-
 public enum SpawnError: Error {
-    case commandFailed(exitCode: Int32, stdout: String, stderr: String, task: Process)
+    case commandFailed(command: String, exitCode: Int32, stdout: String, stderr: String)
 }
 
-internal class ShellExecutor {
+public protocol ShellExecuting {
+    @discardableResult
     func execute(_ command: String,
-                 arguments: [String] = [],
-                 environmentVariables: [String: String] = [:]) -> String {
+                 arguments: [String],
+                 environmentVariables: [String: String]) -> String
+
+    @discardableResult
+    func spawn(_ command: String,
+               arguments: [String],
+               environmentVariables: [String: String]) throws -> String
+}
+
+extension ShellExecuting {
+    @discardableResult
+    public func execute(_ command: String,
+                        arguments: [String]) -> String {
+        return execute(command, arguments: arguments, environmentVariables: [:])
+    }
+
+    @discardableResult
+    public func spawn(_ command: String,
+                      arguments: [String]) throws -> String {
+        return try spawn(command, arguments: arguments, environmentVariables: [:])
+    }
+}
+
+public struct ShellExecutor: ShellExecuting {
+    public init() {}
+
+    public func execute(_ command: String,
+                        arguments: [String],
+                        environmentVariables: [String: String]) -> String {
         let task = makeTask(for: command, with: arguments, environmentVariables: environmentVariables)
 
         let pipe = Pipe()
@@ -19,14 +44,14 @@ internal class ShellExecutor {
         task.waitUntilExit()
 
         let data = pipe.fileHandleForReading.readDataToEndOfFile()
-        return String(data: data, encoding: String.Encoding.utf8)!
+        return String(data: data, encoding: .utf8)!.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines)
     }
 
     // Similar to above, but can throw, and throws with most of
     // what you'd probably need in a scripting environment
-    func spawn(_ command: String,
-               arguments: [String] = [],
-               environmentVariables: [String: String] = [:]) throws -> String {
+    public func spawn(_ command: String,
+                      arguments: [String],
+                      environmentVariables: [String: String]) throws -> String {
         let task = makeTask(for: command, with: arguments, environmentVariables: environmentVariables)
 
         let stdout = Pipe()
@@ -38,28 +63,27 @@ internal class ShellExecutor {
 
         // Pull out the STDOUT as a string because we'll need that regardless
         let stdoutData = stdout.fileHandleForReading.readDataToEndOfFile()
-        let stdoutString = String(data: stdoutData, encoding: String.Encoding.utf8)!
+        let stdoutString = String(data: stdoutData, encoding: .utf8)!
 
         // 0 is no problems in unix land
         if task.terminationStatus == 0 {
-            return stdoutString
+            return stdoutString.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines)
         }
 
         // OK, so it failed, raise a new error with all the useful metadata
         let stderrData = stdout.fileHandleForReading.readDataToEndOfFile()
-        let stderrString = String(data: stderrData, encoding: String.Encoding.utf8)!
+        let stderrString = String(data: stderrData, encoding: .utf8)!
 
-        throw SpawnError.commandFailed(exitCode: task.terminationStatus,
+        throw SpawnError.commandFailed(command: command,
+                                       exitCode: task.terminationStatus,
                                        stdout: stdoutString,
-                                       stderr: stderrString,
-                                       task: task)
+                                       stderr: stderrString)
     }
 
     private func makeTask(for command: String,
                           with arguments: [String],
                           environmentVariables: [String: String]) -> Process {
-        let script = [command,
-                      arguments.joined(separator: " ")].filter { !$0.isEmpty }.joined(separator: " ")
+        let script = "\(command) \(arguments.joined(separator: " "))"
         let processEnv = ProcessInfo.processInfo.environment
         let task = Process()
         task.launchPath = processEnv["SHELL"]

Sources/Runner/Script+Imports.swift

@@ -1,8 +1,7 @@
-import MarathonCore
-
+import DangerShellExecutor
 import Files
 import Foundation
-import ShellOut
+import MarathonCore
 
 extension Script {
     @discardableResult
@@ -57,10 +56,10 @@ extension Script {
     }
 
     private func generateXCodeProjWithConfig(configPath: String) throws {
-        try shellOutToSwiftCommand("package generate-xcodeproj --xcconfig-overrides \(configPath)", in: folder)
+        try executeSwiftCommand("package generate-xcodeproj --xcconfig-overrides \(configPath)", in: folder)
     }
 
-    func shellOutToSwiftCommand(_ command: String, in folder: Folder) throws {
+    func executeSwiftCommand(_ command: String, in folder: Folder) throws {
         func resolveSwiftPath() -> String {
             #if os(Linux)
                 return "swift"
@@ -70,6 +69,7 @@ extension Script {
         }
 
         let swiftPath = resolveSwiftPath()
-        try shellOut(to: "\(swiftPath) \(command)", at: folder.path)
+        let executor = ShellExecutor()
+        try executor.spawn("cd \(folder.path) && \(swiftPath) \(command)", arguments: [])
     }
 }

Sources/RunnerLib/DangerJSVersionFinder.swift

@@ -1,27 +1,14 @@
+import DangerShellExecutor
 import Logger
-import ShellOut
 
 public final class DangerJSVersionFinder {
     public static func findDangerJSVersion(dangerJSPath: String) throws -> String {
-        return try findDangerJSVersion(dangerJSPath: dangerJSPath, executor: ShellOutExecutor())
+        return try findDangerJSVersion(dangerJSPath: dangerJSPath, executor: ShellExecutor())
     }
 
-    static func findDangerJSVersion(dangerJSPath: String, executor: ShellOutExecuting) throws -> String {
+    static func findDangerJSVersion(dangerJSPath: String, executor: ShellExecuting) throws -> String {
         Logger().debug("Finding danger-js version")
 
-        return try executor.shellOut(command: dangerJSPath + " --version")
-    }
-}
-
-public protocol ShellOutExecuting {
-    @discardableResult
-    func shellOut(command: String) throws -> String
-}
-
-public struct ShellOutExecutor: ShellOutExecuting {
-    public init() {}
-
-    public func shellOut(command: String) throws -> String {
-        return try ShellOut.shellOut(to: command)
+        return executor.execute(dangerJSPath, arguments: ["--version"])
     }
 }

Sources/RunnerLib/GetDangerJSPath.swift

@@ -1,13 +1,15 @@
+import DangerShellExecutor
 import Foundation
 import Logger
-import ShellOut
 
-public func getDangerCommandPath(logger: Logger, args: [String] = CommandLine.arguments, shellOutExecutor: ShellOutExecuting = ShellOutExecutor()) throws -> String {
+public func getDangerCommandPath(logger: Logger,
+                                 args: [String] = CommandLine.arguments,
+                                 shellOutExecutor: ShellExecuting = ShellExecutor()) throws -> String {
     if let dangerJSPathOptionIndex = args.firstIndex(of: DangerSwiftOption.dangerJSPath.rawValue),
         dangerJSPathOptionIndex + 1 < args.count {
         return args[dangerJSPathOptionIndex + 1]
     } else {
         logger.debug("Finding out where the danger executable is")
-        return try shellOutExecutor.shellOut(command: "which danger").trimmingCharacters(in: .whitespaces)
+        return try shellOutExecutor.spawn("which danger", arguments: []).trimmingCharacters(in: .whitespaces)
     }
 }

Sources/RunnerLib/SPMDanger.swift

@@ -1,3 +1,4 @@
+import DangerShellExecutor
 import Foundation
 
 public struct SPMDanger {
@@ -25,9 +26,9 @@ public struct SPMDanger {
         }
     }
 
-    public func buildDependencies(executor: ShellOutExecuting = ShellOutExecutor(),
+    public func buildDependencies(executor: ShellExecuting = ShellExecutor(),
                                   fileManager _: FileManager = .default) {
-        _ = try? executor.shellOut(command: "swift build --product \(depsLibName)")
+        _ = executor.execute("swift build", arguments: ["--product \(depsLibName)"])
     }
 
     public var swiftcLibImport: String {

Tests/DangerTests/SwiftLint/DangerSwiftLintTests.swift

@@ -24,7 +24,8 @@ class DangerSwiftLintTests: XCTestCase {
     }
 
     func testExecutesTheShellWithCustomSwiftLintPath() {
-        _ = SwiftLint.lint(danger: danger, shellExecutor: executor, swiftlintPath: "Pods/SwiftLint/swiftlint", currentPathProvider: fakePathProvider)
+        _ = SwiftLint.lint(danger: danger,
+                           shellExecutor: executor, swiftlintPath: "Pods/SwiftLint/swiftlint", currentPathProvider: fakePathProvider)
         XCTAssertEqual(executor.invocations.count, 1)
         XCTAssertEqual(executor.invocations.first?.command, "Pods/SwiftLint/swiftlint")
     }

Tests/DangerTests/SwiftLint/FakeShellExecutor.swift

@@ -1,12 +1,18 @@
 @testable import Danger
+import DangerShellExecutor
 
-class FakeShellExecutor: ShellExecutor {
+final class FakeShellExecutor: ShellExecuting {
     typealias Invocation = (command: String, arguments: [String], environmentVariables: [String: String])
 
     var invocations = [Invocation]() /// All of the invocations received by this instance.
     var output = "[]" /// This is returned by `execute` as the process' standard output. We default to an empty JSON array.
 
-    override func execute(_ command: String, arguments: [String], environmentVariables: [String: String]) -> String {
+    func execute(_ command: String, arguments: [String], environmentVariables: [String: String]) -> String {
+        invocations.append((command: command, arguments: arguments, environmentVariables: environmentVariables))
+        return output
+    }
+
+    func spawn(_ command: String, arguments: [String], environmentVariables: [String: String]) throws -> String {
         invocations.append((command: command, arguments: arguments, environmentVariables: environmentVariables))
         return output
     }

Tests/RunnerLibTests/MockedExecutor.swift

@@ -1,10 +1,16 @@
+import DangerShellExecutor
 import RunnerLib
 
-final class MockedExecutor: ShellOutExecuting {
+final class MockedExecutor: ShellExecuting {
     var receivedCommand: String!
     var result = ""
 
-    func shellOut(command: String) throws -> String {
+    func execute(_ command: String, arguments: [String], environmentVariables _: [String: String]) -> String {
+        receivedCommand = command + " " + arguments.joined(separator: " ")
+        return result
+    }
+
+    func spawn(_ command: String, arguments _: [String], environmentVariables _: [String: String]) throws -> String {
         receivedCommand = command
         return result
     }

Tests/RunnerLibTests/SPMDangerTests.swift

@@ -41,7 +41,7 @@ final class SPMDangerTests: XCTestCase {
         try! ".library(name: \"DangerDeps\"".write(toFile: testPackage, atomically: false, encoding: .utf8)
         SPMDanger(packagePath: testPackage)?.buildDependencies(executor: executor, fileManager: fileManager)
 
-        XCTAssertTrue(executor.receivedCommand == "swift build --product DangerDeps")
+        XCTAssertEqual(executor.receivedCommand, "swift build --product DangerDeps")
     }
 
     func testItReturnsTheCorrectXcodeDepsFlagsWhenThereIsNoDangerLib() {