Add withKnownIssue comments to known Issues

Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift

@@ -93,10 +93,20 @@ extension Event.HumanReadableOutputRecorder {
   /// - Parameters:
   ///   - comments: The comments that should be formatted.
   ///
-  /// - Returns: A formatted string representing `comments`, or `nil` if there
-  ///   are none.
+  /// - Returns: An array of formatted messages representing `comments`, or an
+  /// empty array if there are none.
   private func _formattedComments(_ comments: [Comment]) -> [Message] {
-    comments.map { Message(symbol: .details, stringValue: $0.rawValue) }
+    comments.map(_formattedComment)
+  }
+
+  /// Get a string representing a single comment, formatted for output.
+  ///
+  /// - Parameters:
+  ///   - comment: The comment that should be formatted.
+  ///
+  /// - Returns: A formatted message representing `comment`.
+  private func _formattedComment(_ comment: Comment) -> Message {
+    Message(symbol: .details, stringValue: comment.rawValue)
   }
 
   /// Get a string representing the comments attached to a test, formatted for
@@ -443,6 +453,15 @@ extension Event.HumanReadableOutputRecorder {
         additionalMessages.append(Message(symbol: .difference, stringValue: differenceDescription))
       }
       additionalMessages += _formattedComments(issue.comments)
+      if let knownIssueComment = issue.knownIssueContext?.comment {
+        if case .errorCaught = issue.kind {
+          // `_matchError()` put the known issue comment in `issue.comments`
+          // when it first created the issue, so it's already included in
+          // `additionalMessages`.
+        } else {
+          additionalMessages.append(_formattedComment(knownIssueComment))
+        }
+      }
 
       if verbosity > 0, case let .expectationFailed(expectation) = issue.kind {
         let expression = expectation.evaluatedExpression

Sources/Testing/ExitTests/ExitTest.swift

@@ -746,7 +746,11 @@ extension ExitTest {
         sourceLocation: issue.sourceLocation
       )
       var issueCopy = Issue(kind: issueKind, comments: comments, sourceContext: sourceContext)
-      issueCopy.isKnown = issue.isKnown
+      if issue.isKnown {
+        // The known issue comment, if there was one, is already included in
+        // the `comments` array above.
+        issueCopy.knownIssueContext = Issue.KnownIssueContext()
+      }
       issueCopy.record()
     }
   }

Sources/Testing/Issues/Issue+Recording.swift

@@ -9,14 +9,6 @@
 //
 
 extension Issue {
-  /// The known issue matcher, 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?
-
   /// Record this issue by wrapping it in an ``Event`` and passing it to the
   /// current event handler.
   ///
@@ -38,9 +30,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 = KnownIssueScope.current?.matcher(self) {
       var selfCopy = self
-      selfCopy.isKnown = true
+      selfCopy.knownIssueContext = context
       return selfCopy.record(configuration: configuration)
     }
 

Sources/Testing/Issues/Issue.swift

@@ -115,9 +115,29 @@ public struct Issue: Sendable {
   @_spi(ForToolsIntegrationOnly)
   public var sourceContext: SourceContext
 
+  /// A type representing a
+  /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call
+  /// that matched an issue.
+  @_spi(ForToolsIntegrationOnly)
+  public struct KnownIssueContext: Sendable {
+    /// The comment that was passed to
+    /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``.
+    public var comment: Comment?
+  }
+
+  /// A ``KnownIssueContext-swift.struct`` representing the
+  /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call
+  /// that matched this issue, if any.
+  @_spi(ForToolsIntegrationOnly)
+  public var knownIssueContext: KnownIssueContext? = nil
+
   /// Whether or not this issue is known to occur.
   @_spi(ForToolsIntegrationOnly)
-  public var isKnown: Bool = false
+  public var isKnown: Bool {
+    get { knownIssueContext != nil }
+    @available(*, deprecated, message: "Setting this property has no effect.")
+    set {}
+  }
 
   /// Initialize an issue instance with the specified details.
   ///

Sources/Testing/Issues/KnownIssue.swift

@@ -8,44 +8,83 @@
 // 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 represents an active
+/// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``
+/// call and any parent calls.
+///
+/// A stack of these is stored in `KnownIssueScope.current`.
+struct KnownIssueScope: Sendable {
+  /// A function which determines if an issue matches a known issue scope or
+  /// any of its ancestor scopes.
+  ///
+  /// - Parameters:
+  ///   - issue: The issue being matched.
+  /// - Returns: A known issue context containing information about the known
+  /// issue, if the issue is considered "known" by this known issue scope or any
+  /// ancestor scope, or `nil` otherwise.
+  typealias Matcher = @Sendable (Issue) -> Issue.KnownIssueContext?
+
+  /// Determine if an issue is known to this scope or any of its ancestor
+  /// scopes.
+  ///
+  /// Returns `nil` if the issue is not known.
+  var matcher: @Sendable (Issue) -> Issue.KnownIssueContext?
+
+  /// The number of issues this scope and its ancestors have matched.
+  let matchCounter: Locked<Int>
+
+  /// Create a new ``KnownIssueScope`` by combining a new issue matcher with
+  /// any already-active scope.
+  ///
+  /// - 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.
+  ///   - context: The context to be associated with issues matched by
+  ///     `issueMatcher`.
+  /// - Returns: A new instance of ``KnownIssueScope``.
+  init(parent: KnownIssueScope?, issueMatcher: @escaping KnownIssueMatcher, context: Issue.KnownIssueContext) {
+    let matchCounter = Locked(rawValue: 0)
+    self.matchCounter = matchCounter
+    matcher = { issue in
+      let matchedContext = if issueMatcher(issue) {
+        context
+      } else {
+        parent?.matcher(issue)
+      }
+      if matchedContext != nil {
+        matchCounter.increment()
+      }
+      return matchedContext
     }
-    return false
   }
+
+  /// The active known issue scope for the current task.
+  ///
+  /// If there is no call to
+  /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``
+  /// executing on the current task, the value of this property is `nil`.
+  @TaskLocal
+  static var current: KnownIssueScope?
 }
 
 /// Check if an error matches using an issue-matching function, and throw it if
 /// it does not.
 ///
 /// - Parameters:
 ///   - error: The error to test.
-///   - issueMatcher: A function to which `error` is passed (after boxing it in
-///     an instance of ``Issue``) to determine if it is known to occur.
+///   - scope: The known issue scope that is processing the error.
 ///   - comment: An optional comment to apply to any issues generated by this
 ///     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 scope: KnownIssueScope, 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 context = scope.matcher(issue) {
     // It's a known issue, so mark it as such before recording it.
-    issue.isKnown = true
+    issue.knownIssueContext = context
     issue.record()
   } else {
     // Rethrow the error, allowing the caller to catch it or for it to propagate
@@ -184,18 +223,17 @@ public func withKnownIssue(
   guard precondition() else {
     return try body()
   }
-  let matchCounter = Locked(rawValue: 0)
-  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
+  let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment))
   defer {
     if !isIntermittent {
-      _handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
+      _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation)
     }
   }
