Make the result of an exit test optional with #expect. (#779)

This PR changes the semantics of `#expect(exitsWith:)` such that it only
returns a value on success, and returns `nil` on failure. This is
consistent with general Swift semantics.

This means that if you have an exit test that fails to exit with the
expected exit condition, you won't get a value back, but that's
intentional now because your test will have already failed. This makes
it easier to reason about properties we add to the result structure in
the future: they'll always be valid if you have an instance of the
structure because you can't get a half-baked instance back by mistake.

This change made me realize that our custom comparison operators on
`ExitCondition` do not compare against `nil` correctly, so I've moved
them to `Optional<ExitCondition>`. (I know @stmontgomery is interested
in revisiting whether they should be operators at all, but let's leave
that discussion for a separate PR or thread.)

I also renamed the structure `ExitTestArtifacts` for two reasons:

1. To get it out from under `ExitTest` so that that type does not need
to be API, and
2. To give it an even uglier name so we are forced to debate what to
call it when the feature comes up for review.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.

Author: grynspan

Sources/Testing/CMakeLists.txt

@@ -29,7 +29,7 @@ add_library(Testing
   Events/TimeValue.swift
   ExitTests/ExitCondition.swift
   ExitTests/ExitTest.swift
-  ExitTests/ExitTest.Result.swift
+  ExitTests/ExitTestArtifacts.swift
   ExitTests/SpawnProcess.swift
   ExitTests/WaitFor.swift
   Expectations/Expectation.swift

Sources/Testing/ExitTests/ExitCondition.swift

@@ -10,22 +10,26 @@
 
 private import _TestingInternals
 
-/// An enumeration describing possible conditions under which an exit test will
-/// succeed or fail.
+/// 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:_:sourceLocation:performing:)`` or
-/// ``require(exitsWith:_:sourceLocation:performing:)`` to configure which exit
-/// statuses should be considered successful.
+/// ``require(exitsWith:_:sourceLocation:performing:)``.
 @_spi(Experimental)
 #if SWT_NO_PROCESS_SPAWNING
 @available(*, unavailable, message: "Exit tests are not available on this platform.")
-#elseif SWT_NO_EXIT_TESTS
-@_spi(ForToolsIntegrationOnly)
 #endif
 public enum ExitCondition: Sendable {
   /// The process terminated successfully with status `EXIT_SUCCESS`.
-  public static var success: Self { .exitCode(EXIT_SUCCESS) }
+  public static var success: Self {
+    // Strictly speaking, the C standard treats 0 as a successful exit code and
+    // potentially distinct from EXIT_SUCCESS. To my knowledge, no modern
+    // operating system defines EXIT_SUCCESS to any value other than 0, so the
+    // distinction is academic.
+    .exitCode(EXIT_SUCCESS)
+  }
 
   /// The process terminated abnormally with any status other than
   /// `EXIT_SUCCESS` or with any signal.
@@ -72,23 +76,23 @@ public enum ExitCondition: Sendable {
 
 // MARK: - Equatable
 
+@_spi(Experimental)
 #if SWT_NO_PROCESS_SPAWNING
 @available(*, unavailable, message: "Exit tests are not available on this platform.")
-#elseif SWT_NO_EXIT_TESTS
-@_spi(ForToolsIntegrationOnly)
 #endif
-extension ExitCondition {
-  /// Check whether or not two values of this type are equal.
+extension Optional<ExitCondition> {
+  /// Check whether or not two exit conditions are equal.
   ///
   /// - Parameters:
   ///   - lhs: One value to compare.
   ///   - rhs: Another value to compare.
   ///
   /// - Returns: Whether or not `lhs` and `rhs` are equal.
   ///
-  /// Two instances of this type can be compared; if either instance is equal to
-  /// ``failure``, it will compare equal to any instance except ``success``. To
-  /// check if two instances are exactly equal, use the ``===(_:_:)`` operator:
+  /// Two exit conditions can be compared; if either instance is equal to
+  /// ``ExitCondition/failure``, it will compare equal to any instance except
+  /// ``ExitCondition/success``. To check if two instances are _exactly_ equal,
+  /// use the ``===(_:_:)`` operator:
   ///
   /// ```swift
   /// let lhs: ExitCondition = .failure
@@ -119,18 +123,18 @@ extension ExitCondition {
 #endif
   }
 
-  /// Check whether or not two values of this type are _not_ equal.
+  /// Check whether or not two exit conditions are _not_ equal.
   ///
   /// - Parameters:
   ///   - lhs: One value to compare.
   ///   - rhs: Another value to compare.
   ///
   /// - Returns: Whether or not `lhs` and `rhs` are _not_ equal.
   ///
-  /// Two instances of this type can be compared; if either instance is equal to
-  /// ``failure``, it will compare equal to any instance except ``success``. To
-  /// check if two instances are not exactly equal, use the ``!==(_:_:)``
-  /// operator:
+  /// Two exit conditions can be compared; if either instance is equal to
+  /// ``ExitCondition/failure``, it will compare equal to any instance except
+  /// ``ExitCondition/success``. To check if two instances are not _exactly_
+  /// equal, use the ``!==(_:_:)`` operator:
   ///
   /// ```swift
   /// let lhs: ExitCondition = .failure
@@ -153,17 +157,18 @@ extension ExitCondition {
 #endif
   }
 
-  /// Check whether or not two values of this type are identical.
+  /// Check whether or not two exit conditions are identical.
   ///
   /// - Parameters:
   ///   - lhs: One value to compare.
   ///   - rhs: Another value to compare.
   ///
   /// - Returns: Whether or not `lhs` and `rhs` are identical.
   ///
-  /// Two instances of this type can be compared; if either instance is equal to
-  /// ``failure``, it will compare equal to any instance except ``success``. To
-  /// check if two instances are exactly equal, use the ``===(_:_:)`` operator:
+  /// Two exit conditions can be compared; if either instance is equal to
+  /// ``ExitCondition/failure``, it will compare equal to any instance except
+  /// ``ExitCondition/success``. To check if two instances are _exactly_ equal,
+  /// use the ``===(_:_:)`` operator:
   ///
   /// ```swift
   /// let lhs: ExitCondition = .failure
@@ -180,6 +185,8 @@ extension ExitCondition {
   /// For any values `a` and `b`, `a === b` implies that `a !== b` is `false`.
   public static func ===(lhs: Self, rhs: Self) -> Bool {
     return switch (lhs, rhs) {
+    case (.none, .none):
+      true
     case (.failure, .failure):
       true
     case let (.exitCode(lhs), .exitCode(rhs)):
@@ -191,18 +198,18 @@ extension ExitCondition {
     }
   }
 
-  /// Check whether or not two values of this type are _not_ identical.
+  /// Check whether or not two exit conditions are _not_ identical.
   ///
   /// - Parameters:
   ///   - lhs: One value to compare.
   ///   - rhs: Another value to compare.
   ///
   /// - Returns: Whether or not `lhs` and `rhs` are _not_ identical.
   ///
-  /// Two instances of this type can be compared; if either instance is equal to
-  /// ``failure``, it will compare equal to any instance except ``success``. To
-  /// check if two instances are not exactly equal, use the ``!==(_:_:)``
-  /// operator:
+  /// Two exit conditions can be compared; if either instance is equal to
+  /// ``ExitCondition/failure``, it will compare equal to any instance except
+  /// ``ExitCondition/success``. To check if two instances are not _exactly_
+  /// equal, use the ``!==(_:_:)`` operator:
   ///
   /// ```swift
   /// let lhs: ExitCondition = .failure

Sources/Testing/ExitTests/ExitTest.Result.swift

@@ -23,13 +23,12 @@ private import _TestingInternals
 ///
 /// Instances of this type describe an exit test defined by the test author and
 /// discovered or called at runtime.
-@_spi(Experimental)
+@_spi(Experimental) @_spi(ForToolsIntegrationOnly)
 #if SWT_NO_EXIT_TESTS
 @available(*, unavailable, message: "Exit tests are not available on this platform.")
 #endif
 public struct ExitTest: Sendable, ~Copyable {
   /// The expected exit condition of the exit test.
-  @_spi(ForToolsIntegrationOnly)
   public var expectedExitCondition: ExitCondition
 
   /// The body closure of the exit test.
@@ -39,7 +38,6 @@ public struct ExitTest: Sendable, ~Copyable {
   ///
   /// The source location is unique to each exit test and is consistent between
   /// processes, so it can be used to uniquely identify an exit test at runtime.
-  @_spi(ForToolsIntegrationOnly)
   public var sourceLocation: SourceLocation
 }
 
@@ -97,7 +95,6 @@ extension ExitTest {
   /// to terminate the process; if it does not, the testing library will
   /// terminate the process in a way that causes the corresponding expectation
   /// to fail.
-  @_spi(ForToolsIntegrationOnly)
   public consuming func callAsFunction() async -> Never {
     Self._disableCrashReporting()
 
@@ -162,7 +159,6 @@ extension ExitTest {
   ///
   /// - Returns: The specified exit test function, or `nil` if no such exit test
   ///   could be found.
-  @_spi(ForToolsIntegrationOnly)
   public static func find(at sourceLocation: SourceLocation) -> Self? {
     var result: Self?
 
@@ -208,12 +204,12 @@ func callExitTest(
   isRequired: Bool,
   isolation: isolated (any Actor)? = #isolation,
   sourceLocation: SourceLocation
-) async -> ExitTest.Result {
+) async -> Result<ExitTestArtifacts, any Error> {
   guard let configuration = Configuration.current ?? Configuration.all.first else {
     preconditionFailure("A test must be running on the current task to use #expect(exitsWith:).")
   }
 
-  var result: ExitTest.Result
+  var result: ExitTestArtifacts
   do {
     let exitTest = ExitTest(expectedExitCondition: expectedExitCondition, sourceLocation: sourceLocation)
     result = try await configuration.exitTestHandler(exitTest)
@@ -247,29 +243,22 @@ func callExitTest(
     // For lack of a better way to handle an exit test failing in this way,
     // we record the system issue above, then let the expectation fail below by
     // reporting an exit condition that's the inverse of the expected one.
-    result = ExitTest.Result(exitCondition: expectedExitCondition == .failure ? .success : .failure)
+    result = ExitTestArtifacts(exitCondition: expectedExitCondition == .failure ? .success : .failure)
   }
 
   // How did the exit test actually exit?
   let actualExitCondition = result.exitCondition
 
-  // Plumb the resulting exit condition through the general expectation
-  // machinery. If the expectation failed, capture the ExpectationFailedError
-  // instance so that calls to #require(exitsWith:) throw it correctly.
-  let checkResult = __checkValue(
+  // Plumb the exit test's result through the general expectation machinery.
+  return __checkValue(
     expectedExitCondition == actualExitCondition,
     expression: expression,
     expressionWithCapturedRuntimeValues: expression.capturingRuntimeValues(actualExitCondition),
     mismatchedExitConditionDescription: String(describingForTest: expectedExitCondition),
     comments: comments(),
     isRequired: isRequired,
     sourceLocation: sourceLocation
-  )
-  if case let .failure(error) = checkResult {
-    result.caughtError = error
-  }
-
-  return result
+  ).map { result }
 }
 
 // MARK: - SwiftPM/tools integration
@@ -297,8 +286,7 @@ extension ExitTest {
   /// are available or the child environment is otherwise terminated. The parent
   /// environment is then responsible for interpreting those results and
   /// recording any issues that occur.
-  @_spi(ForToolsIntegrationOnly)
-  public typealias Handler = @Sendable (_ exitTest: borrowing ExitTest) async throws -> ExitTest.Result
+  public typealias Handler = @Sendable (_ exitTest: borrowing ExitTest) async throws -> ExitTestArtifacts
 
   /// The back channel file handle set up by the parent process.
   ///
@@ -477,7 +465,7 @@ extension ExitTest {
         childEnvironment["SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION"] = String(decoding: json, as: UTF8.self)
       }
 
-      return try await withThrowingTaskGroup(of: ExitTest.Result?.self) { taskGroup in
+      return try await withThrowingTaskGroup(of: ExitTestArtifacts?.self) { taskGroup in
         // Create a "back channel" pipe to handle events from the child process.
         let backChannel = try FileHandle.Pipe()
 
@@ -513,7 +501,7 @@ extension ExitTest {
         // Await termination of the child process.
         taskGroup.addTask {
           let exitCondition = try await wait(for: processID)
-          return ExitTest.Result(exitCondition: exitCondition)
+          return ExitTestArtifacts(exitCondition: exitCondition)
         }
 
         // Read back all data written to the back channel by the child process

Sources/Testing/ExitTests/ExitTest.swift

@@ -0,0 +1,37 @@
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
+//
+
+/// 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.
+///
+/// - Warning: The name of this type is still unstable and subject to change.
+@_spi(Experimental)
+#if SWT_NO_EXIT_TESTS
+@available(*, unavailable, message: "Exit tests are not available on this platform.")
+#endif
+public struct ExitTestArtifacts: Sendable {
+  /// The exit condition the exit test exited with.
+  ///
+  /// 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/==(_:_:)``.
+  public var exitCondition: ExitCondition
+
+  @_spi(ForToolsIntegrationOnly)
+  public init(exitCondition: ExitCondition) {
+    self.exitCondition = exitCondition
+  }
+}