Skip to content

Commit f9881e5

Browse files
authored
Unify tests on all supported platforms to ensure consistent behavior and add more tests. (#133)
* Unify tests on all supported platforms to ensure consistent behavior and add more tests. * Implement arg0 override feature for Windows * Introduce ProcessMonitoringTests * Make possibleExecutablePaths() preserve order
1 parent 51b08c0 commit f9881e5

23 files changed

+4456
-2066
lines changed

Sources/Subprocess/Configuration.swift

Lines changed: 69 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -79,38 +79,42 @@ public struct Configuration: Sendable {
7979

8080
let execution = _spawnResult.execution
8181

82-
let result: Swift.Result<Result, Error>
83-
do {
84-
result = try await .success(withAsyncTaskCleanupHandler {
85-
let inputIO = _spawnResult.inputWriteEnd()
86-
let outputIO = _spawnResult.outputReadEnd()
87-
let errorIO = _spawnResult.errorReadEnd()
82+
return try await withAsyncTaskCleanupHandler {
83+
let inputIO = _spawnResult.inputWriteEnd()
84+
let outputIO = _spawnResult.outputReadEnd()
85+
let errorIO = _spawnResult.errorReadEnd()
8886

87+
let result: Swift.Result<Result, Error>
88+
do {
8989
// Body runs in the same isolation
90-
return try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
91-
} onCleanup: {
92-
// Attempt to terminate the child process
93-
await execution.runTeardownSequence(
94-
self.platformOptions.teardownSequence
95-
)
96-
})
97-
} catch {
98-
result = .failure(error)
99-
}
90+
let bodyResult = try await body(_spawnResult.execution, inputIO, outputIO, errorIO)
91+
result = .success(bodyResult)
92+
} catch {
93+
result = .failure(error)
94+
}
10095

101-
// Ensure that we begin monitoring process termination after `body` runs
102-
// and regardless of whether `body` throws, so that the pid gets reaped
103-
// even if `body` throws, and we are not leaving zombie processes in the
104-
// process table which will cause the process termination monitoring thread
105-
// to effectively hang due to the pid never being awaited
106-
let terminationStatus = try await Subprocess.monitorProcessTermination(
107-
for: execution.processIdentifier
108-
)
96+
// Ensure that we begin monitoring process termination after `body` runs
97+
// and regardless of whether `body` throws, so that the pid gets reaped
98+
// even if `body` throws, and we are not leaving zombie processes in the
99+
// process table which will cause the process termination monitoring thread
100+
// to effectively hang due to the pid never being awaited
101+
let terminationStatus = try await monitorProcessTermination(
102+
for: execution.processIdentifier
103+
)
109104

110-
// Close process file descriptor now we finished monitoring
111-
execution.processIdentifier.close()
105+
// Close process file descriptor now we finished monitoring
106+
execution.processIdentifier.close()
112107

113-
return try ExecutionResult(terminationStatus: terminationStatus, value: result.get())
108+
return ExecutionResult(
109+
terminationStatus: terminationStatus,
110+
value: try result.get()
111+
)
112+
} onCleanup: {
113+
// Attempt to terminate the child process
114+
await execution.runTeardownSequence(
115+
self.platformOptions.teardownSequence
116+
)
117+
}
114118
}
115119
}
116120

@@ -246,40 +250,6 @@ extension Executable: CustomStringConvertible, CustomDebugStringConvertible {
246250
}
247251
}
248252

249-
extension Executable {
250-
internal func possibleExecutablePaths(
251-
withPathValue pathValue: String?
252-
) -> Set<String> {
253-
switch self.storage {
254-
case .executable(let executableName):
255-
#if os(Windows)
256-
// Windows CreateProcessW accepts executable name directly
257-
return Set([executableName])
258-
#else
259-
var results: Set<String> = []
260-
// executableName could be a full path
261-
results.insert(executableName)
262-
// Get $PATH from environment
263-
let searchPaths: Set<String>
264-
if let pathValue = pathValue {
265-
let localSearchPaths = pathValue.split(separator: ":").map { String($0) }
266-
searchPaths = Set(localSearchPaths).union(Self.defaultSearchPaths)
267-
} else {
268-
searchPaths = Self.defaultSearchPaths
269-
}
270-
for path in searchPaths {
271-
results.insert(
272-
FilePath(path).appending(executableName).string
273-
)
274-
}
275-
return results
276-
#endif
277-
case .path(let executablePath):
278-
return Set([executablePath.string])
279-
}
280-
}
281-
}
282-
283253
// MARK: - Arguments
284254