-  try Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
+  try KnownIssueScope.$current.withValue(scope) {
     do {
       try body()
     } catch {
-      try _matchError(error, using: issueMatcher, comment: comment, sourceLocation: sourceLocation)
+      try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation)
     }
   }
 }
@@ -304,18 +342,17 @@ public func withKnownIssue(
   guard await precondition() else {
     return try await body()
   }
-  let matchCounter = Locked(rawValue: 0)
-  let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
+  let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment))
   defer {
     if !isIntermittent {
-      _handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
+      _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation)
     }
   }
-  try await Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
+  try await KnownIssueScope.$current.withValue(scope) {
     do {
       try await body()
     } catch {
-      try _matchError(error, using: issueMatcher, comment: comment, sourceLocation: sourceLocation)
+      try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation)
     }
   }
 }

Tests/TestingTests/EventRecorderTests.swift

@@ -497,6 +497,35 @@ struct EventRecorderTests {
       recorder.record(Event(.runEnded, testID: nil, testCaseID: nil), in: context)
     }
   }
+
+  @Test(
+    "HumanReadableOutputRecorder includes known issue comment in messages array",
+    arguments: [
+      ("recordWithoutKnownIssueComment()", ["#expect comment"]),
+      ("recordWithKnownIssueComment()", ["#expect comment", "withKnownIssue comment"]),
+      ("throwWithoutKnownIssueComment()", []),
+      ("throwWithKnownIssueComment()", ["withKnownIssue comment"]),
+    ]
+  )
+  func knownIssueComments(testName: String, expectedComments: [String]) async throws {
+    var configuration = Configuration()
+    let recorder = Event.HumanReadableOutputRecorder()
+    let messages = Locked<[Event.HumanReadableOutputRecorder.Message]>(rawValue: [])
+    configuration.eventHandler = { event, context in
+      guard case .issueRecorded = event.kind else { return }
+      messages.withLock {
+        $0.append(contentsOf: recorder.record(event, in: context))
+      }
+    }
+
+    await runTestFunction(named: testName, in: PredictablyFailingKnownIssueTests.self, configuration: configuration)
+
+    // The first message is something along the lines of "Test foo recorded a
+    // known issue" and includes a source location, so is inconvenient to
+    // include in our expectation here.
+    let actualComments = messages.rawValue.dropFirst().map(\.stringValue)
+    #expect(actualComments == expectedComments)
+  }
 }
 
 // MARK: - Fixtures
@@ -639,3 +668,35 @@ struct EventRecorderTests {
     #expect(arg > 0)
   }
 }
+
+@Suite(.hidden) struct PredictablyFailingKnownIssueTests {
+  @Test(.hidden)
+  func recordWithoutKnownIssueComment() {
+    withKnownIssue {
+      #expect(Bool(false), "#expect comment")
+    }
+  }
+
+  @Test(.hidden)
+  func recordWithKnownIssueComment() {
+    withKnownIssue("withKnownIssue comment") {
+      #expect(Bool(false), "#expect comment")
+    }
+  }
+
+  @Test(.hidden)
+  func throwWithoutKnownIssueComment() {
+    withKnownIssue {
+      struct TheError: Error {}
+      throw TheError()
+    }
+  }
+
+  @Test(.hidden)
+  func throwWithKnownIssueComment() {
+    withKnownIssue("withKnownIssue comment") {
+      struct TheError: Error {}
+      throw TheError()
+    }
+  }
+}