From c6cbd3d83047b84673ac461cfb31c32cbaeb1c07 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 10 Mar 2025 16:19:46 -0400 Subject: [PATCH 01/27] 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. --- Sources/Testing/Issues/Issue+Recording.swift | 19 +- Sources/Testing/Issues/KnownIssue.swift | 83 +++++--- Tests/TestingTests/KnownIssueTests.swift | 212 ++++++++++++++++++- 3 files changed, 279 insertions(+), 35 deletions(-) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index aaf721c6a..aa66f7a09 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/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) } diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index f59e9d422..d5d3f30b7 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/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) -> 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 + + 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) } } } diff --git a/Tests/TestingTests/KnownIssueTests.swift b/Tests/TestingTests/KnownIssueTests.swift index 448b8e35d..1be86764f 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() @@ -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 {} From acdb283d095a255c7dc931eee779dca651eaf85b Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 09:42:12 -0400 Subject: [PATCH 02/27] KnownIssueContext.current --- Sources/Testing/Issues/Issue+Recording.swift | 10 +--------- Sources/Testing/Issues/KnownIssue.swift | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index aa66f7a09..aff2c410e 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -9,14 +9,6 @@ // extension Issue { - /// 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 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". @@ -49,7 +41,7 @@ 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 context = Self.currentKnownIssueContext, let match = context.match(self) { + if !isKnown, let match = KnownIssueContext.current?.match(self) { var selfCopy = self selfCopy.markAsKnown(comment: match.comment) return selfCopy.record(configuration: configuration) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index d5d3f30b7..41394758c 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -52,6 +52,14 @@ struct KnownIssueContext: Sendable { return match } } + + /// 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 current: KnownIssueContext? } /// Check if an error matches using an issue-matching function, and throw it if @@ -209,13 +217,13 @@ public func withKnownIssue( guard precondition() else { return try body() } - let issueContext = KnownIssueContext(parent: Issue.currentKnownIssueContext, issueMatcher: issueMatcher, comment: comment) + let issueContext = KnownIssueContext(parent: .current, issueMatcher: issueMatcher, comment: comment) defer { if !isIntermittent { _handleMiscount(by: issueContext.matchCounter, comment: comment, sourceLocation: sourceLocation) } } - try Issue.$currentKnownIssueContext.withValue(issueContext) { + try KnownIssueContext.$current.withValue(issueContext) { do { try body() } catch { @@ -328,13 +336,13 @@ public func withKnownIssue( guard await precondition() else { return try await body() } - let issueContext = KnownIssueContext(parent: Issue.currentKnownIssueContext, issueMatcher: issueMatcher, comment: comment) + let issueContext = KnownIssueContext(parent: .current, issueMatcher: issueMatcher, comment: comment) defer { if !isIntermittent { _handleMiscount(by: issueContext.matchCounter, comment: comment, sourceLocation: sourceLocation) } } - try await Issue.$currentKnownIssueContext.withValue(issueContext) { + try await KnownIssueContext.$current.withValue(issueContext) { do { try await body() } catch { From 08b1c3f2ade088a21bcd4f3b5c78dada8e3eb1ae Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 09:46:27 -0400 Subject: [PATCH 03/27] KnownIssueContext -> KnownIssueScope --- Sources/Testing/Issues/Issue+Recording.swift | 2 +- Sources/Testing/Issues/KnownIssue.swift | 38 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index aff2c410e..15dd62e55 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -41,7 +41,7 @@ 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 match = KnownIssueContext.current?.match(self) { + if !isKnown, let match = KnownIssueScope.current?.match(self) { var selfCopy = self selfCopy.markAsKnown(comment: match.comment) return selfCopy.record(configuration: configuration) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 41394758c..6497ad807 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -12,13 +12,13 @@ /// /// 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. +struct KnownIssueScope: Sendable { + /// Determine if an issue is known to this scope or any of its ancestor + /// scope. /// /// Returns `nil` if the issue is not known. var match: @Sendable (Issue) -> Match? - /// The number of issues this context and its ancestors have matched. + /// The number of issues this scope and its ancestors have matched. let matchCounter: Locked struct Match { @@ -26,8 +26,8 @@ struct KnownIssueContext: Sendable { var comment: Comment? } - /// Create a new ``KnownIssueContext`` by combining a new issue matcher with - /// any previously-set context. + /// 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` @@ -36,8 +36,8 @@ struct KnownIssueContext: Sendable { /// 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?) { + /// - Returns: A new instance of ``KnownIssueScope``. + init(parent: KnownIssueScope?, issueMatcher: @escaping KnownIssueMatcher, comment: Comment?) { let matchCounter = Locked(rawValue: 0) self.matchCounter = matchCounter match = { issue in @@ -59,7 +59,7 @@ struct KnownIssueContext: Sendable { /// If there is no call to `withKnownIssue()` executing on the current task, /// the value of this property is `nil`. @TaskLocal - static var current: KnownIssueContext? + static var current: KnownIssueScope? } /// Check if an error matches using an issue-matching function, and throw it if @@ -73,10 +73,10 @@ struct KnownIssueContext: Sendable { /// function. /// - sourceLocation: The source location to which the issue should be /// attributed. -private func _matchError(_ error: any Error, using issueContext: KnownIssueContext, 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 let match = issueContext.match(issue) { + if let match = scope.match(issue) { // It's a known issue, so mark it as such before recording it. issue.markAsKnown(comment: match.comment) issue.record() @@ -217,17 +217,17 @@ public func withKnownIssue( guard precondition() else { return try body() } - let issueContext = KnownIssueContext(parent: .current, issueMatcher: issueMatcher, comment: comment) + let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, comment: comment) defer { if !isIntermittent { - _handleMiscount(by: issueContext.matchCounter, comment: comment, sourceLocation: sourceLocation) + _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) } } - try KnownIssueContext.$current.withValue(issueContext) { + try KnownIssueScope.$current.withValue(scope) { do { try body() } catch { - try _matchError(error, using: issueContext, comment: comment, sourceLocation: sourceLocation) + try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation) } } } @@ -336,17 +336,17 @@ public func withKnownIssue( guard await precondition() else { return try await body() } - let issueContext = KnownIssueContext(parent: .current, issueMatcher: issueMatcher, comment: comment) + let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, comment: comment) defer { if !isIntermittent { - _handleMiscount(by: issueContext.matchCounter, comment: comment, sourceLocation: sourceLocation) + _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) } } - try await KnownIssueContext.$current.withValue(issueContext) { + try await KnownIssueScope.$current.withValue(scope) { do { try await body() } catch { - try _matchError(error, using: issueContext, comment: comment, sourceLocation: sourceLocation) + try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation) } } } From 45f8d9fc851657701dd5ebfc02a405d1dc03d4cd Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 10:03:49 -0400 Subject: [PATCH 04/27] Issue.knownIssueContext --- .../ABI/Encoded/ABI.EncodedIssue.swift | 8 ++++--- Sources/Testing/ExitTests/ExitTest.swift | 2 +- Sources/Testing/Issues/Issue+Recording.swift | 13 +---------- Sources/Testing/Issues/Issue.swift | 22 ++++++++++++++++++- Sources/Testing/Issues/KnownIssue.swift | 12 +++++----- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift index 0ea218cc8..9ec08dcc6 100644 --- a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift +++ b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift @@ -29,8 +29,10 @@ extension ABI { /// - Warning: Severity is not yet part of the JSON schema. var _severity: Severity - /// Whether or not this issue is known to occur. - var isKnown: Bool + /// An ``Issue/KnownIssueContext-swift.struct`` representing the + /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call + /// that matched this issue, if any. + var knownIssueContext: Issue.KnownIssueContext? /// The location in source where this issue occurred, if available. var sourceLocation: SourceLocation? @@ -50,7 +52,7 @@ extension ABI { case .warning: .warning case .error: .error } - isKnown = issue.isKnown + knownIssueContext = issue.knownIssueContext sourceLocation = issue.sourceLocation if let backtrace = issue.sourceContext.backtrace { _backtrace = EncodedBacktrace(encoding: backtrace, in: eventContext) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index a28e2eede..7b2b53c2e 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -746,7 +746,7 @@ extension ExitTest { sourceLocation: issue.sourceLocation ) var issueCopy = Issue(kind: issueKind, comments: comments, sourceContext: sourceContext) - issueCopy.isKnown = issue.isKnown + issueCopy.knownIssueContext = issue.knownIssueContext issueCopy.record() } } diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index 15dd62e55..ec33aa50f 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -9,17 +9,6 @@ // extension Issue { - /// 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. /// @@ -43,7 +32,7 @@ extension Issue { // known and record the copy instead. if !isKnown, let match = KnownIssueScope.current?.match(self) { var selfCopy = self - selfCopy.markAsKnown(comment: match.comment) + selfCopy.knownIssueContext = match return selfCopy.record(configuration: configuration) } diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 5d7449b7b..f3515cc35 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, Codable { + /// The comment that was passed to `withKnownIssue()`. + var comment: Comment? + /// The source location that was passed to `withKnownIssue()`. + var sourceLocation: SourceLocation + } + + /// 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 } + set {} + } /// Initialize an issue instance with the specified details. /// diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 6497ad807..600ec63d4 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -17,7 +17,7 @@ struct KnownIssueScope: Sendable { /// scope. /// /// Returns `nil` if the issue is not known. - var match: @Sendable (Issue) -> Match? + var match: @Sendable (Issue) -> Issue.KnownIssueContext? /// The number of issues this scope and its ancestors have matched. let matchCounter: Locked @@ -37,12 +37,12 @@ struct KnownIssueScope: Sendable { /// - comment: Any comment to be associated with issues matched by /// `issueMatcher`. /// - Returns: A new instance of ``KnownIssueScope``. - init(parent: KnownIssueScope?, issueMatcher: @escaping KnownIssueMatcher, comment: Comment?) { + init(parent: KnownIssueScope?, issueMatcher: @escaping KnownIssueMatcher, context: Issue.KnownIssueContext) { let matchCounter = Locked(rawValue: 0) self.matchCounter = matchCounter match = { issue in let match = if issueMatcher(issue) { - Match(comment: comment) + context } else { parent?.match(issue) } @@ -78,7 +78,7 @@ private func _matchError(_ error: any Error, using scope: KnownIssueScope, comme var issue = Issue(kind: .errorCaught(error), comments: Array(comment), sourceContext: sourceContext) if let match = scope.match(issue) { // It's a known issue, so mark it as such before recording it. - issue.markAsKnown(comment: match.comment) + issue.knownIssueContext = match issue.record() } else { // Rethrow the error, allowing the caller to catch it or for it to propagate @@ -217,7 +217,7 @@ public func withKnownIssue( guard precondition() else { return try body() } - let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, comment: comment) + let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment, sourceLocation: sourceLocation)) defer { if !isIntermittent { _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) @@ -336,7 +336,7 @@ public func withKnownIssue( guard await precondition() else { return try await body() } - let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, comment: comment) + let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment, sourceLocation: sourceLocation)) defer { if !isIntermittent { _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) From e2da439ceb6f04aa8c476f4a2730a99474e8a4f8 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 10:08:33 -0400 Subject: [PATCH 05/27] Update comment --- Sources/Testing/Issues/KnownIssue.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 600ec63d4..f06f0325f 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -8,10 +8,10 @@ // See https://swift.org/CONTRIBUTORS.txt for Swift project authors // -/// A type that can be used to determine if an issue is a known issue. +/// A type represents an active `withKnownIssue()` call and any parent calls. /// -/// A stack of these is stored in `Issue.currentKnownIssueContext`. The stack -/// is mutated by calls to `withKnownIssue()`. +/// A stack of these is stored in `KnownIssueContext.current`. The stack is +/// mutated by calls to `withKnownIssue()`. struct KnownIssueScope: Sendable { /// Determine if an issue is known to this scope or any of its ancestor /// scope. From e0251d8f6f12aff8f60a2f301b10ba6d17f906f1 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 10:08:58 -0400 Subject: [PATCH 06/27] Fix typo --- Sources/Testing/Issues/KnownIssue.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index f06f0325f..f3b93077a 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -8,7 +8,7 @@ // See https://swift.org/CONTRIBUTORS.txt for Swift project authors // -/// A type represents an active `withKnownIssue()` call and any parent calls. +/// A type that represents an active `withKnownIssue()` call and any parent calls. /// /// A stack of these is stored in `KnownIssueContext.current`. The stack is /// mutated by calls to `withKnownIssue()`. From 89b0a67160c59dfc351218f6b1e95f1245268750 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 10:09:39 -0400 Subject: [PATCH 07/27] Remove KnownIssueScope.Match --- Sources/Testing/Issues/KnownIssue.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index f3b93077a..f15570e1d 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -14,18 +14,13 @@ /// mutated by calls to `withKnownIssue()`. struct KnownIssueScope: Sendable { /// Determine if an issue is known to this scope or any of its ancestor - /// scope. + /// scopes. /// /// Returns `nil` if the issue is not known. var match: @Sendable (Issue) -> Issue.KnownIssueContext? /// The number of issues this scope and its ancestors have matched. let matchCounter: Locked - struct Match { - /// The comment that was passed to the `withKnownIssue()` call that matched the issue. - var comment: Comment? - } - /// Create a new ``KnownIssueScope`` by combining a new issue matcher with /// any already-active scope. /// From deffa2908330f8d37bfbefa2cc095303787c1585 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 10:10:44 -0400 Subject: [PATCH 08/27] Renames for clarity --- Sources/Testing/Issues/Issue+Recording.swift | 4 ++-- Sources/Testing/Issues/KnownIssue.swift | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index ec33aa50f..f77a81607 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -30,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 match = KnownIssueScope.current?.match(self) { + if !isKnown, let context = KnownIssueScope.current?.match(self) { var selfCopy = self - selfCopy.knownIssueContext = match + selfCopy.knownIssueContext = context return selfCopy.record(configuration: configuration) } diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index f15570e1d..186382fd6 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -36,15 +36,15 @@ struct KnownIssueScope: Sendable { let matchCounter = Locked(rawValue: 0) self.matchCounter = matchCounter match = { issue in - let match = if issueMatcher(issue) { + let matchedContext = if issueMatcher(issue) { context } else { parent?.match(issue) } - if match != nil { + if matchedContext != nil { matchCounter.increment() } - return match + return matchedContext } } @@ -71,9 +71,9 @@ struct KnownIssueScope: Sendable { 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 let match = scope.match(issue) { + if let context = scope.match(issue) { // It's a known issue, so mark it as such before recording it. - issue.knownIssueContext = match + issue.knownIssueContext = context issue.record() } else { // Rethrow the error, allowing the caller to catch it or for it to propagate From e6e4fd434f8942ec630889460ea3f640365bfe0a Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Wed, 12 Mar 2025 10:18:24 -0400 Subject: [PATCH 09/27] Update tests --- Tests/TestingTests/KnownIssueTests.swift | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Tests/TestingTests/KnownIssueTests.swift b/Tests/TestingTests/KnownIssueTests.swift index 1be86764f..6e41a6181 100644 --- a/Tests/TestingTests/KnownIssueTests.swift +++ b/Tests/TestingTests/KnownIssueTests.swift @@ -64,6 +64,7 @@ final class KnownIssueTests: XCTestCase { func testKnownIssueRecordedWithComment() async { let issueRecorded = expectation(description: "Issue recorded") + let sourceLocation = SourceLocation(fileID: "FakeModule/FakeFile.swift", filePath: "", line: 9999, column: 1) var configuration = Configuration() configuration.eventHandler = { event, _ in @@ -76,12 +77,14 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments, ["With Known Issue Comment", "Issue Comment"]) + XCTAssertEqual(issue.comments, ["Issue Comment"]) + XCTAssertEqual(issue.knownIssueContext?.comment, "With Known Issue Comment") + XCTAssertEqual(issue.knownIssueContext?.sourceLocation, sourceLocation) XCTAssertTrue(issue.isKnown) } await Test { - withKnownIssue("With Known Issue Comment") { + withKnownIssue("With Known Issue Comment", sourceLocation: sourceLocation) { Issue.record("Issue Comment") } }.run(configuration: configuration) @@ -147,6 +150,7 @@ final class KnownIssueTests: XCTestCase { func testKnownIssueRecordedWithInnermostMatchingComment() async { let issueRecorded = expectation(description: "Issue recorded") + let sourceLocation = SourceLocation(fileID: "FakeModule/FakeFile.swift", filePath: "", line: 9999, column: 1) var configuration = Configuration() configuration.eventHandler = { event, _ in @@ -159,14 +163,16 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments, ["Inner Contains B", "Issue B"]) + XCTAssertEqual(issue.comments, ["Issue B"]) + XCTAssertEqual(issue.knownIssueContext?.comment, "Inner Contains B") + XCTAssertEqual(issue.knownIssueContext?.sourceLocation, sourceLocation) XCTAssertTrue(issue.isKnown) } await Test { withKnownIssue("Contains A", isIntermittent: true) { withKnownIssue("Outer Contains B", isIntermittent: true) { - withKnownIssue("Inner Contains B") { + withKnownIssue("Inner Contains B", sourceLocation: sourceLocation) { withKnownIssue("Contains C", isIntermittent: true) { Issue.record("Issue B") } matching: { issue in From a9b31633b8fac59ece23eeac2b5ac5834d2be36d Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 18 Mar 2025 10:50:36 -0400 Subject: [PATCH 10/27] Fix up doc comments --- Sources/Testing/Issues/KnownIssue.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 186382fd6..da9bc9e78 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -10,7 +10,7 @@ /// A type that represents an active `withKnownIssue()` call and any parent calls. /// -/// A stack of these is stored in `KnownIssueContext.current`. The stack is +/// A stack of these is stored in `KnownIssueScope.current`. The stack is /// mutated by calls to `withKnownIssue()`. struct KnownIssueScope: Sendable { /// Determine if an issue is known to this scope or any of its ancestor @@ -29,7 +29,7 @@ struct KnownIssueScope: Sendable { /// 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 + /// - 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) { From 1282cab332aad333f93c14a8197182f54e0b27bd Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 18 Mar 2025 10:50:44 -0400 Subject: [PATCH 11/27] Remove Issue.isKnown.set since this is only SPI --- Sources/Testing/Issues/Issue.swift | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index f3515cc35..964853e42 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -134,10 +134,7 @@ public struct Issue: Sendable { /// Whether or not this issue is known to occur. @_spi(ForToolsIntegrationOnly) - public var isKnown: Bool { - get { knownIssueContext != nil } - set {} - } + public var isKnown: Bool { knownIssueContext != nil } /// Initialize an issue instance with the specified details. /// From 0dfcff8b7ab06065f8b64e03cd40bf228e53dd3e Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Tue, 18 Mar 2025 10:52:41 -0400 Subject: [PATCH 12/27] Preserve EncodedIssue ABI --- Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift | 12 ++++++++++-- Sources/Testing/ExitTests/ExitTest.swift | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift index 9ec08dcc6..0a65bec5d 100644 --- a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift +++ b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift @@ -29,10 +29,17 @@ extension ABI { /// - Warning: Severity is not yet part of the JSON schema. var _severity: Severity + /// Whether or not this issue is known to occur. + /// + /// This should perhaps be deprecated in favor of `_knownIssueContext`. + var isKnown: Bool + /// An ``Issue/KnownIssueContext-swift.struct`` representing the /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call /// that matched this issue, if any. - var knownIssueContext: Issue.KnownIssueContext? + /// + /// - Warning: Severity is not yet part of the JSON schema. + var _knownIssueContext: Issue.KnownIssueContext? /// The location in source where this issue occurred, if available. var sourceLocation: SourceLocation? @@ -52,7 +59,8 @@ extension ABI { case .warning: .warning case .error: .error } - knownIssueContext = issue.knownIssueContext + isKnown = issue.isKnown + _knownIssueContext = issue.knownIssueContext sourceLocation = issue.sourceLocation if let backtrace = issue.sourceContext.backtrace { _backtrace = EncodedBacktrace(encoding: backtrace, in: eventContext) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 7b2b53c2e..fe8c74107 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -746,7 +746,7 @@ extension ExitTest { sourceLocation: issue.sourceLocation ) var issueCopy = Issue(kind: issueKind, comments: comments, sourceContext: sourceContext) - issueCopy.knownIssueContext = issue.knownIssueContext + issueCopy.knownIssueContext = issue._knownIssueContext issueCopy.record() } } From 7cd86f6ed33b91c622011870fd9ecfabcfaad94f Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 24 Mar 2025 11:32:38 -0400 Subject: [PATCH 13/27] Add ABI.EncodedKnownIssueContext --- .../ABI/Encoded/ABI.EncodedIssue.swift | 6 ++- .../ABI.EncodedKnownIssueContext.swift | 37 +++++++++++++++++++ Sources/Testing/ExitTests/ExitTest.swift | 4 +- Sources/Testing/Issues/Issue.swift | 2 +- 4 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift index 0a65bec5d..2a8c08b2a 100644 --- a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift +++ b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift @@ -39,7 +39,7 @@ extension ABI { /// that matched this issue, if any. /// /// - Warning: Severity is not yet part of the JSON schema. - var _knownIssueContext: Issue.KnownIssueContext? + var _knownIssueContext: EncodedKnownIssueContext? /// The location in source where this issue occurred, if available. var sourceLocation: SourceLocation? @@ -60,7 +60,9 @@ extension ABI { case .error: .error } isKnown = issue.isKnown - _knownIssueContext = issue.knownIssueContext + if let knownIssueContext = issue.knownIssueContext { + _knownIssueContext = EncodedKnownIssueContext(encoding: knownIssueContext, in: eventContext) + } sourceLocation = issue.sourceLocation if let backtrace = issue.sourceContext.backtrace { _backtrace = EncodedBacktrace(encoding: backtrace, in: eventContext) diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift new file mode 100644 index 000000000..2f9fc9cdb --- /dev/null +++ b/Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift @@ -0,0 +1,37 @@ +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for Swift project authors +// + +extension ABI { + /// A type implementing the JSON encoding of + /// ``Issue/KnownIssueContext-swift.struct`` for the ABI entry point and + /// event stream output. + /// + /// This type is not part of the public interface of the testing library. It + /// assists in converting values to JSON; clients that consume this JSON are + /// expected to write their own decoders. + /// + /// - Warning: Known issues are not yet part of the JSON schema. + struct EncodedKnownIssueContext: Sendable where V: ABI.Version { + /// The comment that was passed to `withKnownIssue()`, if any. + var comment: Comment? + + /// The source location that was passed to `withKnownIssue()`. + var sourceLocation: SourceLocation + + init(encoding context: Issue.KnownIssueContext, in eventContext: borrowing Event.Context) { + comment = context.comment + sourceLocation = context.sourceLocation + } + } +} + +// MARK: - Codable + +extension ABI.EncodedKnownIssueContext: Codable {} diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index fe8c74107..456f6f86a 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -746,7 +746,9 @@ extension ExitTest { sourceLocation: issue.sourceLocation ) var issueCopy = Issue(kind: issueKind, comments: comments, sourceContext: sourceContext) - issueCopy.knownIssueContext = issue._knownIssueContext + if let context = issue._knownIssueContext { + issueCopy.knownIssueContext = Issue.KnownIssueContext(comment: context.comment, sourceLocation: context.sourceLocation) + } issueCopy.record() } } diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 964853e42..526ad661d 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -119,7 +119,7 @@ public struct Issue: Sendable { /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call /// that matched an issue. @_spi(ForToolsIntegrationOnly) - public struct KnownIssueContext: Sendable, Codable { + public struct KnownIssueContext: Sendable { /// The comment that was passed to `withKnownIssue()`. var comment: Comment? /// The source location that was passed to `withKnownIssue()`. From c0e8ce70e45e8d49b69180085b35863d795e30f2 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 24 Mar 2025 11:32:53 -0400 Subject: [PATCH 14/27] Fix doc comments --- Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift | 2 +- Sources/Testing/Issues/Issue.swift | 3 ++- Sources/Testing/Issues/KnownIssue.swift | 3 +-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift index 2a8c08b2a..723ee687a 100644 --- a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift +++ b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift @@ -38,7 +38,7 @@ extension ABI { /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call /// that matched this issue, if any. /// - /// - Warning: Severity is not yet part of the JSON schema. + /// - Warning: Known issues are not yet part of the JSON schema. var _knownIssueContext: EncodedKnownIssueContext? /// The location in source where this issue occurred, if available. diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 526ad661d..5cb140db8 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -120,8 +120,9 @@ public struct Issue: Sendable { /// that matched an issue. @_spi(ForToolsIntegrationOnly) public struct KnownIssueContext: Sendable { - /// The comment that was passed to `withKnownIssue()`. + /// The comment that was passed to `withKnownIssue()`, if any. var comment: Comment? + /// The source location that was passed to `withKnownIssue()`. var sourceLocation: SourceLocation } diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index da9bc9e78..c81e67200 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -62,8 +62,7 @@ struct KnownIssueScope: Sendable { /// /// - 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: A scope that represents the enclosing `withKnownIssue()` call. /// - comment: An optional comment to apply to any issues generated by this /// function. /// - sourceLocation: The source location to which the issue should be From af08da21d198aa8e51c0ade8e4de225b92ccf6cd Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 10:00:01 -0400 Subject: [PATCH 15/27] Replace EncodedIssue._knownIssueContext with an extra Message --- .../ABI/Encoded/ABI.EncodedIssue.swift | 12 ------ .../ABI.EncodedKnownIssueContext.swift | 37 ------------------- .../Event.HumanReadableOutputRecorder.swift | 19 ++++++++-- Sources/Testing/ExitTests/ExitTest.swift | 6 ++- 4 files changed, 20 insertions(+), 54 deletions(-) delete mode 100644 Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift index 723ee687a..0ea218cc8 100644 --- a/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift +++ b/Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift @@ -30,17 +30,8 @@ extension ABI { var _severity: Severity /// Whether or not this issue is known to occur. - /// - /// This should perhaps be deprecated in favor of `_knownIssueContext`. var isKnown: Bool - /// An ``Issue/KnownIssueContext-swift.struct`` representing the - /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` call - /// that matched this issue, if any. - /// - /// - Warning: Known issues are not yet part of the JSON schema. - var _knownIssueContext: EncodedKnownIssueContext? - /// The location in source where this issue occurred, if available. var sourceLocation: SourceLocation? @@ -60,9 +51,6 @@ extension ABI { case .error: .error } isKnown = issue.isKnown - if let knownIssueContext = issue.knownIssueContext { - _knownIssueContext = EncodedKnownIssueContext(encoding: knownIssueContext, in: eventContext) - } sourceLocation = issue.sourceLocation if let backtrace = issue.sourceContext.backtrace { _backtrace = EncodedBacktrace(encoding: backtrace, in: eventContext) diff --git a/Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift b/Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift deleted file mode 100644 index 2f9fc9cdb..000000000 --- a/Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift +++ /dev/null @@ -1,37 +0,0 @@ -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2025 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// See https://swift.org/CONTRIBUTORS.txt for Swift project authors -// - -extension ABI { - /// A type implementing the JSON encoding of - /// ``Issue/KnownIssueContext-swift.struct`` for the ABI entry point and - /// event stream output. - /// - /// This type is not part of the public interface of the testing library. It - /// assists in converting values to JSON; clients that consume this JSON are - /// expected to write their own decoders. - /// - /// - Warning: Known issues are not yet part of the JSON schema. - struct EncodedKnownIssueContext: Sendable where V: ABI.Version { - /// The comment that was passed to `withKnownIssue()`, if any. - var comment: Comment? - - /// The source location that was passed to `withKnownIssue()`. - var sourceLocation: SourceLocation - - init(encoding context: Issue.KnownIssueContext, in eventContext: borrowing Event.Context) { - comment = context.comment - sourceLocation = context.sourceLocation - } - } -} - -// MARK: - Codable - -extension ABI.EncodedKnownIssueContext: Codable {} diff --git a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift index 0e856facf..602d8ee4b 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 456f6f86a..2f96fc123 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -746,8 +746,10 @@ extension ExitTest { sourceLocation: issue.sourceLocation ) var issueCopy = Issue(kind: issueKind, comments: comments, sourceContext: sourceContext) - if let context = issue._knownIssueContext { - issueCopy.knownIssueContext = Issue.KnownIssueContext(comment: context.comment, sourceLocation: context.sourceLocation) + 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() } From b66a484318b51f8cbc4a366d3420bf1b8921e454 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 10:01:08 -0400 Subject: [PATCH 16/27] Remove KnownIssueContext.sourceLocation We don't have a use for it yet. --- Sources/Testing/Issues/Issue.swift | 5 +---- Sources/Testing/Issues/KnownIssue.swift | 4 ++-- Tests/TestingTests/KnownIssueTests.swift | 2 -- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 5cb140db8..0a8381e8d 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -121,10 +121,7 @@ public struct Issue: Sendable { @_spi(ForToolsIntegrationOnly) public struct KnownIssueContext: Sendable { /// The comment that was passed to `withKnownIssue()`, if any. - var comment: Comment? - - /// The source location that was passed to `withKnownIssue()`. - var sourceLocation: SourceLocation + public var comment: Comment? } /// A ``KnownIssueContext-swift.struct`` representing the diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index c81e67200..63f1d8c12 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -211,7 +211,7 @@ public func withKnownIssue( guard precondition() else { return try body() } - let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment, sourceLocation: sourceLocation)) + let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment)) defer { if !isIntermittent { _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) @@ -330,7 +330,7 @@ public func withKnownIssue( guard await precondition() else { return try await body() } - let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment, sourceLocation: sourceLocation)) + let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment)) defer { if !isIntermittent { _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) diff --git a/Tests/TestingTests/KnownIssueTests.swift b/Tests/TestingTests/KnownIssueTests.swift index 6e41a6181..c85e79777 100644 --- a/Tests/TestingTests/KnownIssueTests.swift +++ b/Tests/TestingTests/KnownIssueTests.swift @@ -79,7 +79,6 @@ final class KnownIssueTests: XCTestCase { XCTAssertEqual(issue.comments, ["Issue Comment"]) XCTAssertEqual(issue.knownIssueContext?.comment, "With Known Issue Comment") - XCTAssertEqual(issue.knownIssueContext?.sourceLocation, sourceLocation) XCTAssertTrue(issue.isKnown) } @@ -165,7 +164,6 @@ final class KnownIssueTests: XCTestCase { XCTAssertEqual(issue.comments, ["Issue B"]) XCTAssertEqual(issue.knownIssueContext?.comment, "Inner Contains B") - XCTAssertEqual(issue.knownIssueContext?.sourceLocation, sourceLocation) XCTAssertTrue(issue.isKnown) } From b9275e899c722b34823fa08e83e6d9385f9ae1e7 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 10:01:20 -0400 Subject: [PATCH 17/27] Resurrect Issue.isKnown.setter --- Sources/Testing/Issues/Issue.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 0a8381e8d..0d8491adc 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -132,7 +132,11 @@ public struct Issue: Sendable { /// Whether or not this issue is known to occur. @_spi(ForToolsIntegrationOnly) - public var isKnown: Bool { knownIssueContext != nil } + 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. /// From 81fe877c142be19f4c8e58ff79fec2f92a107608 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 10:06:42 -0400 Subject: [PATCH 18/27] Remove `withKnownIssue()` shorthand --- Sources/Testing/Issues/Issue.swift | 3 ++- Sources/Testing/Issues/KnownIssue.swift | 17 +++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 0d8491adc..d5befd1ed 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -120,7 +120,8 @@ public struct Issue: Sendable { /// that matched an issue. @_spi(ForToolsIntegrationOnly) public struct KnownIssueContext: Sendable { - /// The comment that was passed to `withKnownIssue()`, if any. + /// The comment that was passed to + /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``. public var comment: Comment? } diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 63f1d8c12..1ccdee5f7 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -8,10 +8,11 @@ // See https://swift.org/CONTRIBUTORS.txt for Swift project authors // -/// A type that represents an active `withKnownIssue()` call and any parent calls. +/// 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`. The stack is -/// mutated by calls to `withKnownIssue()`. +/// A stack of these is stored in `KnownIssueScope.current`. struct KnownIssueScope: Sendable { /// Determine if an issue is known to this scope or any of its ancestor /// scopes. @@ -48,11 +49,11 @@ struct KnownIssueScope: Sendable { } } - /// The known issue context, as set by `withKnownIssue()`, associated with the - /// current task. + /// The active known issue scope for the current task. /// - /// If there is no call to `withKnownIssue()` executing on the current task, - /// the value of this property is `nil`. + /// 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? } @@ -62,7 +63,7 @@ struct KnownIssueScope: Sendable { /// /// - Parameters: /// - error: The error to test. -/// - scope: A scope that represents the enclosing `withKnownIssue()` call. +/// - 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 From 9192e4a06b355cd95ff6ebc08d9470e1fb9266a8 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 10:19:29 -0400 Subject: [PATCH 19/27] Add a test for the HumanReadableOutputRecorder change --- Tests/TestingTests/EventRecorderTests.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Tests/TestingTests/EventRecorderTests.swift b/Tests/TestingTests/EventRecorderTests.swift index 8ac7f6728..28186c43e 100644 --- a/Tests/TestingTests/EventRecorderTests.swift +++ b/Tests/TestingTests/EventRecorderTests.swift @@ -497,6 +497,20 @@ struct EventRecorderTests { recorder.record(Event(.runEnded, testID: nil, testCaseID: nil), in: context) } } + + @Test("HumanReadableOutputRecorder includes known issue comment in messages array") + func humanReadableRecorderIncludesKnownIssueCommentInMessagesArray() { + var issue = Issue(kind: .unconditional) + issue.knownIssueContext = Issue.KnownIssueContext(comment: "Known issue comment") + let event = Event(.issueRecorded(issue), testID: nil, testCaseID: nil) + let context = Event.Context(test: nil, testCase: nil, configuration: nil) + + let recorder = Event.HumanReadableOutputRecorder() + let messages = recorder.record(event, in: context) + #expect( + messages.map(\.stringValue).contains("Known issue comment") + ) + } } // MARK: - Fixtures From feb96cf762e5732f8702e1dc9755fa2908bfb5ba Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 10:26:37 -0400 Subject: [PATCH 20/27] Rename KnownIssueScope.match -> matcher --- Sources/Testing/Issues/Issue+Recording.swift | 2 +- Sources/Testing/Issues/KnownIssue.swift | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index f77a81607..098fb2e8d 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -30,7 +30,7 @@ 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 context = KnownIssueScope.current?.match(self) { + if !isKnown, let context = KnownIssueScope.current?.matcher(self) { var selfCopy = self selfCopy.knownIssueContext = context return selfCopy.record(configuration: configuration) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 1ccdee5f7..8a59ca294 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -14,11 +14,22 @@ /// /// 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 match: @Sendable (Issue) -> Issue.KnownIssueContext? + var matcher: @Sendable (Issue) -> Issue.KnownIssueContext? + /// The number of issues this scope and its ancestors have matched. let matchCounter: Locked @@ -36,11 +47,11 @@ struct KnownIssueScope: Sendable { init(parent: KnownIssueScope?, issueMatcher: @escaping KnownIssueMatcher, context: Issue.KnownIssueContext) { let matchCounter = Locked(rawValue: 0) self.matchCounter = matchCounter - match = { issue in + matcher = { issue in let matchedContext = if issueMatcher(issue) { context } else { - parent?.match(issue) + parent?.matcher(issue) } if matchedContext != nil { matchCounter.increment() @@ -71,7 +82,7 @@ struct KnownIssueScope: Sendable { 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 let context = scope.match(issue) { + if let context = scope.matcher(issue) { // It's a known issue, so mark it as such before recording it. issue.knownIssueContext = context issue.record() From 874f830da984c74ea3ccaed4ac531f4c5ca47d1d Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 11:28:27 -0400 Subject: [PATCH 21/27] Deduplicate known issue comments for thrown errors --- .../Event.HumanReadableOutputRecorder.swift | 6 +- Tests/TestingTests/EventRecorderTests.swift | 69 ++++++++++++++++--- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift index 602d8ee4b..3c3e8b686 100644 --- a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift +++ b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift @@ -454,7 +454,11 @@ extension Event.HumanReadableOutputRecorder { } additionalMessages += _formattedComments(issue.comments) if let knownIssueComment = issue.knownIssueContext?.comment { - additionalMessages.append(_formattedComment(knownIssueComment)) + if case .errorCaught = issue.kind { + // The known issue comment is already included in `issue.comments`. + } else { + additionalMessages.append(_formattedComment(knownIssueComment)) + } } if verbosity > 0, case let .expectationFailed(expectation) = issue.kind { diff --git a/Tests/TestingTests/EventRecorderTests.swift b/Tests/TestingTests/EventRecorderTests.swift index 28186c43e..8c6b390a5 100644 --- a/Tests/TestingTests/EventRecorderTests.swift +++ b/Tests/TestingTests/EventRecorderTests.swift @@ -498,18 +498,33 @@ struct EventRecorderTests { } } - @Test("HumanReadableOutputRecorder includes known issue comment in messages array") - func humanReadableRecorderIncludesKnownIssueCommentInMessagesArray() { - var issue = Issue(kind: .unconditional) - issue.knownIssueContext = Issue.KnownIssueContext(comment: "Known issue comment") - let event = Event(.issueRecorded(issue), testID: nil, testCaseID: nil) - let context = Event.Context(test: nil, testCase: nil, configuration: nil) - + @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 = recorder.record(event, in: context) - #expect( - messages.map(\.stringValue).contains("Known issue comment") - ) + 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.withLock(\.self).dropFirst().map(\.stringValue) + #expect(actualComments == expectedComments) } } @@ -653,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() + } + } +} From 6caad072323a3189b9de2732ad07c77a306629cc Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 12:21:33 -0400 Subject: [PATCH 22/27] Use Locked.rawValue Co-authored-by: Jonathan Grynspan --- Tests/TestingTests/EventRecorderTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/TestingTests/EventRecorderTests.swift b/Tests/TestingTests/EventRecorderTests.swift index 8c6b390a5..0db62f6a8 100644 --- a/Tests/TestingTests/EventRecorderTests.swift +++ b/Tests/TestingTests/EventRecorderTests.swift @@ -523,7 +523,7 @@ struct EventRecorderTests { // 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.withLock(\.self).dropFirst().map(\.stringValue) + let actualComments = messages.rawValue.dropFirst().map(\.stringValue) #expect(actualComments == expectedComments) } } From 316d10c16bce57e93ffdebba387a9bffa9435ea6 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Mon, 7 Apr 2025 12:24:41 -0400 Subject: [PATCH 23/27] Explain why we treat .errorCaught specially --- .../Events/Recorder/Event.HumanReadableOutputRecorder.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift index 3c3e8b686..75c652622 100644 --- a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift +++ b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift @@ -455,7 +455,9 @@ extension Event.HumanReadableOutputRecorder { additionalMessages += _formattedComments(issue.comments) if let knownIssueComment = issue.knownIssueContext?.comment { if case .errorCaught = issue.kind { - // The known issue comment is already included in `issue.comments`. + // `_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)) } From 8567a61ad39ee44076377e06e28000834bdcb359 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Thu, 24 Apr 2025 10:00:03 -0400 Subject: [PATCH 24/27] Apply suggestions from code review Co-authored-by: Stuart Montgomery --- .../Recorder/Event.HumanReadableOutputRecorder.swift | 2 +- Sources/Testing/Issues/KnownIssue.swift | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift index 75c652622..d27ecef66 100644 --- a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift +++ b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift @@ -94,7 +94,7 @@ extension Event.HumanReadableOutputRecorder { /// - comments: The comments that should be formatted. /// /// - Returns: An array of formatted messages representing `comments`, or an - /// empty array if there are none. + /// empty array if there are none. private func _formattedComments(_ comments: [Comment]) -> [Message] { comments.map(_formattedComment) } diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 8a59ca294..225ba5fdd 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -19,10 +19,11 @@ struct KnownIssueScope: Sendable { /// /// - 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? + /// 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? /// Determine if an issue is known to this scope or any of its ancestor /// scopes. From 1ae86337b2c213dc02a1c4fae31e470379b64543 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Thu, 24 Apr 2025 10:12:52 -0400 Subject: [PATCH 25/27] Don't put the known issue comment in Issue.comments for thrown errors --- .../Event.HumanReadableOutputRecorder.swift | 8 +--- Sources/Testing/Issues/KnownIssue.swift | 2 +- Tests/TestingTests/KnownIssueTests.swift | 42 ++++++++++++------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift index d27ecef66..7947dbab1 100644 --- a/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift +++ b/Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift @@ -454,13 +454,7 @@ extension Event.HumanReadableOutputRecorder { } 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)) - } + additionalMessages.append(_formattedComment(knownIssueComment)) } if verbosity > 0, case let .expectationFailed(expectation) = issue.kind { diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 225ba5fdd..f08ee795a 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -82,7 +82,7 @@ struct KnownIssueScope: Sendable { /// attributed. 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) + 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.knownIssueContext = context diff --git a/Tests/TestingTests/KnownIssueTests.swift b/Tests/TestingTests/KnownIssueTests.swift index c85e79777..cfcd3f0df 100644 --- a/Tests/TestingTests/KnownIssueTests.swift +++ b/Tests/TestingTests/KnownIssueTests.swift @@ -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 { @@ -63,8 +63,8 @@ final class KnownIssueTests: XCTestCase { } func testKnownIssueRecordedWithComment() async { + let issueMatched = expectation(description: "Issue matched") let issueRecorded = expectation(description: "Issue recorded") - let sourceLocation = SourceLocation(fileID: "FakeModule/FakeFile.swift", filePath: "", line: 9999, column: 1) var configuration = Configuration() configuration.eventHandler = { event, _ in @@ -77,21 +77,29 @@ final class KnownIssueTests: XCTestCase { return } + XCTAssertTrue(issue.isKnown) XCTAssertEqual(issue.comments, ["Issue Comment"]) XCTAssertEqual(issue.knownIssueContext?.comment, "With Known Issue Comment") - XCTAssertTrue(issue.isKnown) } await Test { - withKnownIssue("With Known Issue Comment", sourceLocation: sourceLocation) { + 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: [issueRecorded], timeout: 0.0) + 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() @@ -105,19 +113,26 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments, ["With Known Issue Comment"]) XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["With Known Issue Comment"]) } struct E: Error {} await Test { - withKnownIssue("With Known Issue Comment") { + 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: [issueRecorded], timeout: 0.0) + await fulfillment(of: [issueMatched, issueRecorded], timeout: 0.0) } func testKnownIssueRecordedWithNoComment() async { @@ -134,8 +149,8 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments, ["Issue Comment"]) XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue Comment"]) } await Test { @@ -149,7 +164,6 @@ final class KnownIssueTests: XCTestCase { func testKnownIssueRecordedWithInnermostMatchingComment() async { let issueRecorded = expectation(description: "Issue recorded") - let sourceLocation = SourceLocation(fileID: "FakeModule/FakeFile.swift", filePath: "", line: 9999, column: 1) var configuration = Configuration() configuration.eventHandler = { event, _ in @@ -162,15 +176,15 @@ final class KnownIssueTests: XCTestCase { return } + XCTAssertTrue(issue.isKnown) XCTAssertEqual(issue.comments, ["Issue B"]) XCTAssertEqual(issue.knownIssueContext?.comment, "Inner Contains B") - XCTAssertTrue(issue.isKnown) } await Test { withKnownIssue("Contains A", isIntermittent: true) { withKnownIssue("Outer Contains B", isIntermittent: true) { - withKnownIssue("Inner Contains B", sourceLocation: sourceLocation) { + withKnownIssue("Inner Contains B") { withKnownIssue("Contains C", isIntermittent: true) { Issue.record("Issue B") } matching: { issue in @@ -204,8 +218,8 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments, ["Inner Is B", "B"]) XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Inner Is B", "B"]) } struct A: Error {} @@ -249,8 +263,8 @@ final class KnownIssueTests: XCTestCase { return } - XCTAssertEqual(issue.comments, ["Issue B"]) XCTAssertTrue(issue.isKnown) + XCTAssertEqual(issue.comments, ["Issue B"]) } await Test { From 00ad66d80ddcd394c47d64003f7f5ff12cebebb2 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Thu, 24 Apr 2025 11:58:43 -0400 Subject: [PATCH 26/27] Apply suggestions from code review Co-authored-by: Stuart Montgomery --- Sources/Testing/Issues/KnownIssue.swift | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index f08ee795a..d4627a126 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -25,11 +25,8 @@ struct KnownIssueScope: Sendable { /// ancestor scope, or `nil` otherwise. typealias Matcher = @Sendable (_ issue: 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 matcher function for this known issue scope. + var matcher: Matcher /// The number of issues this scope and its ancestors have matched. let matchCounter: Locked @@ -44,7 +41,6 @@ struct KnownIssueScope: Sendable { /// 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 @@ -61,7 +57,7 @@ struct KnownIssueScope: Sendable { } } - /// The active known issue scope for the current task. + /// The active known issue scope for the current task, if any. /// /// If there is no call to /// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)`` From 0a3a99597e8d02af8d0fb2064a57b1a46b08fe04 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Thu, 24 Apr 2025 15:25:08 -0400 Subject: [PATCH 27/27] Incorporate review comments --- Sources/Testing/Issues/KnownIssue.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index d4627a126..f59185388 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -36,12 +36,12 @@ struct KnownIssueScope: Sendable { /// /// - Parameters: /// - parent: The context that should be checked next if `issueMatcher` - /// fails to match an issue. + /// 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?, issueMatcher: @escaping KnownIssueMatcher, context: Issue.KnownIssueContext) { + init(parent: KnownIssueScope? = .current, issueMatcher: @escaping KnownIssueMatcher, context: Issue.KnownIssueContext) { let matchCounter = Locked(rawValue: 0) self.matchCounter = matchCounter matcher = { issue in @@ -76,7 +76,7 @@ struct KnownIssueScope: Sendable { /// function. /// - sourceLocation: The source location to which the issue should be /// attributed. -private func _matchError(_ error: any Error, using scope: KnownIssueScope, 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: [], sourceContext: sourceContext) if let context = scope.matcher(issue) { @@ -220,7 +220,7 @@ public func withKnownIssue( guard precondition() else { return try body() } - let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment)) + let scope = KnownIssueScope(issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment)) defer { if !isIntermittent { _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) @@ -230,7 +230,7 @@ public func withKnownIssue( do { try body() } catch { - try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation) + try _matchError(error, in: scope, comment: comment, sourceLocation: sourceLocation) } } } @@ -339,7 +339,7 @@ public func withKnownIssue( guard await precondition() else { return try await body() } - let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment)) + let scope = KnownIssueScope(issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment)) defer { if !isIntermittent { _handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation) @@ -349,7 +349,7 @@ public func withKnownIssue( do { try await body() } catch { - try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation) + try _matchError(error, in: scope, comment: comment, sourceLocation: sourceLocation) } } }