285255
/// A collection of arguments to pass to the subprocess.
@@ -300,7 +270,6 @@ public struct Arguments: Sendable, ExpressibleByArrayLiteral, Hashable {
300270
self.executablePathOverride = nil
301271
}
302272

303-
#if !os(Windows) // Windows does NOT support arg0 override
304273
/// Create an `Argument` object using the given values, but
305274
/// override the first Argument value to `executablePathOverride`.
306275
/// If `executablePathOverride` is nil,
@@ -317,7 +286,7 @@ public struct Arguments: Sendable, ExpressibleByArrayLiteral, Hashable {
317286
self.executablePathOverride = nil
318287
}
319288
}
320-
289+
#if !os(Windows) // Windows does not support non-unicode arguments
321290
/// Create an `Argument` object using the given values, but
322291
/// override the first Argument value to `executablePathOverride`.
323292
/// If `executablePathOverride` is nil,
@@ -869,7 +838,7 @@ internal struct CreatedPipe: ~Copyable {
869838
DWORD(readBufferSize),
870839
DWORD(readBufferSize),
871840
0,
872-
&saAttributes
841+
nil
873842
)
874843
}
875844
guard let parentEnd, parentEnd != INVALID_HANDLE_VALUE else {
@@ -1029,3 +998,39 @@ internal func withAsyncTaskCleanupHandler<Result>(
1029998
}
1030999
}
10311000
}
1001+
1002+
internal struct _OrderedSet<Element: Hashable & Sendable>: Hashable, Sendable {
1003+
private var elements: [Element]
1004+
private var hashValueSet: Set<Int>
1005+
1006+
internal init() {
1007+
self.elements = []
1008+
self.hashValueSet = Set()
1009+
}
1010+
1011+
internal init(_ arrayValue: [Element]) {
1012+
self.elements = []
1013+
self.hashValueSet = Set()
1014+
1015+
for element in arrayValue {
1016+
self.insert(element)
1017+
}
1018+
}
1019+
1020+
mutating func insert(_ element: Element) {
1021+
guard !self.hashValueSet.contains(element.hashValue) else {
1022+
return
1023+
}
1024+
self.elements.append(element)
1025+
self.hashValueSet.insert(element.hashValue)
1026+
}
1027+
}
1028+
1029+
extension _OrderedSet : Sequence {
1030+
typealias Iterator = Array<Element>.Iterator
1031+
1032+
internal func makeIterator() -> Iterator {
1033+
return self.elements.makeIterator()
1034+
}
1035+
}
1036+

Sources/Subprocess/Error.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ extension SubprocessError: CustomStringConvertible, CustomDebugStringConvertible
111111
case .failedToWriteToSubprocess:
112112
return "Failed to write bytes to the child process."
113113
case .failedToMonitorProcess:
114-
return "Failed to monitor the state of child process with underlying error: \(self.underlyingError!)"
114+
return "Failed to monitor the state of child process with underlying error: \(self.underlyingError.map { "\($0)" } ?? "nil")"
115115
case .streamOutputExceedsLimit(let limit):
116116
return "Failed to create output from current buffer because the output limit (\(limit)) was reached."
117117
case .asyncIOFailed(let reason):

Sources/Subprocess/IO/AsyncIO+Darwin.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ internal import Dispatch
2525
final class AsyncIO: Sendable {
2626
static let shared: AsyncIO = AsyncIO()
2727

28-
private init() {}
28+
internal init() {}
29+
30+
internal func shutdown() { /* noop on Darwin */ }
2931

3032
internal func read(
3133
from diskIO: borrowing IOChannel,

0 commit comments

Comments
 (0)