From 48cf2c6bbfbbdba220db43d651d580c9dd07dadc Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 18 Oct 2024 13:29:51 -0400 Subject: [PATCH 01/13] Add support for collecting `stdout` and `stderr` in an exit test. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR adds `stdout` and `stderr` capture to exit tests. Gathering these streams can be very expensive, so they are _opt-in_. There is a new argument to `#expect(exitsWith:)`, `observedValues`, to which you can pass one or more key paths on `ExitTest.Result` to indicate which bits you want: ```swift let result = await #expect(exitsWith: .failure, observing: [\.standardOutputContent]) { print("Goodbye, world!") fatalError() } #expect(result.standardOutputContent.contains(UInt8(ascii: "G"))) ``` Some changes to pipes were needed to satisfy the compiler's concerns about `Pipe` being a move-only type that contains other move-only types—now it's just a function that returns two `FileHandle` instances. This is simpler anyway. It is possible some callers will want `stdout` and `stderr` to be set, but to something other than a pipe (such as a file on disk or a PTY.) While these are interesting cases, they are beyond the scope of this PR. --- Sources/Testing/ExitTests/ExitTest.swift | 103 +++++++++++++-- .../Testing/ExitTests/ExitTestArtifacts.swift | 52 ++++++++ Sources/Testing/ExitTests/SpawnProcess.swift | 84 +++++++++--- .../Expectations/Expectation+Macro.swift | 64 +++++++++ .../ExpectationChecking+Macro.swift | 2 + Sources/Testing/Support/FileHandle.swift | 122 +++++++++--------- Sources/TestingMacros/ConditionMacro.swift | 7 + Tests/TestingTests/ExitTestTests.swift | 21 +++ .../Support/FileHandleTests.swift | 24 ++-- 9 files changed, 374 insertions(+), 105 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index e73a59883..1859a111f 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -34,6 +34,35 @@ public struct ExitTest: Sendable, ~Copyable { /// The body closure of the exit test. fileprivate var body: @Sendable () async throws -> Void = {} + /// Storage for ``observedValues``. + fileprivate nonisolated(unsafe) var _observedValues = Set>() + + /// Key paths representing results from within this exit test that should be + /// observed and returned to the caller. + /// + /// The testing library sets this property to match what was passed by the + /// developer to the `#expect(exitsWith:)` or `#require(exitsWith:)` macro. + /// If you are implementing an exit test handler, you can check the value of + /// this property to determine what information you need to preserve from your + /// child process. + /// + /// The value of this property always includes ``Result/exitCondition`` even + /// if the test author does not specify it. + /// + /// Within a child process running an exit test, the value of this property is + /// otherwise unspecified. + @_spi(ForToolsIntegrationOnly) + public var observedValues: Set> { + get { + var result = _observedValues + result.insert(\.exitCondition) + return result + } + set { + _observedValues = newValue + } + } + /// The source location of the exit test. /// /// The source location is unique to each exit test and is consistent between @@ -199,6 +228,7 @@ extension ExitTest { /// convention. func callExitTest( exitsWith expectedExitCondition: ExitCondition, + observing observedValues: Set>, expression: __Expression, comments: @autoclosure () -> [Comment], isRequired: Bool, @@ -211,7 +241,8 @@ func callExitTest( var result: ExitTestArtifacts do { - let exitTest = ExitTest(expectedExitCondition: expectedExitCondition, sourceLocation: sourceLocation) + var exitTest = ExitTest(expectedExitCondition: expectedExitCondition, sourceLocation: sourceLocation) + exitTest.observedValues = observedValues result = try await configuration.exitTestHandler(exitTest) #if os(Windows) @@ -465,20 +496,43 @@ extension ExitTest { childEnvironment["SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION"] = String(decoding: json, as: UTF8.self) } - return try await withThrowingTaskGroup(of: ExitTestArtifacts?.self) { taskGroup in + typealias ResultUpdater = @Sendable (inout ExitTestArtifacts) -> Void + return try await withThrowingTaskGroup(of: ResultUpdater?.self) { taskGroup in + // Set up stdout and stderr streams. By POSIX convention, stdin/stdout + // are line-buffered by default and stderr is unbuffered by default. + // SEE: https://en.cppreference.com/w/cpp/io/c/std_streams + var stdoutReadEnd: FileHandle? + var stdoutWriteEnd: FileHandle? + if exitTest._observedValues.contains(\.standardOutputContent) { + try FileHandle.makePipe(readEnd: &stdoutReadEnd, writeEnd: &stdoutWriteEnd) + stdoutWriteEnd?.withUnsafeCFILEHandle { stdout in + _ = setvbuf(stdout, nil, _IOLBF, Int(BUFSIZ)) + } + } + var stderrReadEnd: FileHandle? + var stderrWriteEnd: FileHandle? + if exitTest._observedValues.contains(\.standardErrorContent) { + try FileHandle.makePipe(readEnd: &stderrReadEnd, writeEnd: &stderrWriteEnd) + stderrWriteEnd?.withUnsafeCFILEHandle { stderr in + _ = setvbuf(stderr, nil, _IONBF, Int(BUFSIZ)) + } + } + // Create a "back channel" pipe to handle events from the child process. - let backChannel = try FileHandle.Pipe() + var backChannelReadEnd: FileHandle! + var backChannelWriteEnd: FileHandle! + try FileHandle.makePipe(readEnd: &backChannelReadEnd, writeEnd: &backChannelWriteEnd) // Let the child process know how to find the back channel by setting a // known environment variable to the corresponding file descriptor // (HANDLE on Windows.) var backChannelEnvironmentVariable: String? #if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) - backChannelEnvironmentVariable = backChannel.writeEnd.withUnsafePOSIXFileDescriptor { fd in + backChannelEnvironmentVariable = backChannelWriteEnd.withUnsafePOSIXFileDescriptor { fd in fd.map(String.init(describing:)) } #elseif os(Windows) - backChannelEnvironmentVariable = backChannel.writeEnd.withUnsafeWindowsHANDLE { handle in + backChannelEnvironmentVariable = backChannelWriteEnd.withUnsafeWindowsHANDLE { handle in handle.flatMap { String(describing: UInt(bitPattern: $0)) } } #else @@ -489,32 +543,55 @@ extension ExitTest { } // Spawn the child process. - let processID = try withUnsafePointer(to: backChannel.writeEnd) { writeEnd in + let processID = try withUnsafePointer(to: backChannelWriteEnd) { backChannelWriteEnd in try spawnExecutable( atPath: childProcessExecutablePath, arguments: childArguments, environment: childEnvironment, - additionalFileHandles: .init(start: writeEnd, count: 1) + standardOutput: stdoutWriteEnd, + standardError: stderrWriteEnd, + additionalFileHandles: [backChannelWriteEnd] ) } // Await termination of the child process. taskGroup.addTask { let exitCondition = try await wait(for: processID) - return ExitTestArtifacts(exitCondition: exitCondition) + return { $0.exitCondition = exitCondition } + } + + // Read back the stdout and stderr streams. + if let stdoutReadEnd { + stdoutWriteEnd?.close() + taskGroup.addTask { + let standardOutputContent = try stdoutReadEnd.readToEnd() + return { $0.standardOutputContent = standardOutputContent } + } + } + if let stderrReadEnd { + stderrWriteEnd?.close() + taskGroup.addTask { + let standardErrorContent = try stderrReadEnd.readToEnd() + return { $0.standardErrorContent = standardErrorContent } + } } // Read back all data written to the back channel by the child process // and process it as a (minimal) event stream. - let readEnd = backChannel.closeWriteEnd() + backChannelWriteEnd.close() taskGroup.addTask { - Self._processRecords(fromBackChannel: readEnd) + Self._processRecords(fromBackChannel: backChannelReadEnd) return nil } - // This is a roundabout way of saying "and return the exit condition - // yielded by wait(for:)". - return try await taskGroup.compactMap { $0 }.first { _ in true }! + // Collate the various bits of the result. The exit condition .failure + // here is just a placeholder and will be replaced by the result of one + // of the tasks above. + var result = ExitTestArtifacts(exitCondition: .failure) + for try await update in taskGroup { + update?(&result) + } + return result } } } diff --git a/Sources/Testing/ExitTests/ExitTestArtifacts.swift b/Sources/Testing/ExitTests/ExitTestArtifacts.swift index 8a808262e..05fe3344d 100644 --- a/Sources/Testing/ExitTests/ExitTestArtifacts.swift +++ b/Sources/Testing/ExitTests/ExitTestArtifacts.swift @@ -30,6 +30,58 @@ public struct ExitTestArtifacts: Sendable { /// instances of ``ExitCondition`` with ``/Swift/Optional/==(_:_:)``. public var exitCondition: ExitCondition + /// All bytes written to the standard output stream of the exit test before + /// it exited. + /// + /// The value of this property may contain any arbitrary sequence of bytes, + /// including sequences that are not valid UTF-8 and cannot be decoded by + /// [`String.init(cString:)`](https://developer.apple.com/documentation/swift/string/init(cstring:)-6kr8s). + /// Consider using [`String.init(validatingCString:)`](https://developer.apple.com/documentation/swift/string/init(validatingcstring:)-992vo) + /// instead. + /// + /// When checking the value of this property, keep in mind that the standard + /// output stream is globally accessible, and any code running in an exit + /// test may write to it including including the operating system and any + /// third-party dependencies you have declared in your package. Rather than + /// comparing the value of this property with [`==`](https://developer.apple.com/documentation/swift/array/==(_:_:)), + /// use [`contains(_:)`](https://developer.apple.com/documentation/swift/collection/contains(_:)) + /// to check if expected output is present. + /// + /// To enable gathering output from the standard output stream during an + /// exit test, pass `\.standardOutputContent` in the `observedValues` + /// argument of ``expect(exitsWith:_:sourceLocation:performing:)`` or + /// ``require(exitsWith:_:sourceLocation:performing:)``. + /// + /// If the exit test could not be started or if you did not request standard + /// output content, the value of this property is the empty array. + public var standardOutputContent = [UInt8]() + + /// All bytes written to the standard error stream of the exit test before + /// it exited. + /// + /// The value of this property may contain any arbitrary sequence of bytes, + /// including sequences that are not valid UTF-8 and cannot be decoded by + /// [`String.init(cString:)`](https://developer.apple.com/documentation/swift/string/init(cstring:)-6kr8s). + /// Consider using [`String.init(validatingCString:)`](https://developer.apple.com/documentation/swift/string/init(validatingcstring:)-992vo) + /// instead. + /// + /// When checking the value of this property, keep in mind that the standard + /// output stream is globally accessible, and any code running in an exit + /// test may write to it including including the operating system and any + /// third-party dependencies you have declared in your package. Rather than + /// comparing the value of this property with [`==`](https://developer.apple.com/documentation/swift/array/==(_:_:)), + /// use [`contains(_:)`](https://developer.apple.com/documentation/swift/collection/contains(_:)) + /// to check if expected output is present. + /// + /// To enable gathering output from the standard error stream during an exit + /// test, pass `\.standardErrorContent` in the `observedValues` argument of + /// ``expect(exitsWith:_:sourceLocation:performing:)`` or + /// ``require(exitsWith:_:sourceLocation:performing:)``. + /// + /// If the exit test could not be started or if you did not request standard + /// error content, the value of this property is the empty array. + public var standardErrorContent = [UInt8]() + @_spi(ForToolsIntegrationOnly) public init(exitCondition: ExitCondition) { self.exitCondition = exitCondition diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index 8a079d0cc..c25d66b1b 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -29,6 +29,15 @@ typealias ProcessID = Never /// - arguments: The arguments to pass to the executable, not including the /// executable path. /// - environment: The environment block to pass to the executable. +/// - standardInput: If not `nil`, a file handle the child process should +/// inherit as its standard input stream. This file handle must be backed +/// by a file descriptor and be open for reading. +/// - standardOutput: If not `nil`, a file handle the child process should +/// inherit as its standard output stream. This file handle must be backed +/// by a file descriptor and be open for writing. +/// - standardError: If not `nil`, a file handle the child process should +/// inherit as its standard error stream. This file handle must be backed +/// by a file descriptor and be open for writing. /// - additionalFileHandles: A collection of file handles to inherit in the /// child process. /// @@ -42,7 +51,10 @@ func spawnExecutable( atPath executablePath: String, arguments: [String], environment: [String: String], - additionalFileHandles: UnsafeBufferPointer = .init(start: nil, count: 0) + standardInput: borrowing FileHandle? = nil, + standardOutput: borrowing FileHandle? = nil, + standardError: borrowing FileHandle? = nil, + additionalFileHandles: [UnsafePointer] = [] ) throws -> ProcessID { // Darwin and Linux differ in their optionality for the posix_spawn types we // use, so use this typealias to paper over the differences. @@ -88,27 +100,42 @@ func spawnExecutable( flags |= CShort(POSIX_SPAWN_SETSIGDEF) } - // Do not forward standard I/O. - _ = posix_spawn_file_actions_addopen(fileActions, STDIN_FILENO, "/dev/null", O_RDONLY, 0) - _ = posix_spawn_file_actions_addopen(fileActions, STDOUT_FILENO, "/dev/null", O_WRONLY, 0) - _ = posix_spawn_file_actions_addopen(fileActions, STDERR_FILENO, "/dev/null", O_WRONLY, 0) - + // Forward standard I/O streams and any explicitly added file handles. #if os(Linux) || os(FreeBSD) - var highestFD = CInt(0) + var highestFD = CInt(-1) #endif - for i in 0 ..< additionalFileHandles.count { - try additionalFileHandles[i].withUnsafePOSIXFileDescriptor { fd in + func inherit(_ fileHandle: borrowing FileHandle, as standardFD: CInt? = nil) throws { + try fileHandle.withUnsafePOSIXFileDescriptor { fd in guard let fd else { - throw SystemError(description: "A child process inherit a file handle without an associated file descriptor. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") + throw SystemError(description: "A child process cannot inherit a file handle without an associated file descriptor. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") } + if let standardFD { + _ = posix_spawn_file_actions_adddup2(fileActions, fd, standardFD) + } else { #if SWT_TARGET_OS_APPLE - _ = posix_spawn_file_actions_addinherit_np(fileActions, fd) + _ = posix_spawn_file_actions_addinherit_np(fileActions, fd) #elseif os(Linux) || os(FreeBSD) - highestFD = max(highestFD, fd) + highestFD = max(highestFD, fd) #endif + } + } + } + func inherit(_ fileHandle: borrowing FileHandle?, as standardFD: CInt? = nil) throws { + if fileHandle != nil { + try inherit(fileHandle!, as: standardFD) + } else if let standardFD { + let mode = (standardFD == STDIN_FILENO) ? O_RDONLY : O_WRONLY + _ = posix_spawn_file_actions_addopen(fileActions, standardFD, "/dev/null", mode, 0) } } + try inherit(standardInput, as: STDIN_FILENO) + try inherit(standardOutput, as: STDOUT_FILENO) + try inherit(standardError, as: STDERR_FILENO) + for additionalFileHandle in additionalFileHandles { + try inherit(additionalFileHandle.pointee) + } + #if SWT_TARGET_OS_APPLE // Close all other file descriptors open in the parent. flags |= CShort(POSIX_SPAWN_CLOEXEC_DEFAULT) @@ -152,6 +179,28 @@ func spawnExecutable( } #elseif os(Windows) return try _withStartupInfoEx(attributeCount: 1) { startupInfo in + func inherit(_ fileHandle: borrowing FileHandle, as outWindowsHANDLE: inout HANDLE?) throws { + try fileHandle.withUnsafeWindowsHANDLE { windowsHANDLE in + guard let windowsHANDLE else { + throw SystemError(description: "A child process cannot inherit a file handle without an associated Windows handle. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") + } + outWindowsHANDLE = windowsHANDLE + } + } + func inherit(_ fileHandle: borrowing FileHandle?, as outWindowsHANDLE: inout HANDLE?) throws { + if fileHandle != nil { + try inherit(fileHandle!, as: outWindowsHANDLE) + } else { + outWindowsHANDLE = nil + } + } + + // Forward standard I/O streams. + try inherit(standardInput, as: &startupInfo.pointee.StartupInfo.hStdInput) + try inherit(standardOutput, as: &startupInfo.pointee.StartupInfo.hStdOutput) + try inherit(standardError, as: &startupInfo.pointee.StartupInfo.hStdError) + startupInfo.pointee.StartupInfo.dwFlags |= STARTF_USESTDHANDLES + // Forward the back channel's write end to the child process so that it can // send information back to us. Note that we don't keep the pipe open as // bidirectional, though we could if we find we need to in the future. @@ -159,16 +208,9 @@ func spawnExecutable( defer { inheritedHandlesBuffer.deallocate() } - for i in 0 ..< additionalFileHandles.count { - try additionalFileHandles[i].withUnsafeWindowsHANDLE { handle in - guard let handle else { - throw SystemError(description: "A child process inherit a file handle without an associated Windows handle. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") - } - inheritedHandlesBuffer[i] = handle - } + for additionalFileHandle in additionalFileHandles { + try inherit(additionalFileHandle.pointee, as: &inheritedHandlesBuffer[i]) } - - // Update the attribute list to hold the handle buffer. _ = UpdateProcThreadAttribute( startupInfo.pointee.lpAttributeList, 0, diff --git a/Sources/Testing/Expectations/Expectation+Macro.swift b/Sources/Testing/Expectations/Expectation+Macro.swift index 35b09f5e7..832ada676 100644 --- a/Sources/Testing/Expectations/Expectation+Macro.swift +++ b/Sources/Testing/Expectations/Expectation+Macro.swift @@ -417,6 +417,9 @@ public macro require( /// /// - Parameters: /// - expectedExitCondition: The expected exit condition. +/// - observedValues: An array of key paths representing results from within +/// the exit test that should be observed and returned by this macro. The +/// ``ExitTest/Result/exitCondition`` property is always returned. /// - comment: A comment describing the expectation. /// - sourceLocation: The source location to which recorded expectations and /// issues should be attributed. @@ -458,6 +461,34 @@ public macro require( /// its exit status against `exitCondition`. If they match, the exit test has /// passed; otherwise, it has failed and an issue is recorded. /// +/// ## Child process output +/// +/// By default, the child process is configured without a standard output or +/// standard error stream. If your test needs to review the content of either of +/// these streams, you can pass its key path in the `observedValues` argument: +/// +/// ```swift +/// let result = await #expect( +/// exitsWith: .failure, +/// observing: [\.standardOutputContent] +/// ) { +/// print("Goodbye, world!") +/// fatalError() +/// } +/// #expect(result.standardOutputContent.contains(UInt8(ascii: "G"))) +/// ``` +/// +/// - Note: The content of the standard output and standard error streams may +/// contain any arbitrary sequence of bytes, including sequences that are not +/// valid UTF-8 and cannot be decoded by [`String.init(cString:)`](https://developer.apple.com/documentation/swift/string/init(cstring:)-6kr8s). +/// These streams are globally accessible within the child process, and any +/// code running in an exit test may write to it including including the +/// operating system and any third-party dependencies you have declared in +/// your package. +/// +/// The actual exit condition of the child process is always reported by the +/// testing library even if you do not specify it in `observedValues`. +/// /// ## Runtime constraints /// /// Exit tests cannot capture any state originating in the parent process or @@ -486,6 +517,7 @@ public macro require( @discardableResult @freestanding(expression) public macro expect( exitsWith expectedExitCondition: ExitCondition, + observing observedValues: Set> = [], _ comment: @autoclosure () -> Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation, performing expression: @convention(thin) () async throws -> Void @@ -496,6 +528,9 @@ public macro require( /// /// - Parameters: /// - expectedExitCondition: The expected exit condition. +/// - observedValues: An array of key paths representing results from within +/// the exit test that should be observed and returned by this macro. The +/// ``ExitTest/Result/exitCondition`` property is always returned. /// - comment: A comment describing the expectation. /// - sourceLocation: The source location to which recorded expectations and /// issues should be attributed. @@ -539,6 +574,34 @@ public macro require( /// its exit status against `exitCondition`. If they match, the exit test has /// passed; otherwise, it has failed and an issue is recorded. /// +/// ## Child process output +/// +/// By default, the child process is configured without a standard output or +/// standard error stream. If your test needs to review the content of either of +/// these streams, you can pass its key path in the `observedValues` argument: +/// +/// ```swift +/// let result = try await #require( +/// exitsWith: .failure, +/// observing: [\.standardOutputContent] +/// ) { +/// print("Goodbye, world!") +/// fatalError() +/// } +/// #expect(result.standardOutputContent.contains(UInt8(ascii: "G"))) +/// ``` +/// +/// - Note: The content of the standard output and standard error streams may +/// contain any arbitrary sequence of bytes, including sequences that are not +/// valid UTF-8 and cannot be decoded by [`String.init(cString:)`](https://developer.apple.com/documentation/swift/string/init(cstring:)-6kr8s). +/// These streams are globally accessible within the child process, and any +/// code running in an exit test may write to it including including the +/// operating system and any third-party dependencies you have declared in +/// your package. +/// +/// The actual exit condition of the child process is always reported by the +/// testing library even if you do not specify it in `observedValues`. +/// /// ## Runtime constraints /// /// Exit tests cannot capture any state originating in the parent process or @@ -567,6 +630,7 @@ public macro require( @discardableResult @freestanding(expression) public macro require( exitsWith expectedExitCondition: ExitCondition, + observing observedValues: Set> = [], _ comment: @autoclosure () -> Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation, performing expression: @convention(thin) () async throws -> Void diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 077d911b1..70fa17e85 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -1142,6 +1142,7 @@ public func __checkClosureCall( @_spi(Experimental) public func __checkClosureCall( exitsWith expectedExitCondition: ExitCondition, + observing observedValues: Set>, performing body: @convention(thin) () -> Void, expression: __Expression, comments: @autoclosure () -> [Comment], @@ -1151,6 +1152,7 @@ public func __checkClosureCall( ) async -> Result { await callExitTest( exitsWith: expectedExitCondition, + observing: observedValues, expression: expression, comments: comments(), isRequired: isRequired, diff --git a/Sources/Testing/Support/FileHandle.swift b/Sources/Testing/Support/FileHandle.swift index ac51622bd..293f16f5b 100644 --- a/Sources/Testing/Support/FileHandle.swift +++ b/Sources/Testing/Support/FileHandle.swift @@ -147,11 +147,7 @@ struct FileHandle: ~Copyable, Sendable { #endif guard let fileHandle else { let errorCode = swt_errno() -#if os(Windows) - _close(fd) -#else - _TestingInternals.close(fd) -#endif + Self._close(fd) throw CError(rawValue: errorCode) } self.init(unsafeCFILEHandle: fileHandle, closeWhenDone: true) @@ -174,6 +170,21 @@ struct FileHandle: ~Copyable, Sendable { _closeWhenDone = true } + /// Close a file descriptor. + /// + /// - Parameters: + /// - fd: The file descriptor to close. If the value of this argument is + /// less than `0`, this function does nothing. + private static func _close(_ fd: CInt) { + if fd >= 0 { +#if os(Windows) + _TestingInternals._close(fd) +#else + _TestingInternals.close(fd) +#endif + } + } + /// Call a function and pass the underlying C file handle to it. /// /// - Parameters: @@ -435,71 +446,58 @@ extension FileHandle { // MARK: - Pipes extension FileHandle { - /// A type representing a bidirectional pipe between two file handles. - struct Pipe: ~Copyable, Sendable { - /// The end of the pipe capable of reading. - var readEnd: FileHandle - - /// The end of the pipe capable of writing. - var writeEnd: FileHandle - - /// Initialize a new anonymous pipe. - /// - /// - Throws: Any error that prevented creation of the pipe. - init() throws { - let (fdReadEnd, fdWriteEnd) = try withUnsafeTemporaryAllocation(of: CInt.self, capacity: 2) { fds in + /// Make a pipe connecting two new file handles. + /// + /// - Parameters: + /// - readEnd: On successful return, set to a file handle that can read + /// bytes written to `writeEnd`. On failure, set to `nil`. + /// - writeEnd: On successful return, set to a file handle that can write + /// bytes read by `writeEnd`. On failure, set to `nil`. + /// + /// - Throws: Any error preventing creation of the pipe or corresponding file + /// handles. If an error occurs, both `readEnd` and `writeEnd` are set to + /// `nil` to avoid an inconsistent state. + /// + /// - Bug: This function should return a tuple containing the file handles + /// instead of returning them via `inout` arguments. Swift does not support + /// tuples with move-only elements. ([104669935](rdar://104669935)) + static func makePipe(readEnd: inout FileHandle?, writeEnd: inout FileHandle?) throws { + var (fdReadEnd, fdWriteEnd) = try withUnsafeTemporaryAllocation(of: CInt.self, capacity: 2) { fds in #if os(Windows) - guard 0 == _pipe(fds.baseAddress, 0, _O_BINARY) else { - throw CError(rawValue: swt_errno()) - } -#else - guard 0 == pipe(fds.baseAddress!) else { - throw CError(rawValue: swt_errno()) - } -#endif - return (fds[0], fds[1]) + guard 0 == _pipe(fds.baseAddress, 0, _O_BINARY) else { + throw CError(rawValue: swt_errno()) } - - // NOTE: Partial initialization of a move-only type is disallowed, as is - // conditional initialization of a local move-only value, which is why - // this section looks a little awkward. - let readEnd: FileHandle - do { - readEnd = try FileHandle(unsafePOSIXFileDescriptor: fdReadEnd, mode: "rb") - } catch { -#if os(Windows) - _close(fdWriteEnd) #else - _TestingInternals.close(fdWriteEnd) -#endif - throw error + guard 0 == pipe(fds.baseAddress!) else { + throw CError(rawValue: swt_errno()) } - let writeEnd = try FileHandle(unsafePOSIXFileDescriptor: fdWriteEnd, mode: "wb") - self.readEnd = readEnd - self.writeEnd = writeEnd +#endif + return (fds[0], fds[1]) } - - /// Close the read end of this pipe. - /// - /// - Returns: The remaining open end of the pipe. - /// - /// After calling this function, the read end is closed and the write end - /// remains open. - consuming func closeReadEnd() -> FileHandle { - readEnd.close() - return writeEnd + defer { + Self._close(fdReadEnd) + Self._close(fdWriteEnd) } - /// Close the write end of this pipe. - /// - /// - Returns: The remaining open end of the pipe. - /// - /// After calling this function, the write end is closed and the read end - /// remains open. - consuming func closeWriteEnd() -> FileHandle { - writeEnd.close() - return readEnd + do { + defer { + fdReadEnd = -1 + } + try readEnd = FileHandle(unsafePOSIXFileDescriptor: fdReadEnd, mode: "rb") + defer { + fdWriteEnd = -1 + } + try writeEnd = FileHandle(unsafePOSIXFileDescriptor: fdWriteEnd, mode: "wb") + } catch { + // Don't leak file handles! Ensure we've cleared both pointers before + // returning so the state is consistent in the caller. + readEnd = nil + writeEnd = nil + + throw error } + + print(fdReadEnd, fdWriteEnd) } } #endif diff --git a/Sources/TestingMacros/ConditionMacro.swift b/Sources/TestingMacros/ConditionMacro.swift index 92daafb2a..341b27d7d 100644 --- a/Sources/TestingMacros/ConditionMacro.swift +++ b/Sources/TestingMacros/ConditionMacro.swift @@ -373,6 +373,13 @@ extension ExitTestConditionMacro { guard let expectedExitConditionIndex else { fatalError("Could not find the exit condition for this exit test. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") } + let observationListIndex = arguments.firstIndex { $0.label?.tokenKind == .identifier("observing") } + if observationListIndex == nil { + arguments.insert( + Argument(label: "observing", expression: ArrayExprSyntax(expressions: [])), + at: arguments.index(after: expectedExitConditionIndex) + ) + } let trailingClosureIndex = arguments.firstIndex { $0.label?.tokenKind == _trailingClosureLabel.tokenKind } guard let trailingClosureIndex else { fatalError("Could not find the body argument to this exit test. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") diff --git a/Tests/TestingTests/ExitTestTests.swift b/Tests/TestingTests/ExitTestTests.swift index 22da6691b..7fd3e4dcf 100644 --- a/Tests/TestingTests/ExitTestTests.swift +++ b/Tests/TestingTests/ExitTestTests.swift @@ -383,6 +383,27 @@ private import _TestingInternals } } } + + @Test("Result contains stdout/stderr") + func exitTestResultContainsStandardStreams() async throws { + var result = try await #require(exitsWith: .success, observing: [\.standardOutputContent]) { + try FileHandle.stdout.write("STANDARD OUTPUT") + try FileHandle.stderr.write(String("STANDARD ERROR".reversed())) + exit(EXIT_SUCCESS) + } + #expect(result.exitCondition === .success) + #expect(result.standardOutputContent.contains("STANDARD OUTPUT".utf8)) + #expect(result.standardErrorContent.isEmpty) + + result = try await #require(exitsWith: .success, observing: [\.standardErrorContent]) { + try FileHandle.stdout.write("STANDARD OUTPUT") + try FileHandle.stderr.write(String("STANDARD ERROR".reversed())) + exit(EXIT_SUCCESS) + } + #expect(result.exitCondition === .success) + #expect(result.standardOutputContent.isEmpty) + #expect(result.standardErrorContent.contains("STANDARD ERROR".utf8.reversed())) + } } // MARK: - Fixtures diff --git a/Tests/TestingTests/Support/FileHandleTests.swift b/Tests/TestingTests/Support/FileHandleTests.swift index 5d93a249f..c7f347357 100644 --- a/Tests/TestingTests/Support/FileHandleTests.swift +++ b/Tests/TestingTests/Support/FileHandleTests.swift @@ -152,9 +152,11 @@ struct FileHandleTests { #if !SWT_NO_PIPES @Test("Can recognize opened pipe") func isPipe() throws { - let pipe = try FileHandle.Pipe() - #expect(pipe.readEnd.isPipe as Bool) - #expect(pipe.writeEnd.isPipe as Bool) + var readEnd: FileHandle! + var writeEnd: FileHandle! + try FileHandle.makePipe(readEnd: &readEnd, writeEnd: &writeEnd) + #expect(readEnd.isPipe as Bool) + #expect(writeEnd.isPipe as Bool) } #endif @@ -162,13 +164,17 @@ struct FileHandleTests { @Test("Can close ends of a pipe") func closeEndsOfPipe() async throws { try await confirmation("File handle closed", expectedCount: 2) { closed in - var pipe1 = try FileHandle.Pipe() - pipe1.readEnd = try fileHandleForCloseMonitoring(with: closed) - _ = pipe1.closeReadEnd() + var pipe1ReadEnd: FileHandle! + var pipe1WriteEnd: FileHandle! + try FileHandle.makePipe(readEnd: &pipe1ReadEnd, writeEnd: &pipe1WriteEnd) + pipe1ReadEnd = try fileHandleForCloseMonitoring(with: closed) + pipe1ReadEnd.close() - var pipe2 = try FileHandle.Pipe() - pipe2.writeEnd = try fileHandleForCloseMonitoring(with: closed) - _ = pipe2.closeWriteEnd() + var pipe2ReadEnd: FileHandle! + var pipe2WriteEnd: FileHandle! + try FileHandle.makePipe(readEnd: &pipe2ReadEnd, writeEnd: &pipe2WriteEnd) + pipe2WriteEnd = try fileHandleForCloseMonitoring(with: closed) + pipe2WriteEnd.close() } } #endif From 5eddb68753ee5ae197b1cc4db16604fe770bbac8 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 18 Oct 2024 17:25:10 -0400 Subject: [PATCH 02/13] Fix Windows --- Sources/Testing/ExitTests/SpawnProcess.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index c25d66b1b..6bdd024fd 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -208,8 +208,8 @@ func spawnExecutable( defer { inheritedHandlesBuffer.deallocate() } - for additionalFileHandle in additionalFileHandles { - try inherit(additionalFileHandle.pointee, as: &inheritedHandlesBuffer[i]) + for i in 0 ..< additionalFileHandles.count { + try inherit(additionalFileHandles[i].pointee, as: &inheritedHandlesBuffer[i]) } _ = UpdateProcThreadAttribute( startupInfo.pointee.lpAttributeList, From ac3b0557670f208d51b4d43404bd0770ff79a48b Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 19 Oct 2024 10:44:23 -0400 Subject: [PATCH 03/13] Fix typos --- Sources/Testing/ExitTests/SpawnProcess.swift | 2 +- Sources/Testing/Expectations/Expectation+Macro.swift | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index 6bdd024fd..a6f9ef47e 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -189,7 +189,7 @@ func spawnExecutable( } func inherit(_ fileHandle: borrowing FileHandle?, as outWindowsHANDLE: inout HANDLE?) throws { if fileHandle != nil { - try inherit(fileHandle!, as: outWindowsHANDLE) + try inherit(fileHandle!, as: &outWindowsHANDLE) } else { outWindowsHANDLE = nil } diff --git a/Sources/Testing/Expectations/Expectation+Macro.swift b/Sources/Testing/Expectations/Expectation+Macro.swift index 832ada676..3b2698d7f 100644 --- a/Sources/Testing/Expectations/Expectation+Macro.swift +++ b/Sources/Testing/Expectations/Expectation+Macro.swift @@ -482,9 +482,8 @@ public macro require( /// contain any arbitrary sequence of bytes, including sequences that are not /// valid UTF-8 and cannot be decoded by [`String.init(cString:)`](https://developer.apple.com/documentation/swift/string/init(cstring:)-6kr8s). /// These streams are globally accessible within the child process, and any -/// code running in an exit test may write to it including including the -/// operating system and any third-party dependencies you have declared in -/// your package. +/// code running in an exit test may write to it including the operating +/// system and any third-party dependencies you have declared in your package. /// /// The actual exit condition of the child process is always reported by the /// testing library even if you do not specify it in `observedValues`. @@ -595,9 +594,8 @@ public macro require( /// contain any arbitrary sequence of bytes, including sequences that are not /// valid UTF-8 and cannot be decoded by [`String.init(cString:)`](https://developer.apple.com/documentation/swift/string/init(cstring:)-6kr8s). /// These streams are globally accessible within the child process, and any -/// code running in an exit test may write to it including including the -/// operating system and any third-party dependencies you have declared in -/// your package. +/// code running in an exit test may write to it including the operating +/// system and any third-party dependencies you have declared in your package. /// /// The actual exit condition of the child process is always reported by the /// testing library even if you do not specify it in `observedValues`. From aa50afd64b913e19abbd07b9d5691479d0ed243e Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 19 Oct 2024 10:49:45 -0400 Subject: [PATCH 04/13] Update docs --- Sources/Testing/ExitTests/ExitCondition.swift | 8 ++++---- Sources/Testing/ExitTests/ExitTest.swift | 14 +++++++++----- Sources/Testing/ExitTests/ExitTestArtifacts.swift | 8 ++++---- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitCondition.swift b/Sources/Testing/ExitTests/ExitCondition.swift index 2950b18e7..0edbd94bd 100644 --- a/Sources/Testing/ExitTests/ExitCondition.swift +++ b/Sources/Testing/ExitTests/ExitCondition.swift @@ -13,10 +13,10 @@ private import _TestingInternals /// An enumeration describing possible conditions under which a process will /// exit. /// -/// Values of this type are used to describe the conditions under which an exit -/// test is expected to pass or fail by passing them to -/// ``expect(exitsWith:_:sourceLocation:performing:)`` or -/// ``require(exitsWith:_:sourceLocation:performing:)``. +/// Values of this type can be passed to +/// ``expect(exitsWith:observing:_:sourceLocation:performing:)`` or +/// ``require(exitsWith:observing:_:sourceLocation:performing:)`` to configure +/// which exit statuses should be considered successful. @_spi(Experimental) #if SWT_NO_PROCESS_SPAWNING @available(*, unavailable, message: "Exit tests are not available on this platform.") diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 1859a111f..092256fbf 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -307,11 +307,15 @@ extension ExitTest { /// the exit test. /// /// This handler is invoked when an exit test (i.e. a call to either - /// ``expect(exitsWith:_:sourceLocation:performing:)`` or - /// ``require(exitsWith:_:sourceLocation:performing:)``) is started. The - /// handler is responsible for initializing a new child environment (typically - /// a child process) and running the exit test identified by `sourceLocation` - /// there. The exit test's body can be found using ``ExitTest/find(at:)``. + /// ``expect(exitsWith:observing:_:sourceLocation:performing:)`` or + /// ``require(exitsWith:observing:_:sourceLocation:performing:)``) is started. + /// The handler is responsible for initializing a new child environment + /// (typically a child process) and running the exit test identified by + /// `sourceLocation` there. + /// + /// In the child environment, you can find the exit test again using + /// ``ExitTest/find(at:)`` and can be run by calling + /// ``ExitTest/callAsFunction()``. /// /// The parent environment should suspend until the results of the exit test /// are available or the child environment is otherwise terminated. The parent diff --git a/Sources/Testing/ExitTests/ExitTestArtifacts.swift b/Sources/Testing/ExitTests/ExitTestArtifacts.swift index 05fe3344d..71ace9b10 100644 --- a/Sources/Testing/ExitTests/ExitTestArtifacts.swift +++ b/Sources/Testing/ExitTests/ExitTestArtifacts.swift @@ -49,8 +49,8 @@ public struct ExitTestArtifacts: Sendable { /// /// To enable gathering output from the standard output stream during an /// exit test, pass `\.standardOutputContent` in the `observedValues` - /// argument of ``expect(exitsWith:_:sourceLocation:performing:)`` or - /// ``require(exitsWith:_:sourceLocation:performing:)``. + /// argument of ``expect(exitsWith:observing:_:sourceLocation:performing:)`` + /// or ``require(exitsWith:observing:_:sourceLocation:performing:)``. /// /// If the exit test could not be started or if you did not request standard /// output content, the value of this property is the empty array. @@ -75,8 +75,8 @@ public struct ExitTestArtifacts: Sendable { /// /// To enable gathering output from the standard error stream during an exit /// test, pass `\.standardErrorContent` in the `observedValues` argument of - /// ``expect(exitsWith:_:sourceLocation:performing:)`` or - /// ``require(exitsWith:_:sourceLocation:performing:)``. + /// ``expect(exitsWith:observing:_:sourceLocation:performing:)`` or + /// ``require(exitsWith:observing:_:sourceLocation:performing:)``. /// /// If the exit test could not be started or if you did not request standard /// error content, the value of this property is the empty array. From c9a46caef09e67dcfa089ba3fdf52c47d58fc437 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 19 Oct 2024 10:53:19 -0400 Subject: [PATCH 05/13] Typo --- Sources/Testing/ExitTests/ExitTest.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 092256fbf..43a8c2d1c 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -313,8 +313,8 @@ extension ExitTest { /// (typically a child process) and running the exit test identified by /// `sourceLocation` there. /// - /// In the child environment, you can find the exit test again using - /// ``ExitTest/find(at:)`` and can be run by calling + /// In the child environment, you can find the exit test again by calling + /// ``ExitTest/find(at:)`` and can run it by calling /// ``ExitTest/callAsFunction()``. /// /// The parent environment should suspend until the results of the exit test From c48ce8b73902dcc8215e9b786048554a5b7f5450 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 19 Oct 2024 11:06:13 -0400 Subject: [PATCH 06/13] Remove stray print --- Sources/Testing/Support/FileHandle.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/Testing/Support/FileHandle.swift b/Sources/Testing/Support/FileHandle.swift index 293f16f5b..e7b75e57a 100644 --- a/Sources/Testing/Support/FileHandle.swift +++ b/Sources/Testing/Support/FileHandle.swift @@ -496,8 +496,6 @@ extension FileHandle { throw error } - - print(fdReadEnd, fdWriteEnd) } } #endif From 5aff959f5ebf8da03cb2665e19db789d43661d1d Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 19 Oct 2024 11:25:54 -0400 Subject: [PATCH 07/13] Ensure stdio handles are inherited on Windows --- Sources/Testing/ExitTests/SpawnProcess.swift | 79 ++++++++++---------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/Sources/Testing/ExitTests/SpawnProcess.swift b/Sources/Testing/ExitTests/SpawnProcess.swift index a6f9ef47e..79099c072 100644 --- a/Sources/Testing/ExitTests/SpawnProcess.swift +++ b/Sources/Testing/ExitTests/SpawnProcess.swift @@ -201,50 +201,53 @@ func spawnExecutable( try inherit(standardError, as: &startupInfo.pointee.StartupInfo.hStdError) startupInfo.pointee.StartupInfo.dwFlags |= STARTF_USESTDHANDLES - // Forward the back channel's write end to the child process so that it can - // send information back to us. Note that we don't keep the pipe open as - // bidirectional, though we could if we find we need to in the future. - let inheritedHandlesBuffer = UnsafeMutableBufferPointer.allocate(capacity: additionalFileHandles.count) - defer { - inheritedHandlesBuffer.deallocate() - } + // Ensure standard I/O streams and any explicitly added file handles are + // inherited by the child process. + var inheritedHandles = [HANDLE?](repeating: nil, count: additionalFileHandles.count + 3) + try inherit(standardInput, as: &inheritedHandles[0]) + try inherit(standardOutput, as: &inheritedHandles[1]) + try inherit(standardError, as: &inheritedHandles[2]) for i in 0 ..< additionalFileHandles.count { - try inherit(additionalFileHandles[i].pointee, as: &inheritedHandlesBuffer[i]) + try inherit(additionalFileHandles[i].pointee, as: &inheritedHandles[i + 3]) } - _ = UpdateProcThreadAttribute( - startupInfo.pointee.lpAttributeList, - 0, - swt_PROC_THREAD_ATTRIBUTE_HANDLE_LIST(), - inheritedHandlesBuffer.baseAddress!, - SIZE_T(MemoryLayout.stride * inheritedHandlesBuffer.count), - nil, - nil - ) + inheritedHandles = inheritedHandles.compactMap(\.self) - let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments) - let environ = environment.map { "\($0.key)=\($0.value)" }.joined(separator: "\0") + "\0\0" + return try inheritedHandles.withUnsafeMutableBufferPointer { inheritedHandles in + _ = UpdateProcThreadAttribute( + startupInfo.pointee.lpAttributeList, + 0, + swt_PROC_THREAD_ATTRIBUTE_HANDLE_LIST(), + inheritedHandles.baseAddress!, + SIZE_T(MemoryLayout.stride * inheritedHandles.count), + nil, + nil + ) - return try commandLine.withCString(encodedAs: UTF16.self) { commandLine in - try environ.withCString(encodedAs: UTF16.self) { environ in - var processInfo = PROCESS_INFORMATION() + let commandLine = _escapeCommandLine(CollectionOfOne(executablePath) + arguments) + let environ = environment.map { "\($0.key)=\($0.value)" }.joined(separator: "\0") + "\0\0" - guard CreateProcessW( - nil, - .init(mutating: commandLine), - nil, - nil, - true, // bInheritHandles - DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT), - .init(mutating: environ), - nil, - startupInfo.pointer(to: \.StartupInfo)!, - &processInfo - ) else { - throw Win32Error(rawValue: GetLastError()) - } - _ = CloseHandle(processInfo.hThread) + return try commandLine.withCString(encodedAs: UTF16.self) { commandLine in + try environ.withCString(encodedAs: UTF16.self) { environ in + var processInfo = PROCESS_INFORMATION() - return processInfo.hProcess! + guard CreateProcessW( + nil, + .init(mutating: commandLine), + nil, + nil, + true, // bInheritHandles + DWORD(CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT), + .init(mutating: environ), + nil, + startupInfo.pointer(to: \.StartupInfo)!, + &processInfo + ) else { + throw Win32Error(rawValue: GetLastError()) + } + _ = CloseHandle(processInfo.hThread) + + return processInfo.hProcess! + } } } } From 9114a83c6600bf9df5b45d97a1ba80b4ef6adb41 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Sat, 19 Oct 2024 21:22:41 -0400 Subject: [PATCH 08/13] observedValues does not need to be a set --- Sources/Testing/ExitTests/ExitTest.swift | 10 ++++++---- Sources/Testing/Expectations/Expectation+Macro.swift | 4 ++-- .../Expectations/ExpectationChecking+Macro.swift | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 43a8c2d1c..ba179c298 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -35,7 +35,7 @@ public struct ExitTest: Sendable, ~Copyable { fileprivate var body: @Sendable () async throws -> Void = {} /// Storage for ``observedValues``. - fileprivate nonisolated(unsafe) var _observedValues = Set>() + fileprivate nonisolated(unsafe) var _observedValues = [PartialKeyPath]() /// Key paths representing results from within this exit test that should be /// observed and returned to the caller. @@ -52,10 +52,12 @@ public struct ExitTest: Sendable, ~Copyable { /// Within a child process running an exit test, the value of this property is /// otherwise unspecified. @_spi(ForToolsIntegrationOnly) - public var observedValues: Set> { + public var observedValues: [PartialKeyPath] { get { var result = _observedValues - result.insert(\.exitCondition) + if !result.contains(\.exitCondition) { // O(n), but n <= 3 (no Set needed) + result.append(\.exitCondition) + } return result } set { @@ -228,7 +230,7 @@ extension ExitTest { /// convention. func callExitTest( exitsWith expectedExitCondition: ExitCondition, - observing observedValues: Set>, + observing observedValues: [PartialKeyPath], expression: __Expression, comments: @autoclosure () -> [Comment], isRequired: Bool, diff --git a/Sources/Testing/Expectations/Expectation+Macro.swift b/Sources/Testing/Expectations/Expectation+Macro.swift index 3b2698d7f..a85b1490a 100644 --- a/Sources/Testing/Expectations/Expectation+Macro.swift +++ b/Sources/Testing/Expectations/Expectation+Macro.swift @@ -516,7 +516,7 @@ public macro require( @discardableResult @freestanding(expression) public macro expect( exitsWith expectedExitCondition: ExitCondition, - observing observedValues: Set> = [], + observing observedValues: [PartialKeyPath] = [], _ comment: @autoclosure () -> Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation, performing expression: @convention(thin) () async throws -> Void @@ -628,7 +628,7 @@ public macro require( @discardableResult @freestanding(expression) public macro require( exitsWith expectedExitCondition: ExitCondition, - observing observedValues: Set> = [], + observing observedValues: [PartialKeyPath] = [], _ comment: @autoclosure () -> Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation, performing expression: @convention(thin) () async throws -> Void diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 70fa17e85..60ff81a03 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -1142,7 +1142,7 @@ public func __checkClosureCall( @_spi(Experimental) public func __checkClosureCall( exitsWith expectedExitCondition: ExitCondition, - observing observedValues: Set>, + observing observedValues: [PartialKeyPath], performing body: @convention(thin) () -> Void, expression: __Expression, comments: @autoclosure () -> [Comment], From 375f6af0073120ba5162ad2edf4964d7ad557b66 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 22 Oct 2024 10:41:13 -0400 Subject: [PATCH 09/13] Rebase on jgrynspan/exit-test-result-optional --- .../Testing/ExitTests/ExitTestArtifacts.swift | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTestArtifacts.swift b/Sources/Testing/ExitTests/ExitTestArtifacts.swift index 71ace9b10..6696c791b 100644 --- a/Sources/Testing/ExitTests/ExitTestArtifacts.swift +++ b/Sources/Testing/ExitTests/ExitTestArtifacts.swift @@ -11,9 +11,9 @@ /// A type representing the result of an exit test after it has exited and /// returned control to the calling test function. /// -/// Both ``expect(exitsWith:_:sourceLocation:performing:)`` and -/// ``require(exitsWith:_:sourceLocation:performing:)`` return instances of -/// this type. +/// Both ``expect(exitsWith:observing:_:sourceLocation:performing:)`` and +/// ``require(exitsWith:observing:_:sourceLocation:performing:)`` return +/// instances of this type. /// /// - Warning: The name of this type is still unstable and subject to change. @_spi(Experimental) @@ -25,9 +25,10 @@ public struct ExitTestArtifacts: Sendable { /// /// When the exit test passes, the value of this property is equal to the /// value of the `expectedExitCondition` argument passed to - /// ``expect(exitsWith:_:sourceLocation:performing:)`` or to - /// ``require(exitsWith:_:sourceLocation:performing:)``. You can compare two - /// instances of ``ExitCondition`` with ``/Swift/Optional/==(_:_:)``. + /// ``expect(exitsWith:observing:_:sourceLocation:performing:)`` or to + /// ``require(exitsWith:observing:_:sourceLocation:performing:)``. You can + /// compare two instances of ``ExitCondition`` with + /// ``/Swift/Optional/==(_:_:)``. public var exitCondition: ExitCondition /// All bytes written to the standard output stream of the exit test before @@ -52,8 +53,8 @@ public struct ExitTestArtifacts: Sendable { /// argument of ``expect(exitsWith:observing:_:sourceLocation:performing:)`` /// or ``require(exitsWith:observing:_:sourceLocation:performing:)``. /// - /// If the exit test could not be started or if you did not request standard - /// output content, the value of this property is the empty array. + /// If you did not request standard output content when running an exit test, + /// the value of this property is the empty array. public var standardOutputContent = [UInt8]() /// All bytes written to the standard error stream of the exit test before @@ -78,8 +79,8 @@ public struct ExitTestArtifacts: Sendable { /// ``expect(exitsWith:observing:_:sourceLocation:performing:)`` or /// ``require(exitsWith:observing:_:sourceLocation:performing:)``. /// - /// If the exit test could not be started or if you did not request standard - /// error content, the value of this property is the empty array. + /// If you did not request standard error content when running an exit test, + /// the value of this property is the empty array. public var standardErrorContent = [UInt8]() @_spi(ForToolsIntegrationOnly) From 1d3b1d44bc2b687f7468602c81433a1085282f95 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 22 Oct 2024 13:29:08 -0400 Subject: [PATCH 10/13] Make result type of expectations optional for consistency with #780 --- Sources/Testing/ExitTests/ExitTest.swift | 2 +- .../ExpectationChecking+Macro.swift | 2 +- .../Support/Additions/ResultAdditions.swift | 19 ++++++++++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index ba179c298..db38423c5 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -236,7 +236,7 @@ func callExitTest( isRequired: Bool, isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation -) async -> Result { +) async -> Result { guard let configuration = Configuration.current ?? Configuration.all.first else { preconditionFailure("A test must be running on the current task to use #expect(exitsWith:).") } diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 60ff81a03..09a5964e8 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -1149,7 +1149,7 @@ public func __checkClosureCall( isRequired: Bool, isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation -) async -> Result { +) async -> Result { await callExitTest( exitsWith: expectedExitCondition, observing: observedValues, diff --git a/Sources/Testing/Support/Additions/ResultAdditions.swift b/Sources/Testing/Support/Additions/ResultAdditions.swift index b0101b4ac..f14f68c85 100644 --- a/Sources/Testing/Support/Additions/ResultAdditions.swift +++ b/Sources/Testing/Support/Additions/ResultAdditions.swift @@ -15,11 +15,23 @@ extension Result { /// `#require()` macros. Do not call it directly. @inlinable public func __expected() where Success == Void {} + /// Handle this instance as if it were returned from a call to `#require()`. + /// + /// - Warning: This function is used to implement the `#expect()` and + /// `#require()` macros. Do not call it directly. + @inlinable public func __required() throws -> Success { + try get() + } +} + +// MARK: - Optional success values + +extension Result { /// Handle this instance as if it were returned from a call to `#expect()`. /// /// - Warning: This function is used to implement the `#expect()` and /// `#require()` macros. Do not call it directly. - @inlinable public func __expected() -> Success? { + @inlinable public func __expected() -> Success where Success == T? { try? get() } @@ -27,7 +39,8 @@ extension Result { /// /// - Warning: This function is used to implement the `#expect()` and /// `#require()` macros. Do not call it directly. - @inlinable public func __required() throws -> Success { - try get() + @inlinable public func __required() throws -> T where Success == T? { + // TODO: handle edge case where the value is nil (see #780) + try get()! } } From 4b8e7a841b18bf73d59232e18705172ab8c93201 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 22 Oct 2024 13:31:12 -0400 Subject: [PATCH 11/13] Update documentation comment --- Sources/Testing/Expectations/Expectation+Macro.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/Testing/Expectations/Expectation+Macro.swift b/Sources/Testing/Expectations/Expectation+Macro.swift index a85b1490a..6d424f046 100644 --- a/Sources/Testing/Expectations/Expectation+Macro.swift +++ b/Sources/Testing/Expectations/Expectation+Macro.swift @@ -475,7 +475,9 @@ public macro require( /// print("Goodbye, world!") /// fatalError() /// } -/// #expect(result.standardOutputContent.contains(UInt8(ascii: "G"))) +/// if let result { +/// #expect(result.standardOutputContent.contains(UInt8(ascii: "G"))) +/// } /// ``` /// /// - Note: The content of the standard output and standard error streams may From a57d90964b272c7ee5e51b3a468eb36f021fb2ac Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 22 Oct 2024 14:59:30 -0400 Subject: [PATCH 12/13] Update DocC links --- Sources/Testing/ExitTests/ExitCondition.swift | 6 +++--- Sources/Testing/Expectations/Expectation+Macro.swift | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitCondition.swift b/Sources/Testing/ExitTests/ExitCondition.swift index 0edbd94bd..d589b5367 100644 --- a/Sources/Testing/ExitTests/ExitCondition.swift +++ b/Sources/Testing/ExitTests/ExitCondition.swift @@ -13,10 +13,10 @@ private import _TestingInternals /// An enumeration describing possible conditions under which a process will /// exit. /// -/// Values of this type can be passed to +/// Values of this type are used to describe the conditions under which an exit +/// test is expected to pass or fail by passing them to /// ``expect(exitsWith:observing:_:sourceLocation:performing:)`` or -/// ``require(exitsWith:observing:_:sourceLocation:performing:)`` to configure -/// which exit statuses should be considered successful. +/// ``require(exitsWith:observing:_:sourceLocation:performing:)``. @_spi(Experimental) #if SWT_NO_PROCESS_SPAWNING @available(*, unavailable, message: "Exit tests are not available on this platform.") diff --git a/Sources/Testing/Expectations/Expectation+Macro.swift b/Sources/Testing/Expectations/Expectation+Macro.swift index 6d424f046..27a2ba806 100644 --- a/Sources/Testing/Expectations/Expectation+Macro.swift +++ b/Sources/Testing/Expectations/Expectation+Macro.swift @@ -419,7 +419,7 @@ public macro require( /// - expectedExitCondition: The expected exit condition. /// - observedValues: An array of key paths representing results from within /// the exit test that should be observed and returned by this macro. The -/// ``ExitTest/Result/exitCondition`` property is always returned. +/// ``ExitTestArtifacts/exitCondition`` property is always returned. /// - comment: A comment describing the expectation. /// - sourceLocation: The source location to which recorded expectations and /// issues should be attributed. @@ -531,7 +531,7 @@ public macro require( /// - expectedExitCondition: The expected exit condition. /// - observedValues: An array of key paths representing results from within /// the exit test that should be observed and returned by this macro. The -/// ``ExitTest/Result/exitCondition`` property is always returned. +/// ``ExitTestArtifacts/exitCondition`` property is always returned. /// - comment: A comment describing the expectation. /// - sourceLocation: The source location to which recorded expectations and /// issues should be attributed. From 6c3ca16c4a0ad706bbe30c9606b62a44e5d8935c Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 22 Oct 2024 16:18:23 -0400 Subject: [PATCH 13/13] Incorporate feedback --- Sources/Testing/ExitTests/ExitTest.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index db38423c5..b3bf6ce4d 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -35,6 +35,10 @@ public struct ExitTest: Sendable, ~Copyable { fileprivate var body: @Sendable () async throws -> Void = {} /// Storage for ``observedValues``. + /// + /// Key paths are not sendable because the properties they refer to may or may + /// not be, so this property needs to be `nonisolated(unsafe)`. It is safe to + /// use it in this fashion because `ExitTestArtifacts` is sendable. fileprivate nonisolated(unsafe) var _observedValues = [PartialKeyPath]() /// Key paths representing results from within this exit test that should be @@ -215,6 +219,9 @@ extension ExitTest { /// /// - Parameters: /// - expectedExitCondition: The expected exit condition. +/// - observedValues: An array of key paths representing results from within +/// the exit test that should be observed and returned by this macro. The +/// ``ExitTestArtifacts/exitCondition`` property is always returned. /// - expression: The expression, corresponding to `condition`, that is being /// evaluated (if available at compile time.) /// - comments: An array of comments describing the expectation. This array