Skip to content

Add withKnownIssue comments to known Issues #1014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c6cbd3d
Add withKnownIssue comments to known Issues
aroben Mar 10, 2025
acdb283
KnownIssueContext.current
aroben Mar 12, 2025
08b1c3f
KnownIssueContext -> KnownIssueScope
aroben Mar 12, 2025
45f8d9f
Issue.knownIssueContext
aroben Mar 12, 2025
e2da439
Update comment
aroben Mar 12, 2025
e0251d8
Fix typo
aroben Mar 12, 2025
89b0a67
Remove KnownIssueScope.Match
aroben Mar 12, 2025
deffa29
Renames for clarity
aroben Mar 12, 2025
e6e4fd4
Update tests
aroben Mar 12, 2025
a9b3163
Fix up doc comments
aroben Mar 18, 2025
1282cab
Remove Issue.isKnown.set since this is only SPI
aroben Mar 18, 2025
0dfcff8
Preserve EncodedIssue ABI
aroben Mar 18, 2025
7cd86f6
Add ABI.EncodedKnownIssueContext
aroben Mar 24, 2025
c0e8ce7
Fix doc comments
aroben Mar 24, 2025
af08da2
Replace EncodedIssue._knownIssueContext with an extra Message
aroben Apr 7, 2025
b66a484
Remove KnownIssueContext.sourceLocation
aroben Apr 7, 2025
b9275e8
Resurrect Issue.isKnown.setter
aroben Apr 7, 2025
81fe877
Remove `withKnownIssue()` shorthand
aroben Apr 7, 2025
9192e4a
Add a test for the HumanReadableOutputRecorder change
aroben Apr 7, 2025
feb96cf
Rename KnownIssueScope.match -> matcher
aroben Apr 7, 2025
874f830
Deduplicate known issue comments for thrown errors
aroben Apr 7, 2025
6caad07
Use Locked.rawValue
aroben Apr 7, 2025
316d10c
Explain why we treat .errorCaught specially
aroben Apr 7, 2025
8567a61
Apply suggestions from code review
aroben Apr 24, 2025
1ae8633
Don't put the known issue comment in Issue.comments for thrown errors
aroben Apr 24, 2025
00ad66d
Apply suggestions from code review
aroben Apr 24, 2025
0a3a995
Incorporate review comments
aroben Apr 24, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Sources/Testing/ABI/Encoded/ABI.EncodedIssue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,17 @@ 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<V>?

/// The location in source where this issue occurred, if available.
var sourceLocation: SourceLocation?

Expand All @@ -51,6 +60,9 @@ 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)
Expand Down
37 changes: 37 additions & 0 deletions Sources/Testing/ABI/Encoded/ABI.EncodedKnownIssueContext.swift
Original file line number Diff line number Diff line change
@@ -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<V>: 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 {}
4 changes: 3 additions & 1 deletion Sources/Testing/ExitTests/ExitTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,9 @@ extension ExitTest {
sourceLocation: issue.sourceLocation
)
var issueCopy = Issue(kind: issueKind, comments: comments, sourceContext: sourceContext)
issueCopy.isKnown = issue.isKnown
if let context = issue._knownIssueContext {
issueCopy.knownIssueContext = Issue.KnownIssueContext(comment: context.comment, sourceLocation: context.sourceLocation)
}
issueCopy.record()
}
}
Expand Down
12 changes: 2 additions & 10 deletions Sources/Testing/Issues/Issue+Recording.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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?.match(self) {
var selfCopy = self
selfCopy.isKnown = true
selfCopy.knownIssueContext = context
return selfCopy.record(configuration: configuration)
}

