Merge branch 'master' into review-before-submmitted

danger · Oct 14, 2024 · 3261c11 · 3261c11
2 parents b8721e6 + b22f43c
commit 3261c11
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,8 +14,10 @@
 ## Master
 
 - Fix: `GitHub.Review.submittedAt` may be `nil` [@417-72KI][] - [#624](https://github.com/danger/swift/pull/624)
+- Drain stdout while shell commands are running to prevent execution from locking up if there's too much output [@jflan-dd][] - [#614](https://github.com/danger/swift/pull/614)
 
 ## 3.20.0
+
 - Remove deprecated `lint` function with `lintAllFiles` flag [@417-72KI][] - [#622](https://github.com/danger/swift/pull/622)
 - Updated Swift 6 build process: Danger files moved to .build/debug/Modules, and SwiftFormat module map conflict resolved by adjusting the Swift import search path. [@abhi-m-simformsolutons][] -[#626](https://github.com/danger/swift/pull/626)
 

diff --git a/Sources/Danger/Plugins/SwiftLint/SwiftLint.swift b/Sources/Danger/Plugins/SwiftLint/SwiftLint.swift
@@ -185,7 +185,7 @@ extension SwiftLint {
         var arguments = arguments
 
         if let directory = directory {
-            arguments.append("--path \"\(directory)\"")
+            arguments.append(directory)
         }
 
         return swiftlintViolations(swiftlintPath: swiftlintPath,

diff --git a/Sources/DangerShellExecutor/ShellExecutor.swift b/Sources/DangerShellExecutor/ShellExecutor.swift
@@ -61,9 +61,11 @@ public struct ShellExecutor: ShellExecuting {
         let pipe = Pipe()
         task.standardOutput = pipe
         task.launch()
-        task.waitUntilExit()
 
         let data = pipe.fileHandleForReading.readDataToEndOfFile()
+
+        task.waitUntilExit()
+
         return String(data: data, encoding: .utf8)!.trimmingCharacters(in: .whitespacesAndNewlines)
     }
 
@@ -83,19 +85,22 @@ public struct ShellExecutor: ShellExecuting {
         let stderr = Pipe()
         task.standardError = stderr
         task.launch()
-        task.waitUntilExit()
 
         // Pull out the STDOUT as a string because we'll need that regardless
         let stdoutData = stdout.fileHandleForReading.readDataToEndOfFile()
         let stdoutString = String(data: stdoutData, encoding: .utf8)!
 
+        // Read from STDERR to ensure the `Pipe` does not fill up
+        let stderrData = stderr.fileHandleForReading.readDataToEndOfFile()
+
+        task.waitUntilExit()
+
         // 0 is no problems in unix land
         if task.terminationStatus == 0 {
             return stdoutString.trimmingCharacters(in: .whitespacesAndNewlines)
         }
 
         // OK, so it failed, raise a new error with all the useful metadata
-        let stderrData = stderr.fileHandleForReading.readDataToEndOfFile()
         let stderrString = String(data: stderrData, encoding: .utf8)!
 
         throw SpawnError.commandFailed(command: command,

diff --git a/Tests/DangerTests/SwiftLint/DangerSwiftLintTests.swift b/Tests/DangerTests/SwiftLint/DangerSwiftLintTests.swift
@@ -268,7 +268,7 @@ final class DangerSwiftLintTests: XCTestCase {
         XCTAssertNotNil(swiftlintCommand)
         XCTAssertEqual(swiftlintCommand!.environmentVariables.count, 0)
         XCTAssertFalse(swiftlintCommand!.environmentVariables.values.contains { $0.contains("Tests/SomeFile.swift") })
-        XCTAssertTrue(swiftlintCommand!.arguments.contains("--path \"Tests\""))
+        XCTAssertEqual(swiftlintCommand!.arguments.last, directory)
     }
 
     func testFiltersOnSwiftFiles() {