Propagate SkipInfo to child tasks when a test is cancelled. (#1298)

This PR ensures that, when a test is cancelled, its test cases report
the same `SkipInfo` structure as they are recursively cancelled. For
instance:

```swift
@test(arguments: [a ... z]) func foo(arg: T) throws {
  try Test.cancel("Message")
}
```

Prior to this change, the `.testCaseCancelled` events would be posted
without the user-supplied comment `"Message"` and each one would need to
construct a source context by capturing a backtrace from deep within
Swift Testing and the stdlib.

With this change in place, said events post with the same comment and
source context as is associated with the initial `.testCancelled` event.

### 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/ExitTests/ExitTest.swift

@@ -1048,6 +1048,7 @@ extension ExitTest {
       backtrace: nil, // A backtrace from the child process will have the wrong address space.
       sourceLocation: event._sourceLocation
     )
+    lazy var skipInfo = SkipInfo(comment: comments.first, sourceContext: sourceContext)
     if let issue = event.issue {
       // Translate the issue back into a "real" issue and record it
       // in the parent process. This translation is, of course, lossy
@@ -1075,9 +1076,9 @@ extension ExitTest {
     } else if let attachment = event.attachment {
       Attachment.record(attachment, sourceLocation: event._sourceLocation!)
     } else if case .testCancelled = event.kind {
-      _ = try? Test.cancel(comments: comments, sourceContext: sourceContext)
+      _ = try? Test.cancel(with: skipInfo)
     } else if case .testCaseCancelled = event.kind {
-      _ = try? Test.Case.cancel(comments: comments, sourceContext: sourceContext)
+      _ = try? Test.Case.cancel(with: skipInfo)
     }
   }
 

Sources/Testing/Running/SkipInfo.swift

@@ -69,7 +69,7 @@ extension SkipInfo {
       self = skipInfo
     } else if error is CancellationError, Task.isCancelled {
       // Synthesize skip info for this cancellation error.
-      let backtrace = Backtrace(forFirstThrowOf: error) ?? .current()
+      let backtrace = Backtrace(forFirstThrowOf: error)
       let sourceContext = SourceContext(backtrace: backtrace, sourceLocation: nil)
       self.init(comment: nil, sourceContext: sourceContext)
     } else {

Sources/Testing/Test+Cancellation.swift

@@ -16,17 +16,15 @@ protocol TestCancellable: Sendable {
   /// Cancel the current instance of this type.
   ///
   /// - Parameters:
-  ///   - comments: Comments describing why you are cancelling the test/case.
-  ///   - sourceContext: The source context to which the testing library will
-  ///     attribute the cancellation.
+  ///   - skipInfo: Information about the cancellation event.
   ///
   /// - Throws: An error indicating that the current instance of this type has
   ///   been cancelled.
   ///
   /// Note that the public ``Test/cancel(_:sourceLocation:)`` function has a
   /// different signature and accepts a source location rather than a source
   /// context value.
-  static func cancel(comments: [Comment], sourceContext: @autoclosure () -> SourceContext) throws -> Never
+  static func cancel(with skipInfo: SkipInfo) throws -> Never
 
   /// Make an instance of ``Event/Kind`` appropriate for an instance of this
   /// type.
@@ -47,8 +45,17 @@ private struct _TaskReference: Sendable {
   private nonisolated(unsafe) var _unsafeCurrentTask = Locked<UnsafeCurrentTask?>()
 
   init() {
-    let unsafeCurrentTask = withUnsafeCurrentTask { $0 }
-    _unsafeCurrentTask = Locked(rawValue: unsafeCurrentTask)
+    // WARNING! Normally, allowing an instance of `UnsafeCurrentTask` to escape
+    // its scope is dangerous because it could be used unsafely after the task
+    // ends. However, because we take care not to allow the task object to
+    // escape the task (by only storing it in a task-local value), we can ensure
+    // these unsafe scenarios won't occur.
+    //
+    // TODO: when our deployment targets allow, we should switch to calling the
+    // `async` overload of `withUnsafeCurrentTask()` from the body of
+    // `withCancellationHandling(_:)`. That will allow us to use the task object
+    // in a safely scoped fashion.
+    _unsafeCurrentTask = withUnsafeCurrentTask { Locked(rawValue: $0) }
   }
 
   /// Take this instance's reference to its associated task.
@@ -69,8 +76,14 @@ private struct _TaskReference: Sendable {
 
 /// A dictionary of tracked tasks, keyed by types that conform to
 /// ``TestCancellable``.
-@TaskLocal
-private var _currentTaskReferences = [ObjectIdentifier: _TaskReference]()
+@TaskLocal private var _currentTaskReferences = [ObjectIdentifier: _TaskReference]()
+
+/// The instance of ``SkipInfo`` to propagate to children of the current task.
+///
+/// We set this value while calling `UnsafeCurrentTask.cancel()` so that its
+/// value is available in tracked child tasks when their cancellation handlers
+/// are called (in ``TestCancellable/withCancellationHandling(_:)`` below).
+@TaskLocal private var _currentSkipInfo: SkipInfo?
 
 extension TestCancellable {
   /// Call a function while the ``unsafeCurrentTask`` property of this instance
@@ -95,10 +108,9 @@ extension TestCancellable {
       } onCancel: {
         // The current task was cancelled, so cancel the test case or test
         // associated with it.
-        _ = try? Self.cancel(
-          comments: [],
-          sourceContext: SourceContext(backtrace: .current(), sourceLocation: nil)
-        )
+
+        let skipInfo = _currentSkipInfo ?? SkipInfo(sourceContext: SourceContext(backtrace: .current(), sourceLocation: nil))
+        _ = try? Self.cancel(with: skipInfo)
       }
     }
   }
@@ -112,24 +124,21 @@ extension TestCancellable {
 ///   - cancellableValue: The test or test case to cancel, or `nil` if neither
 ///     is set and we need fallback handling.
 ///   - testAndTestCase: The test and test case to use when posting an event.
-///   - comments: Comments describing why you are cancelling the test/case.
-///   - sourceContext: The source context to which the testing library will
-///     attribute the cancellation.
+///   - skipInfo: Information about the cancellation event.
 ///
 /// - Throws: An instance of ``SkipInfo`` describing the cancellation.
-private func _cancel<T>(_ cancellableValue: T?, for testAndTestCase: (Test?, Test.Case?), comments: [Comment], sourceContext: @autoclosure () -> SourceContext) throws -> Never where T: TestCancellable {
-  var skipInfo = SkipInfo(comment: comments.first, sourceContext: .init(backtrace: nil, sourceLocation: nil))
-
+private func _cancel<T>(_ cancellableValue: T?, for testAndTestCase: (Test?, Test.Case?), skipInfo: SkipInfo) throws -> Never where T: TestCancellable {
   if cancellableValue != nil {
-    // If the current test case is still running, cancel its task and clear its
-    // task property (which signals that it has been cancelled.)
+    // If the current test case is still running, take its task property (which
+    // signals to subsequent callers that it has been cancelled.)
     let task = _currentTaskReferences[ObjectIdentifier(T.self)]?.takeUnsafeCurrentTask()
-    task?.cancel()
 
     // If we just cancelled the current test case's task, post a corresponding
     // event with the relevant skip info.
-    if task != nil {
-      skipInfo.sourceContext = sourceContext()
+    if let task {
+      $_currentSkipInfo.withValue(skipInfo) {
+        task.cancel()
+      }
       Event.post(T.makeCancelledEventKind(with: skipInfo), for: testAndTestCase)
     }
   } else {
@@ -147,13 +156,18 @@ private func _cancel<T>(_ cancellableValue: T?, for testAndTestCase: (Test?, Tes
       // This code is running in an exit test. We don't have a "current test" or
       // "current test case" in the child process, so we'll let the parent
       // process sort that out.
-      skipInfo.sourceContext = sourceContext()
       Event.post(T.makeCancelledEventKind(with: skipInfo), for: (nil, nil))
     } else {
       // Record an API misuse issue for trying to cancel the current test/case
       // outside of any useful context.
-      let comments = ["Attempted to cancel the current test or test case, but one is not associated with the current task."] + comments
-      let issue = Issue(kind: .apiMisused, comments: comments, sourceContext: sourceContext())
+      let issue = Issue(
+        kind: .apiMisused,
+        comments: [
+          "Attempted to cancel the current test or test case, but one is not associated with the current task.",
+          skipInfo.comment,
+        ].compactMap(\.self),
+        sourceContext: skipInfo.sourceContext
+      )
       issue.record()
     }
   }
@@ -208,15 +222,13 @@ extension Test: TestCancellable {
   /// test alone, call ``Test/Case/cancel(_:sourceLocation:)`` instead.
   @_spi(Experimental)
   public static func cancel(_ comment: Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation) throws -> Never {
-    try Self.cancel(
-      comments: Array(comment),
-      sourceContext: SourceContext(backtrace: .current(), sourceLocation: sourceLocation)
-    )
+    let skipInfo = SkipInfo(comment: comment, sourceContext: SourceContext(backtrace: nil, sourceLocation: sourceLocation))
+    try Self.cancel(with: skipInfo)
   }
 
-  static func cancel(comments: [Comment], sourceContext: @autoclosure () -> SourceContext) throws -> Never {
+  static func cancel(with skipInfo: SkipInfo) throws -> Never {
     let test = Test.current
-    try _cancel(test, for: (test, nil), comments: comments, sourceContext: sourceContext())
+    try _cancel(test, for: (test, nil), skipInfo: skipInfo)
   }
 
   static func makeCancelledEventKind(with skipInfo: SkipInfo) -> Event.Kind {
@@ -271,23 +283,20 @@ extension Test.Case: TestCancellable {
   /// ``Test/cancel(_:sourceLocation:)`` instead.
   @_spi(Experimental)
   public static func cancel(_ comment: Comment? = nil, sourceLocation: SourceLocation = #_sourceLocation) throws -> Never {
-    try Self.cancel(
-      comments: Array(comment),
-      sourceContext: SourceContext(backtrace: .current(), sourceLocation: sourceLocation)
-    )
+    let skipInfo = SkipInfo(comment: comment, sourceContext: SourceContext(backtrace: nil, sourceLocation: sourceLocation))
+    try Self.cancel(with: skipInfo)
   }
 
-  static func cancel(comments: [Comment], sourceContext: @autoclosure () -> SourceContext) throws -> Never {
+  static func cancel(with skipInfo: SkipInfo) throws -> Never {
     let test = Test.current
     let testCase = Test.Case.current
-    let sourceContext = sourceContext() // evaluated twice, avoid laziness
 
     do {
       // Cancel the current test case (if it's nil, that's the API misuse path.)
-      try _cancel(testCase, for: (test, testCase), comments: comments, sourceContext: sourceContext)
+      try _cancel(testCase, for: (test, testCase), skipInfo: skipInfo)
     } catch _ where test?.isParameterized == false {
       // The current test is not parameterized, so cancel the whole test too.
-      try _cancel(test, for: (test, nil), comments: comments, sourceContext: sourceContext)
+      try _cancel(test, for: (test, nil), skipInfo: skipInfo)
     }
   }
 

Sources/Testing/Traits/ConditionTrait.swift

@@ -85,15 +85,13 @@ public struct ConditionTrait: TestTrait, SuiteTrait {
 
   public func prepare(for test: Test) async throws {
     let isEnabled = try await evaluate()
-
     if !isEnabled {
       // We don't need to consider including a backtrace here because it will
       // primarily contain frames in the testing library, not user code. If an
       // error was thrown by a condition evaluated above, the caller _should_
       // attempt to get the backtrace of the caught error when creating an issue
       // for it, however.
-      let sourceContext = SourceContext(backtrace: nil, sourceLocation: sourceLocation)
-      throw SkipInfo(comment: comments.first, sourceContext: sourceContext)
+      try Test.cancel(comments.first, sourceLocation: sourceLocation)
     }
   }
 

Tests/TestingTests/TestCancellationTests.swift

@@ -11,13 +11,20 @@
 @testable @_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing
 
 @Suite(.serialized) struct `Test cancellation tests` {
-  func testCancellation(testCancelled: Int = 0, testSkipped: Int = 0, testCaseCancelled: Int = 0, issueRecorded: Int = 0, _ body: @Sendable (Configuration) async -> Void) async {
+  func testCancellation(
+    testCancelled: Int = 0,
+    testSkipped: Int = 0,
+    testCaseCancelled: Int = 0,
+    issueRecorded: Int = 0,
+    _ body: @Sendable (Configuration) async -> Void,
+    eventHandler: @escaping @Sendable (borrowing Event, borrowing Event.Context) -> Void = { _, _ in }
+  ) async {
     await confirmation("Test cancelled", expectedCount: testCancelled) { testCancelled in
       await confirmation("Test skipped", expectedCount: testSkipped) { testSkipped in
         await confirmation("Test case cancelled", expectedCount: testCaseCancelled) { testCaseCancelled in
           await confirmation("Issue recorded", expectedCount: issueRecorded) { [issueRecordedCount = issueRecorded] issueRecorded in
             var configuration = Configuration()
-            configuration.eventHandler = { event, _ in
+            configuration.eventHandler = { event, eventContext in
               switch event.kind {
               case .testCancelled:
                 testCancelled()
@@ -33,6 +40,7 @@
               default:
                 break
               }
+              eventHandler(event, eventContext)
             }
 #if !SWT_NO_EXIT_TESTS
             configuration.exitTestHandler = ExitTest.handlerForEntryPoint()
@@ -84,6 +92,20 @@
     }
   }
 
+  @Test func `Cancelling a test propagates its SkipInfo to its test cases`() async {
+    let sourceLocation = #_sourceLocation
+    await testCancellation(testCancelled: 1, testCaseCancelled: 1) { configuration in
+      await Test {
+        try Test.cancel("Cancelled test", sourceLocation: sourceLocation)
+      }.run(configuration: configuration)
+    } eventHandler: { event, _ in
+      if case let .testCaseCancelled(skipInfo) = event.kind {
+        #expect(skipInfo.comment?.rawValue == "Cancelled test")
+        #expect(skipInfo.sourceContext.sourceLocation == sourceLocation)
+      }
+    }
+  }
+
   @Test func `Cancelling a test by cancelling its task (throwing)`() async {
     await testCancellation(testCancelled: 1, testCaseCancelled: 1) { configuration in
       await Test {