Expand Down
20 changes: 19 additions & 1 deletion Sources/Testing/Issues/Issue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,27 @@ 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()`, if any.
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 { knownIssueContext != nil }

/// Initialize an issue instance with the specified details.
///
Expand Down
89 changes: 57 additions & 32 deletions Sources/Testing/Issues/KnownIssue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,71 @@
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
//

/// Combine an instance of ``KnownIssueMatcher`` with any previously-set one.
///
/// - Parameters:
/// - issueMatcher: A function to invoke when an issue occurs that is used to
/// determine if the issue is known to occur.
/// - matchCounter: The counter responsible for tracking the number of matches
/// found with `issueMatcher`.
///
/// - Returns: A new instance of ``Configuration`` or `nil` if there was no
/// current configuration set.
private func _combineIssueMatcher(_ issueMatcher: @escaping KnownIssueMatcher, matchesCountedBy matchCounter: Locked<Int>) -> KnownIssueMatcher {
let oldIssueMatcher = Issue.currentKnownIssueMatcher
return { issue in
if issueMatcher(issue) || true == oldIssueMatcher?(issue) {
matchCounter.increment()
return true
/// A type that represents an active `withKnownIssue()` call and any parent calls.
///
/// 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
/// 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<Int>

/// Create a new ``KnownIssueScope`` by combining a new issue matcher with
/// any already-active scope.
///
/// - Parameters:
/// - parent: The context that should be checked next if `issueMatcher`
/// fails to match an issue.
/// - issueMatcher: A function to invoke when an issue occurs that is used
/// to determine if the issue is known to occur.
/// - context: The context to be associated with issues matched by
/// `issueMatcher`.
/// - Returns: A new instance of ``KnownIssueScope``.
init(parent: KnownIssueScope?, issueMatcher: @escaping KnownIssueMatcher, context: Issue.KnownIssueContext) {
let matchCounter = Locked(rawValue: 0)
self.matchCounter = matchCounter
match = { issue in
let matchedContext = if issueMatcher(issue) {
context
} else {
parent?.match(issue)
}
if matchedContext != nil {
matchCounter.increment()
}
return matchedContext
}
return false
}

/// 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: KnownIssueScope?
}

/// Check if an error matches using an issue-matching function, and throw it if
/// it does not.
///
/// - Parameters:
/// - error: The error to test.
/// - issueMatcher: A function to which `error` is passed (after boxing it in
/// an instance of ``Issue``) to determine if it is known to occur.
/// - scope: 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
/// attributed.
private func _matchError(_ error: any Error, using issueMatcher: KnownIssueMatcher, comment: Comment?, sourceLocation: SourceLocation) throws {
private func _matchError(_ error: any Error, using scope: KnownIssueScope, comment: Comment?, sourceLocation: SourceLocation) throws {
let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error), sourceLocation: sourceLocation)
var issue = Issue(kind: .errorCaught(error), comments: Array(comment), sourceContext: sourceContext)
if issueMatcher(issue) {
if let context = scope.match(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
Expand Down Expand Up @@ -184,18 +211,17 @@ public func withKnownIssue(
guard precondition() else {
return try body()
}
let matchCounter = Locked(rawValue: 0)
let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment, sourceLocation: sourceLocation))
defer {
if !isIntermittent {
_handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
_handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation)
}
}
try Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
try KnownIssueScope.$current.withValue(scope) {
do {
try body()
} catch {
try _matchError(error, using: issueMatcher, comment: comment, sourceLocation: sourceLocation)
try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation)
}
}
}
Expand Down Expand Up @@ -304,18 +330,17 @@ public func withKnownIssue(
guard await precondition() else {
return try await body()
}
let matchCounter = Locked(rawValue: 0)
let issueMatcher = _combineIssueMatcher(issueMatcher, matchesCountedBy: matchCounter)
let scope = KnownIssueScope(parent: .current, issueMatcher: issueMatcher, context: Issue.KnownIssueContext(comment: comment, sourceLocation: sourceLocation))
defer {
if !isIntermittent {
_handleMiscount(by: matchCounter, comment: comment, sourceLocation: sourceLocation)
_handleMiscount(by: scope.matchCounter, comment: comment, sourceLocation: sourceLocation)
}
}
try await Issue.$currentKnownIssueMatcher.withValue(issueMatcher) {
try await KnownIssueScope.$current.withValue(scope) {
do {
try await body()
} catch {
try _matchError(error, using: issueMatcher, comment: comment, sourceLocation: sourceLocation)
try _matchError(error, using: scope, comment: comment, sourceLocation: sourceLocation)
}
}
}
Loading