Add withKnownIssue comments to known Issues

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 {}