diff --git a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift index 0e856facf..7947dbab1 100644 --- a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift +++ b/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,9 @@ extension Event.HumanReadableOutputRecorder { additionalMessages.append(Message(symbol: .difference, stringValue: differenceDescription)) } additionalMessages += _formattedComments(issue.comments) + if let knownIssueComment = issue.knownIssueContext?.comment { + additionalMessages.append(_formattedComment(knownIssueComment)) + } if verbosity > 0, case let .expectationFailed(expectation) = issue.kind { let expression = expectation.evaluatedExpression diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index a28e2eede..2f96fc123 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/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() } } diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index aaf721c6a..098fb2e8d 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/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) } diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 5d7449b7b..d5befd1ed 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/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. /// diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index f59e9d422..f59185388 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -8,25 +8,62 @@ // 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) -> 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) -> Issue.KnownIssueContext? + + /// The matcher function for this known issue scope. + var matcher: Matcher + + /// The number of issues this scope and its ancestors have matched. + let matchCounter: Locked + + /// 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. Defaults to ``KnownIssueScope.current``. + /// - 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`. + init(parent: KnownIssueScope? = .current, 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 any. + /// + /// 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 @@ -34,18 +71,17 @@ private func _combineIssueMatcher(_ issueMatcher: @escaping KnownIssueMatcher, m /// /// - 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, in 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) { + var issue = Issue(kind: .errorCaught(error), comments: [], sourceContext: sourceContext) + 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 +220,17 @@ public func withKnownIssue( guard precondition() else { return try body() } - let matchCounter = Locked(rawValue: 0) - let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter) + let scope = KnownIssueScope(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, in: scope, comment: comment, sourceLocation: sourceLocation) } } } @@ -304,18 +339,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(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, in: scope, comment: comment, sourceLocation: sourceLocation) } } } diff --git a/Tests/TestingTests/EventRecorderTests.swift b/Tests/TestingTests/EventRecorderTests.swift index 8ac7f6728..0db62f6a8 100644 --- a/Tests/TestingTests/EventRecorderTests.swift +++ b/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() + } + } +} diff --git a/Tests/TestingTests/KnownIssueTests.swift b/Tests/TestingTests/KnownIssueTests.swift index 448b8e35d..cfcd3f0df 100644 --- a/Tests/TestingTests/KnownIssueTests.swift +++ b/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() @@ -51,8 +51,8 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments.first, "With Known Issue Comment") XCTAssertFalse(issue.isKnown) + XCTAssertEqual(issue.comments, ["With Known Issue Comment"]) } await Test { @@ -62,6 +62,234 @@ final class KnownIssueTests: XCTestCase { await fulfillment(of: [issueRecorded], timeout: 0.0) } + func testKnownIssueRecordedWithComment() async { + let issueMatched = expectation(description: "Issue matched") + 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 + } + + XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue Comment"]) + XCTAssertEqual(issue.knownIssueContext?.comment, "With Known Issue Comment") + } + + await Test { + withKnownIssue("With Known Issue Comment") { + Issue.record("Issue Comment") + } matching: { issue in + // The issue isn't yet considered known since we haven't matched it yet. + XCTAssertFalse(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue Comment"]) + XCTAssertNil(issue.knownIssueContext) + issueMatched.fulfill() + return true + } + }.run(configuration: configuration) + + await fulfillment(of: [issueMatched, issueRecorded], timeout: 0.0) + } + + func testThrownKnownIssueRecordedWithComment() async { + let issueMatched = expectation(description: "Issue matched") + 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 + } + + XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["With Known Issue Comment"]) + } + + struct E: Error {} + + await Test { + try withKnownIssue("With Known Issue Comment") { + throw E() + } matching: { issue in + // The issue isn't yet considered known since we haven't matched it yet. + XCTAssertFalse(issue.isKnown) + XCTAssertEqual(issue.comments, []) + XCTAssertNil(issue.knownIssueContext) + issueMatched.fulfill() + return true + } + }.run(configuration: configuration) + + await fulfillment(of: [issueMatched, 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 + } + + XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue Comment"]) + } + + 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 + } + + XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue B"]) + XCTAssertEqual(issue.knownIssueContext?.comment, "Inner Contains B") + } + + 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 + } + + XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Inner Is B", "B"]) + } + + 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 + } + + XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue B"]) + } + + 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 {}