Add withKnownIssue comments to known Issues

### Motivation:

The Comment passed to withKnownIssue() is currently only used when a
known issue does not occur. When a known issue does occur, it is marked
isKnown but does not contain any record of the withKnownIssue() comment,
making it harder to understand why the issue was considered to be known,
especially if there are multiple nested withKnownIssue() calls.

### Modifications:

When an issue is marked "known" by a `withKnownIssue()` call, the
recorded issue will now have multiple comments in its `comments` array:

1. The comment passed to `withKnownIssue()`, if any
2. The Issue's own comment(s)

If the issue is recorded within multiple nested `withKnownIssue()`
calls, only the comment of the innermost matching call is added to the
Issue.

### 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: aroben

Sources/Testing/Issues/Issue+Recording.swift

@@ -9,13 +9,24 @@
 //
 
 extension Issue {
-  /// The known issue matcher, as set by `withKnownIssue()`, associated with the
+  /// The known issue context, as set by `withKnownIssue()`, associated with the
   /// current task.
   ///
   /// If there is no call to `withKnownIssue()` executing on the current task,
   /// the value of this property is `nil`.
   @TaskLocal
-  static var currentKnownIssueMatcher: KnownIssueMatcher?
+  static var currentKnownIssueContext: KnownIssueContext?
+
+  /// Mark that an issue is a known issue.
+  /// - Parameter comment: The comment passed to the invocation of
+  ///   ``withKnownIssue`` that caused this issue to be considered "known".
+  mutating func markAsKnown(comment: Comment?) {
+    precondition(!isKnown)
+    isKnown = true
+    if let comment {
+      comments.insert(comment, at: 0)
+    }
+  }
 
   /// Record this issue by wrapping it in an ``Event`` and passing it to the
   /// current event handler.
@@ -38,9 +49,9 @@ extension Issue {
 
     // If this issue matches via the known issue matcher, set a copy of it to be
     // known and record the copy instead.
-    if !isKnown, let issueMatcher = Self.currentKnownIssueMatcher, issueMatcher(self) {
+    if !isKnown, let context = Self.currentKnownIssueContext, let match = context.match(self) {
       var selfCopy = self
-      selfCopy.isKnown = true
+      selfCopy.markAsKnown(comment: match.comment)
       return selfCopy.record(configuration: configuration)
     }
 

Sources/Testing/Issues/KnownIssue.swift

@@ -8,24 +8,49 @@
 // See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 //
 
-/// Combine an instance of ``KnownIssueMatcher`` with any previously-set one.
-///
-/// - Parameters:
-///   - issueMatcher: A function to invoke when an issue occurs that is used to
-///     determine if the issue is known to occur.
-///   - matchCounter: The counter responsible for tracking the number of matches
-///     found with `issueMatcher`.
-///
-/// - Returns: A new instance of ``Configuration`` or `nil` if there was no
-///   current configuration set.
-private func _combineIssueMatcher(_ issueMatcher: @escaping KnownIssueMatcher, matchesCountedBy matchCounter: Locked<Int>) -> KnownIssueMatcher {
-  let oldIssueMatcher = Issue.currentKnownIssueMatcher
-  return { issue in
-    if issueMatcher(issue) || true == oldIssueMatcher?(issue) {
-      matchCounter.increment()
-      return true
+/// A type that can be used to determine if an issue is a known issue.
+///
+/// A stack of these is stored in `Issue.currentKnownIssueContext`. The stack
+/// is mutated by calls to `withKnownIssue()`.
+struct KnownIssueContext: Sendable {
+  /// Determine if an issue is known to this context or any of its ancestor
+  /// contexts.
+  ///
+  /// Returns `nil` if the issue is not known.
+  var match: @Sendable (Issue) -> Match?
+  /// The number of issues this context and its ancestors have matched.
+  let matchCounter: Locked<Int>
+
+  struct Match {
+    /// The comment that was passed to the `withKnownIssue()` call that matched the issue.
+    var comment: Comment?
+  }
+
+  /// Create a new ``KnownIssueContext`` by combining a new issue matcher with
+  /// any previously-set context.
+  ///
+  /// - Parameters:
+  ///   - parent: The context that should be checked next if `issueMatcher`
+  ///     fails to match an issue.
+  ///   - issueMatcher: A function to invoke when an issue occurs that is used
+  ///     to determine if the issue is known to occur.
+  ///   - comment: Any comment to be associated with issues matched by
+  ///     `issueMatcher`.
+  /// - Returns: A new instance of ``KnownIssueContext``.
+  init(parent: KnownIssueContext?, issueMatcher: @escaping KnownIssueMatcher, comment: Comment?) {
+    let matchCounter = Locked(rawValue: 0)
+    self.matchCounter = matchCounter
+    match = { issue in
+      let match = if issueMatcher(issue) {
+        Match(comment: comment)
+      } else {
+        parent?.match(issue)
+      }
+      if match != nil {
+        matchCounter.increment()
+      }
+      return match
     }
-    return false
   }
 }
 
@@ -40,12 +65,12 @@ private func _combineIssueMatcher(_ issueMatcher: @escaping KnownIssueMatcher, m
 ///     function.
 ///   - sourceLocation: The source location to which the issue should be
 ///     attributed.
-private func _matchError(_ error: any Error, using issueMatcher: KnownIssueMatcher, comment: Comment?, sourceLocation: SourceLocation) throws {
+private func _matchError(_ error: any Error, using issueContext: KnownIssueContext, comment: Comment?, sourceLocation: SourceLocation) throws {
   let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error), sourceLocation: sourceLocation)
   var issue = Issue(kind: .errorCaught(error), comments: Array(comment), sourceContext: sourceContext)
-  if issueMatcher(issue) {
+  if let match = issueContext.match(issue) {
     // It's a known issue, so mark it as such before recording it.
-    issue.isKnown = true
+    issue.markAsKnown(comment: match.comment)
     issue.record()
   } else {
     // Rethrow the error, allowing the caller to catch it or for it to propagate
@@ -184,18 +209,17 @@ public func withKnownIssue(
   guard precondition() else {
     return try body()
   }
-  let matchCounter = Locked(rawValue: 0)
-  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
+  let issueContext = KnownIssueContext(parent: Issue.currentKnownIssueContext, issueMatcher: issueMatcher, comment: comment)
   defer {
     if !isIntermittent {
-      _handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
+      _handleMiscount(by: issueContext.matchCounter, comment: comment, sourceLocation: sourceLocation)
     }
   }
-  try Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
+  try Issue.$currentKnownIssueContext.withValue(issueContext) {
     do {
       try body()
     } catch {
-      try _matchError(error, using: issueMatcher, comment: comment, sourceLocation: sourceLocation)
+      try _matchError(error, using: issueContext, comment: comment, sourceLocation: sourceLocation)
     }
   }
 }
@@ -304,18 +328,17 @@ public func withKnownIssue(
   guard await precondition() else {
     return try await body()
   }
-  let matchCounter = Locked(rawValue: 0)
-  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
+  let issueContext = KnownIssueContext(parent: Issue.currentKnownIssueContext, issueMatcher: issueMatcher, comment: comment)
   defer {
     if !isIntermittent {
-      _handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
+      _handleMiscount(by: issueContext.matchCounter, comment: comment, sourceLocation: sourceLocation)
     }
   }
-  try await Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
+  try await Issue.$currentKnownIssueContext.withValue(issueContext) {
     do {
       try await body()
     } catch {
-      try _matchError(error, using: issueMatcher, comment: comment, sourceLocation: sourceLocation)
+      try _matchError(error, using: issueContext, comment: comment, sourceLocation: sourceLocation)
     }
   }
 }

Tests/TestingTests/KnownIssueTests.swift

@@ -37,7 +37,7 @@ final class KnownIssueTests: XCTestCase {
     await fulfillment(of: [issueRecorded], timeout: 0.0)
   }
 
-  func testKnownIssueWithComment() async {
+  func testKnownIssueNotRecordedWithComment() async {
     let issueRecorded = expectation(description: "Issue recorded")
 
     var configuration = Configuration()
@@ -62,6 +62,216 @@ final class KnownIssueTests: XCTestCase {
     await fulfillment(of: [issueRecorded], timeout: 0.0)
   }
 
+  func testKnownIssueRecordedWithComment() async {
+    let issueRecorded = expectation(description: "Issue recorded")
+
+    var configuration = Configuration()
+    configuration.eventHandler = { event, _ in
+      guard case let .issueRecorded(issue) = event.kind else {
+        return
+      }
+      issueRecorded.fulfill()
+
+      guard case .unconditional = issue.kind else {
+        return
+      }
+
+      XCTAssertEqual(issue.comments, ["With Known Issue Comment", "Issue Comment"])
+      XCTAssertTrue(issue.isKnown)
+    }
+
+    await Test {
+      withKnownIssue("With Known Issue Comment") {
+        Issue.record("Issue Comment")
+      }
+    }.run(configuration: configuration)
+
+    await fulfillment(of: [issueRecorded], timeout: 0.0)
+  }
+
+  func testThrownKnownIssueRecordedWithComment() async {
+    let issueRecorded = expectation(description: "Issue recorded")
+
+    var configuration = Configuration()
+    configuration.eventHandler = { event, _ in
+      guard case let .issueRecorded(issue) = event.kind else {
+        return
+      }
+      issueRecorded.fulfill()
+
+      guard case .unconditional = issue.kind else {
+        return
+      }
+
+      XCTAssertEqual(issue.comments, ["With Known Issue Comment"])
+      XCTAssertTrue(issue.isKnown)
+    }
+
+    struct E: Error {}
+
+    await Test {
+      withKnownIssue("With Known Issue Comment") {
+        throw E()
+      }
+    }.run(configuration: configuration)
+
+    await fulfillment(of: [issueRecorded], timeout: 0.0)
+  }
+
+  func testKnownIssueRecordedWithNoComment() async {
+    let issueRecorded = expectation(description: "Issue recorded")
+
+    var configuration = Configuration()
+    configuration.eventHandler = { event, _ in
+      guard case let .issueRecorded(issue) = event.kind else {
+        return
+      }
+      issueRecorded.fulfill()
+
+      guard case .unconditional = issue.kind else {
+        return
+      }
+
+      XCTAssertEqual(issue.comments, ["Issue Comment"])
+      XCTAssertTrue(issue.isKnown)
+    }
+
+    await Test {
+      withKnownIssue {
+        Issue.record("Issue Comment")
+      }
+    }.run(configuration: configuration)
+
+    await fulfillment(of: [issueRecorded], timeout: 0.0)
+  }
+
+  func testKnownIssueRecordedWithInnermostMatchingComment() async {
+    let issueRecorded = expectation(description: "Issue recorded")
+
+    var configuration = Configuration()
+    configuration.eventHandler = { event, _ in
+      guard case let .issueRecorded(issue) = event.kind else {
+        return
+      }
+      issueRecorded.fulfill()
+
+      guard case .unconditional = issue.kind else {
+        return
+      }
+
+      XCTAssertEqual(issue.comments, ["Inner Contains B", "Issue B"])
+      XCTAssertTrue(issue.isKnown)
+    }
+
+    await Test {
+      withKnownIssue("Contains A", isIntermittent: true) {
+        withKnownIssue("Outer Contains B", isIntermittent: true) {
+          withKnownIssue("Inner Contains B") {
+            withKnownIssue("Contains C", isIntermittent: true) {
+              Issue.record("Issue B")
+            } matching: { issue in
+              issue.comments.contains { $0.rawValue.contains("C") }
+            }
+          } matching: { issue in
+            issue.comments.contains { $0.rawValue.contains("B") }
+          }
+        } matching: { issue in
+          issue.comments.contains { $0.rawValue.contains("B") }
+        }
+      } matching: { issue in
+        issue.comments.contains { $0.rawValue.contains("A") }
+      }
+    }.run(configuration: configuration)
+
+    await fulfillment(of: [issueRecorded], timeout: 0.0)
+  }
+
+  func testThrownKnownIssueRecordedWithInnermostMatchingComment() async {
+    let issueRecorded = expectation(description: "Issue recorded")
+
+    var configuration = Configuration()
+    configuration.eventHandler = { event, _ in
+      guard case let .issueRecorded(issue) = event.kind else {
+        return
+      }
+      issueRecorded.fulfill()
+
+      guard case .unconditional = issue.kind else {
+        return
+      }
+
+      XCTAssertEqual(issue.comments, ["Inner Is B", "B"])
+      XCTAssertTrue(issue.isKnown)
+    }
+
+    struct A: Error {}
+    struct B: Error {}
+    struct C: Error {}
+
+    await Test {
+      try withKnownIssue("Is A", isIntermittent: true) {
+        try withKnownIssue("Outer Is B", isIntermittent: true) {
+          try withKnownIssue("Inner Is B") {
+            try withKnownIssue("Is C", isIntermittent: true) {
+              throw B()
+            } matching: { issue in
+              issue.error is C
+            }
+          } matching: { issue in
+            issue.error is B
+          }
+        } matching: { issue in
+          issue.error is B
+        }
+      } matching: { issue in
+        issue.error is A
+      }
+    }.run(configuration: configuration)
+
+    await fulfillment(of: [issueRecorded], timeout: 0.0)
+  }
+
+  func testKnownIssueRecordedWithNoCommentOnInnermostMatch() async {
+    let issueRecorded = expectation(description: "Issue recorded")
+
+    var configuration = Configuration()
+    configuration.eventHandler = { event, _ in
+      guard case let .issueRecorded(issue) = event.kind else {
+        return
+      }
+      issueRecorded.fulfill()
+
+      guard case .unconditional = issue.kind else {
+        return
+      }
+
+      XCTAssertEqual(issue.comments, ["Issue B"])
+      XCTAssertTrue(issue.isKnown)
+    }
+
+    await Test {
+      withKnownIssue("Contains A", isIntermittent: true) {
+        withKnownIssue("Outer Contains B", isIntermittent: true) {
+          withKnownIssue { // No comment here on the withKnownIssue that will actually match.
+            withKnownIssue("Contains C", isIntermittent: true) {
+              Issue.record("Issue B")
+            } matching: { issue in
+              issue.comments.contains { $0.rawValue.contains("C") }
+            }
+          } matching: { issue in
+            issue.comments.contains { $0.rawValue.contains("B") }
+          }
+        } matching: { issue in
+          issue.comments.contains { $0.rawValue.contains("B") }
+        }
+      } matching: { issue in
+        issue.comments.contains { $0.rawValue.contains("A") }
+      }
+    }.run(configuration: configuration)
+
+    await fulfillment(of: [issueRecorded], timeout: 0.0)
+  }
+
   func testIssueIsKnownPropertyIsSetCorrectlyWithCustomIssueMatcher() async {
     struct MyError: Error {}