From 37f3afe67609375c8daeb65e8e3cfb402184176d Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 12 Sep 2024 12:06:16 -0500 Subject: [PATCH 1/7] Default 'comments:' to an empty array when creating an Issue --- Sources/Testing/Issues/Issue+Recording.swift | 14 ++++++-------- Sources/Testing/Issues/Issue.swift | 6 +++--- Sources/Testing/Running/Runner.Plan.swift | 4 ++-- Sources/Testing/Running/Runner.swift | 1 - Tests/TestingTests/EventRecorderTests.swift | 4 ++-- Tests/TestingTests/IssueTests.swift | 16 ++++------------ 6 files changed, 17 insertions(+), 28 deletions(-) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index 4578373a7..fcbca2b5c 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -21,8 +21,8 @@ extension Issue { /// /// - Parameters: /// - kind: The kind of issue. - /// - comments: An array of comments describing the issue. This array may be - /// empty. + /// - comments: An array of comments describing the issue. The default value + /// is an empty array. /// - backtrace: The backtrace of the issue, if available. This value is /// used to construct an instance of ``SourceContext``. /// - sourceLocation: The source location of the issue. This value is used @@ -32,7 +32,7 @@ extension Issue { /// /// - Returns: The issue that was recorded. @discardableResult - static func record(_ kind: Kind, comments: [Comment], backtrace: Backtrace?, sourceLocation: SourceLocation, configuration: Configuration? = nil) -> Self { + static func record(_ kind: Kind, comments: [Comment] = [], backtrace: Backtrace?, sourceLocation: SourceLocation, configuration: Configuration? = nil) -> Self { let sourceContext = SourceContext(backtrace: backtrace, sourceLocation: sourceLocation) return record(kind, comments: comments, sourceContext: sourceContext, configuration: configuration) } @@ -41,15 +41,15 @@ extension Issue { /// /// - Parameters: /// - kind: The kind of issue. - /// - comments: An array of comments describing the issue. This array may be - /// empty. + /// - comments: An array of comments describing the issue. The default value + /// is an empty array. /// - sourceContext: The source context of the issue. /// - configuration: The test configuration to use when recording the issue. /// The default value is ``Configuration/current``. /// /// - Returns: The issue that was recorded. @discardableResult - static func record(_ kind: Kind, comments: [Comment], sourceContext: SourceContext, configuration: Configuration? = nil) -> Self { + static func record(_ kind: Kind, comments: [Comment] = [], sourceContext: SourceContext, configuration: Configuration? = nil) -> Self { let issue = Issue(kind: kind, comments: comments, sourceContext: sourceContext) return issue.record(configuration: configuration) } @@ -178,7 +178,6 @@ extension Issue { } catch { Issue.record( .errorCaught(error), - comments: [], backtrace: Backtrace(forFirstThrowOf: error), sourceLocation: sourceLocation, configuration: configuration @@ -224,7 +223,6 @@ extension Issue { } catch { Issue.record( .errorCaught(error), - comments: [], backtrace: Backtrace(forFirstThrowOf: error), sourceLocation: sourceLocation, configuration: configuration diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 91602ef7c..628ae571e 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -105,15 +105,15 @@ public struct Issue: Sendable { /// /// - Parameters: /// - kind: The kind of issue this value represents. - /// - comments: An array of comments describing the issue. This array may be - /// empty. + /// - comments: An array of comments describing the issue. The default value + /// is an empty array. /// - sourceContext: A ``SourceContext`` indicating where and how this issue /// occurred. This defaults to a default source context returned by /// calling ``SourceContext/init(backtrace:sourceLocation:)`` with zero /// arguments. init( kind: Kind, - comments: [Comment], + comments: [Comment] = [], sourceContext: SourceContext = .init() ) { self.kind = kind diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift index ff57b2d88..1b2978398 100644 --- a/Sources/Testing/Running/Runner.Plan.swift +++ b/Sources/Testing/Running/Runner.Plan.swift @@ -242,7 +242,7 @@ extension Runner.Plan { // throw an error, then the action is to record an issue for that error. if case .run = action, let error = firstCaughtError { let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error)) - let issue = Issue(kind: .errorCaught(error), comments: [], sourceContext: sourceContext) + let issue = Issue(kind: .errorCaught(error), sourceContext: sourceContext) action = .recordIssue(issue) } @@ -258,7 +258,7 @@ extension Runner.Plan { try await test.evaluateTestCases() } catch { let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error)) - let issue = Issue(kind: .errorCaught(error), comments: [], sourceContext: sourceContext) + let issue = Issue(kind: .errorCaught(error), sourceContext: sourceContext) action = .recordIssue(issue) } } diff --git a/Sources/Testing/Running/Runner.swift b/Sources/Testing/Running/Runner.swift index 7642ad373..bb6252705 100644 --- a/Sources/Testing/Running/Runner.swift +++ b/Sources/Testing/Running/Runner.swift @@ -342,7 +342,6 @@ extension Runner { } timeoutHandler: { timeLimit in Issue.record( .timeLimitExceeded(timeLimitComponents: timeLimit), - comments: [], backtrace: .current(), sourceLocation: sourceLocation, configuration: configuration diff --git a/Tests/TestingTests/EventRecorderTests.swift b/Tests/TestingTests/EventRecorderTests.swift index a6e284a84..525e9d252 100644 --- a/Tests/TestingTests/EventRecorderTests.swift +++ b/Tests/TestingTests/EventRecorderTests.swift @@ -358,7 +358,7 @@ struct EventRecorderTests { @Test("HumanReadableOutputRecorder counts issues without associated tests") func humanReadableRecorderCountsIssuesWithoutTests() { - let issue = Issue(kind: .unconditional, comments: [], sourceContext: .init()) + let issue = Issue(kind: .unconditional) let event = Event(.issueRecorded(issue), testID: nil, testCaseID: nil) let context = Event.Context(test: nil, testCase: nil, configuration: nil) @@ -373,7 +373,7 @@ struct EventRecorderTests { @Test("JUnitXMLRecorder counts issues without associated tests") func junitRecorderCountsIssuesWithoutTests() async throws { - let issue = Issue(kind: .unconditional, comments: [], sourceContext: .init()) + let issue = Issue(kind: .unconditional) let event = Event(.issueRecorded(issue), testID: nil, testCaseID: nil) let context = Event.Context(test: nil, testCase: nil, configuration: nil) diff --git a/Tests/TestingTests/IssueTests.swift b/Tests/TestingTests/IssueTests.swift index e31041464..9f28641e2 100644 --- a/Tests/TestingTests/IssueTests.swift +++ b/Tests/TestingTests/IssueTests.swift @@ -1038,7 +1038,7 @@ final class IssueTests: XCTestCase { func testSetSourceLocationProperty() async throws { let sourceLocation = SourceLocation(fileID: "A/B", filePath: "", line: 12345, column: 1) - var issue = Issue(kind: .unconditional, comments: [], sourceContext: .init(sourceLocation: sourceLocation)) + var issue = Issue(kind: .unconditional, sourceContext: .init(sourceLocation: sourceLocation)) var issueSourceLocation = try XCTUnwrap(issue.sourceLocation) XCTAssertEqual(issueSourceLocation.line, 12345) @@ -1480,7 +1480,7 @@ struct IssueCodingTests { } @Test func errorSnapshot() throws { - let issue = Issue(kind: .errorCaught(NSError(domain: "Domain", code: 13)), comments: []) + let issue = Issue(kind: .errorCaught(NSError(domain: "Domain", code: 13))) let underlyingError = try #require(issue.error) let issueSnapshot = Issue.Snapshot(snapshotting: issue) @@ -1501,11 +1501,7 @@ struct IssueCodingTests { sourceLocation: sourceLocation ) - let issue = Issue( - kind: .apiMisused, - comments: [], - sourceContext: sourceContext - ) + let issue = Issue(kind: .apiMisused, sourceContext: sourceContext) let issueSnapshot = Issue.Snapshot(snapshotting: issue) #expect(issueSnapshot.sourceContext == sourceContext) @@ -1525,11 +1521,7 @@ struct IssueCodingTests { sourceLocation: initialSourceLocation ) - let issue = Issue( - kind: .apiMisused, - comments: [], - sourceContext: sourceContext - ) + let issue = Issue(kind: .apiMisused, sourceContext: sourceContext) let updatedSourceLocation = SourceLocation( fileID: "fileID2", From 07b4a3956b6410f025a216c881db4a8ded298ab3 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 12 Sep 2024 15:03:02 -0500 Subject: [PATCH 2/7] Take a different approach --- Sources/Testing/ExitTests/ExitTest.swift | 4 +- .../ExpectationChecking+Macro.swift | 4 +- Sources/Testing/Issues/Confirmation.swift | 8 +-- Sources/Testing/Issues/Issue+Recording.swift | 53 ++----------------- Sources/Testing/Issues/Issue.swift | 29 ++++++++-- Sources/Testing/Issues/KnownIssue.swift | 8 +-- Sources/Testing/Running/Runner.Plan.swift | 6 +-- Sources/Testing/Running/Runner.swift | 10 ++-- Sources/Testing/Test+Macro.swift | 8 +-- .../TestSupport/TestingAdditions.swift | 10 ++++ 10 files changed, 64 insertions(+), 76 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index 6ef1d8175..c54302b06 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -178,7 +178,9 @@ func callExitTest( // common issues, however they would constitute a failure of the test // infrastructure rather than the test itself and perhaps should not cause // the test to terminate early. - Issue.record(.errorCaught(error), comments: comments(), backtrace: .current(), sourceLocation: sourceLocation, configuration: configuration) + let issue = Issue(kind: .errorCaught(error), comments: comments(), sourceContext: .init(sourceLocation: sourceLocation)) + issue.record(configuration: configuration) + return __checkValue( false, expression: expression, diff --git a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift index 3e433cb1a..60556a39c 100644 --- a/Sources/Testing/Expectations/ExpectationChecking+Macro.swift +++ b/Sources/Testing/Expectations/ExpectationChecking+Macro.swift @@ -114,7 +114,9 @@ public func __checkValue( // Ensure the backtrace is captured here so it has fewer extraneous frames // from the testing framework which aren't relevant to the user. let backtrace = Backtrace.current() - Issue.record(.expectationFailed(expectation), comments: comments(), backtrace: backtrace, sourceLocation: sourceLocation) + let issue = Issue(kind: .expectationFailed(expectation), comments: comments(), sourceContext: .init(backtrace: backtrace, sourceLocation: sourceLocation)) + issue.record() + return .failure(ExpectationFailedError(expectation: expectation)) } diff --git a/Sources/Testing/Issues/Confirmation.swift b/Sources/Testing/Issues/Confirmation.swift index 5848dfb50..706437ed4 100644 --- a/Sources/Testing/Issues/Confirmation.swift +++ b/Sources/Testing/Issues/Confirmation.swift @@ -173,12 +173,12 @@ public func confirmation( defer { let actualCount = confirmation.count.rawValue if !expectedCount.contains(actualCount) { - Issue.record( - expectedCount.issueKind(forActualCount: actualCount), + let issue = Issue( + kind: expectedCount.issueKind(forActualCount: actualCount), comments: Array(comment), - backtrace: .current(), - sourceLocation: sourceLocation + sourceContext: .init(sourceLocation: sourceLocation) ) + issue.record() } } return try await body(confirmation) diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index fcbca2b5c..aeb4a1580 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -17,43 +17,6 @@ extension Issue { @TaskLocal static var currentKnownIssueMatcher: KnownIssueMatcher? - /// Record a new issue with the specified properties. - /// - /// - Parameters: - /// - kind: The kind of issue. - /// - comments: An array of comments describing the issue. The default value - /// is an empty array. - /// - backtrace: The backtrace of the issue, if available. This value is - /// used to construct an instance of ``SourceContext``. - /// - sourceLocation: The source location of the issue. This value is used - /// to construct an instance of ``SourceContext``. - /// - configuration: The test configuration to use when recording the issue. - /// The default value is ``Configuration/current``. - /// - /// - Returns: The issue that was recorded. - @discardableResult - static func record(_ kind: Kind, comments: [Comment] = [], backtrace: Backtrace?, sourceLocation: SourceLocation, configuration: Configuration? = nil) -> Self { - let sourceContext = SourceContext(backtrace: backtrace, sourceLocation: sourceLocation) - return record(kind, comments: comments, sourceContext: sourceContext, configuration: configuration) - } - - /// Record a new issue with the specified properties. - /// - /// - Parameters: - /// - kind: The kind of issue. - /// - comments: An array of comments describing the issue. The default value - /// is an empty array. - /// - sourceContext: The source context of the issue. - /// - configuration: The test configuration to use when recording the issue. - /// The default value is ``Configuration/current``. - /// - /// - Returns: The issue that was recorded. - @discardableResult - static func record(_ kind: Kind, comments: [Comment] = [], sourceContext: SourceContext, configuration: Configuration? = nil) -> Self { - let issue = Issue(kind: kind, comments: comments, sourceContext: sourceContext) - return issue.record(configuration: configuration) - } - /// Record this issue by wrapping it in an ``Event`` and passing it to the /// current event handler. /// @@ -176,12 +139,8 @@ extension Issue { // condition evaluated to `false`. Those functions record their own issue, // so we don't need to record another one redundantly. } catch { - Issue.record( - .errorCaught(error), - backtrace: Backtrace(forFirstThrowOf: error), - sourceLocation: sourceLocation, - configuration: configuration - ) + let issue = Issue(forCaughtError: error, sourceLocation: sourceLocation) + issue.record(configuration: configuration) return error } @@ -221,12 +180,8 @@ extension Issue { // condition evaluated to `false`. Those functions record their own issue, // so we don't need to record another one redundantly. } catch { - Issue.record( - .errorCaught(error), - backtrace: Backtrace(forFirstThrowOf: error), - sourceLocation: sourceLocation, - configuration: configuration - ) + let issue = Issue(forCaughtError: error, sourceLocation: sourceLocation) + issue.record(configuration: configuration) return error } diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 628ae571e..7ac7bbff3 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -105,22 +105,43 @@ public struct Issue: Sendable { /// /// - Parameters: /// - kind: The kind of issue this value represents. - /// - comments: An array of comments describing the issue. The default value - /// is an empty array. + /// - comments: An array of comments describing the issue. This array may be + /// empty. /// - sourceContext: A ``SourceContext`` indicating where and how this issue /// occurred. This defaults to a default source context returned by /// calling ``SourceContext/init(backtrace:sourceLocation:)`` with zero /// arguments. init( kind: Kind, - comments: [Comment] = [], - sourceContext: SourceContext = .init() + comments: [Comment], + sourceContext: SourceContext ) { self.kind = kind self.comments = comments self.sourceContext = sourceContext } + /// Initialize an issue instance representing a caught error. + /// + /// - Parameters: + /// - error: The error which was caught which this issue is describing. + /// - sourceLocation: The source location of the issue. This value is used + /// to construct an instance of ``SourceContext``. + /// + /// The ``sourceContext`` property will have a ``SourceContext/backtrace`` + /// property whose value is the backtrace for the first throw of `error`. + init( + forCaughtError error: any Error, + sourceLocation: SourceLocation? = nil + ) { + let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error), sourceLocation: sourceLocation) + self.init( + kind: .errorCaught(error), + comments: [], + sourceContext: sourceContext + ) + } + /// The error which was associated with this issue, if any. /// /// The value of this property is non-`nil` when ``kind-swift.property`` is diff --git a/Sources/Testing/Issues/KnownIssue.swift b/Sources/Testing/Issues/KnownIssue.swift index 70c9c3875..4d7c16739 100644 --- a/Sources/Testing/Issues/KnownIssue.swift +++ b/Sources/Testing/Issues/KnownIssue.swift @@ -65,12 +65,12 @@ private func _matchError(_ error: any Error, using issueMatcher: KnownIssueMatch /// attributed. private func _handleMiscount(by matchCounter: Locked, comment: Comment?, sourceLocation: SourceLocation) { if matchCounter.rawValue == 0 { - Issue.record( - .knownIssueNotRecorded, + let issue = Issue( + kind: .knownIssueNotRecorded, comments: Array(comment), - backtrace: nil, - sourceLocation: sourceLocation + sourceContext: .init(backtrace: nil, sourceLocation: sourceLocation) ) + issue.record() } } diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift index 1b2978398..f9c8d6f25 100644 --- a/Sources/Testing/Running/Runner.Plan.swift +++ b/Sources/Testing/Running/Runner.Plan.swift @@ -241,8 +241,7 @@ extension Runner.Plan { // If no trait specified that the test should be skipped, but one did // throw an error, then the action is to record an issue for that error. if case .run = action, let error = firstCaughtError { - let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error)) - let issue = Issue(kind: .errorCaught(error), sourceContext: sourceContext) + let issue = Issue(forCaughtError: error) action = .recordIssue(issue) } @@ -257,8 +256,7 @@ extension Runner.Plan { do { try await test.evaluateTestCases() } catch { - let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error)) - let issue = Issue(kind: .errorCaught(error), sourceContext: sourceContext) + let issue = Issue(forCaughtError: error) action = .recordIssue(issue) } } diff --git a/Sources/Testing/Running/Runner.swift b/Sources/Testing/Running/Runner.swift index bb6252705..d73756c73 100644 --- a/Sources/Testing/Running/Runner.swift +++ b/Sources/Testing/Running/Runner.swift @@ -340,12 +340,12 @@ extension Runner { try await testCase.body() } } timeoutHandler: { timeLimit in - Issue.record( - .timeLimitExceeded(timeLimitComponents: timeLimit), - backtrace: .current(), - sourceLocation: sourceLocation, - configuration: configuration + let issue = Issue( + kind: .timeLimitExceeded(timeLimitComponents: timeLimit), + comments: [], + sourceContext: .init(sourceLocation: sourceLocation) ) + issue.record(configuration: configuration) } } } diff --git a/Sources/Testing/Test+Macro.swift b/Sources/Testing/Test+Macro.swift index 7fc2cf0eb..50afa755d 100644 --- a/Sources/Testing/Test+Macro.swift +++ b/Sources/Testing/Test+Macro.swift @@ -602,11 +602,11 @@ public func __invokeXCTestCaseMethod( guard let xcTestCaseClass, isClass(xcTestCaseSubclass, subclassOf: xcTestCaseClass) else { return false } - Issue.record( - .apiMisused, + let issue = Issue( + kind: .apiMisused, comments: ["The @Test attribute cannot be applied to methods on a subclass of XCTestCase."], - backtrace: nil, - sourceLocation: sourceLocation + sourceContext: .init(backtrace: nil, sourceLocation: sourceLocation) ) + issue.record() return true } diff --git a/Tests/TestingTests/TestSupport/TestingAdditions.swift b/Tests/TestingTests/TestSupport/TestingAdditions.swift index 05dbb6302..668056b84 100644 --- a/Tests/TestingTests/TestSupport/TestingAdditions.swift +++ b/Tests/TestingTests/TestSupport/TestingAdditions.swift @@ -366,3 +366,13 @@ extension Trait where Self == TimeLimitTrait { return Self(timeLimit: timeLimit) } } + +extension Issue { + init(kind: Kind) { + self.init(kind: kind, sourceContext: .init()) + } + + init(kind: Kind, sourceContext: SourceContext) { + self.init(kind: kind, comments: [], sourceContext: sourceContext) + } +} From 92441cda2e07c6f913795075a6370307e0abf1a5 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 12 Sep 2024 15:51:08 -0500 Subject: [PATCH 3/7] Ensure backtraces are captured eagerly to avoid unnecessary additional frames --- Sources/Testing/ExitTests/ExitTest.swift | 2 +- Sources/Testing/Issues/Confirmation.swift | 2 +- Sources/Testing/Issues/Issue+Recording.swift | 4 ++-- Sources/Testing/Issues/Issue.swift | 2 +- Sources/Testing/Running/Runner.Plan.swift | 6 ++---- Sources/Testing/Running/Runner.swift | 2 +- 6 files changed, 8 insertions(+), 10 deletions(-) diff --git a/Sources/Testing/ExitTests/ExitTest.swift b/Sources/Testing/ExitTests/ExitTest.swift index c54302b06..d0e98c65b 100644 --- a/Sources/Testing/ExitTests/ExitTest.swift +++ b/Sources/Testing/ExitTests/ExitTest.swift @@ -178,7 +178,7 @@ func callExitTest( // common issues, however they would constitute a failure of the test // infrastructure rather than the test itself and perhaps should not cause // the test to terminate early. - let issue = Issue(kind: .errorCaught(error), comments: comments(), sourceContext: .init(sourceLocation: sourceLocation)) + let issue = Issue(kind: .errorCaught(error), comments: comments(), sourceContext: .init(backtrace: .current(), sourceLocation: sourceLocation)) issue.record(configuration: configuration) return __checkValue( diff --git a/Sources/Testing/Issues/Confirmation.swift b/Sources/Testing/Issues/Confirmation.swift index 706437ed4..82f140a54 100644 --- a/Sources/Testing/Issues/Confirmation.swift +++ b/Sources/Testing/Issues/Confirmation.swift @@ -176,7 +176,7 @@ public func confirmation( let issue = Issue( kind: expectedCount.issueKind(forActualCount: actualCount), comments: Array(comment), - sourceContext: .init(sourceLocation: sourceLocation) + sourceContext: .init(backtrace: .current(), sourceLocation: sourceLocation) ) issue.record() } diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index aeb4a1580..2a6facefb 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -139,7 +139,7 @@ extension Issue { // condition evaluated to `false`. Those functions record their own issue, // so we don't need to record another one redundantly. } catch { - let issue = Issue(forCaughtError: error, sourceLocation: sourceLocation) + let issue = Issue(for: error, sourceLocation: sourceLocation) issue.record(configuration: configuration) return error } @@ -180,7 +180,7 @@ extension Issue { // condition evaluated to `false`. Those functions record their own issue, // so we don't need to record another one redundantly. } catch { - let issue = Issue(forCaughtError: error, sourceLocation: sourceLocation) + let issue = Issue(for: error, sourceLocation: sourceLocation) issue.record(configuration: configuration) return error } diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 7ac7bbff3..08f6abc15 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -131,7 +131,7 @@ public struct Issue: Sendable { /// The ``sourceContext`` property will have a ``SourceContext/backtrace`` /// property whose value is the backtrace for the first throw of `error`. init( - forCaughtError error: any Error, + for error: any Error, sourceLocation: SourceLocation? = nil ) { let sourceContext = SourceContext(backtrace: Backtrace(forFirstThrowOf: error), sourceLocation: sourceLocation) diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift index f9c8d6f25..93e43af5a 100644 --- a/Sources/Testing/Running/Runner.Plan.swift +++ b/Sources/Testing/Running/Runner.Plan.swift @@ -241,8 +241,7 @@ extension Runner.Plan { // If no trait specified that the test should be skipped, but one did // throw an error, then the action is to record an issue for that error. if case .run = action, let error = firstCaughtError { - let issue = Issue(forCaughtError: error) - action = .recordIssue(issue) + action = .recordIssue(Issue(for: error)) } // If the test is still planned to run (i.e. nothing thus far has caused @@ -256,8 +255,7 @@ extension Runner.Plan { do { try await test.evaluateTestCases() } catch { - let issue = Issue(forCaughtError: error) - action = .recordIssue(issue) + action = .recordIssue(Issue(for: error)) } } diff --git a/Sources/Testing/Running/Runner.swift b/Sources/Testing/Running/Runner.swift index d73756c73..954485339 100644 --- a/Sources/Testing/Running/Runner.swift +++ b/Sources/Testing/Running/Runner.swift @@ -343,7 +343,7 @@ extension Runner { let issue = Issue( kind: .timeLimitExceeded(timeLimitComponents: timeLimit), comments: [], - sourceContext: .init(sourceLocation: sourceLocation) + sourceContext: .init(backtrace: .current(), sourceLocation: sourceLocation) ) issue.record(configuration: configuration) } From 91573379dbfd7548984e6cf086cd3dc8dc9f7d1b Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 12 Sep 2024 16:17:02 -0500 Subject: [PATCH 4/7] Change SourceContext to no longer default the backtrace to .current(). Instead, require callers pass it explicitly --- Sources/Testing/Issues/Issue.swift | 4 +--- Sources/Testing/Running/Runner.Plan.swift | 4 ++-- Sources/Testing/Running/SkipInfo.swift | 5 +++-- Sources/Testing/SourceAttribution/SourceContext.swift | 9 ++++----- Sources/Testing/Traits/ConditionTrait.swift | 7 ++++++- Tests/TestingTests/RunnerTests.swift | 3 ++- Tests/TestingTests/TestSupport/TestingAdditions.swift | 10 ++++++++++ 7 files changed, 28 insertions(+), 14 deletions(-) diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 08f6abc15..bb3b6f2fe 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -108,9 +108,7 @@ public struct Issue: Sendable { /// - comments: An array of comments describing the issue. This array may be /// empty. /// - sourceContext: A ``SourceContext`` indicating where and how this issue - /// occurred. This defaults to a default source context returned by - /// calling ``SourceContext/init(backtrace:sourceLocation:)`` with zero - /// arguments. + /// occurred. init( kind: Kind, comments: [Comment], diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift index 93e43af5a..72afef804 100644 --- a/Sources/Testing/Running/Runner.Plan.swift +++ b/Sources/Testing/Running/Runner.Plan.swift @@ -41,7 +41,7 @@ extension Runner { /// /// - Parameters: /// - skipInfo: A ``SkipInfo`` representing the details of this skip. - indirect case skip(_ skipInfo: SkipInfo = .init()) + indirect case skip(_ skipInfo: SkipInfo) /// The test should record an issue due to a failure during /// planning. @@ -261,7 +261,7 @@ extension Runner.Plan { // If the test is parameterized but has no cases, mark it as skipped. if case .run = action, let testCases = test.testCases, testCases.first(where: { _ in true }) == nil { - action = .skip(SkipInfo(comment: "No test cases found.")) + action = .skip(SkipInfo(comment: "No test cases found.", sourceContext: .init(backtrace: nil, sourceLocation: test.sourceLocation))) } actionGraph.updateValue(action, at: keyPath) diff --git a/Sources/Testing/Running/SkipInfo.swift b/Sources/Testing/Running/SkipInfo.swift index 13bbd67dc..b54aaf656 100644 --- a/Sources/Testing/Running/SkipInfo.swift +++ b/Sources/Testing/Running/SkipInfo.swift @@ -34,10 +34,11 @@ public struct SkipInfo: Sendable { /// Defaults to `nil`. /// - sourceContext: A source context indicating where this skip occurred. /// Defaults to a source context returned by calling - /// ``SourceContext/init(backtrace:sourceLocation:)`` with zero arguments. + /// ``SourceContext/init(backtrace:sourceLocation:)`` passing only the + /// current backtrace. public init( comment: Comment? = nil, - sourceContext: SourceContext = .init() + sourceContext: SourceContext = .init(backtrace: .current()) ) { self.comment = comment self.sourceContext = sourceContext diff --git a/Sources/Testing/SourceAttribution/SourceContext.swift b/Sources/Testing/SourceAttribution/SourceContext.swift index dd56dc14c..df3c5a386 100644 --- a/Sources/Testing/SourceAttribution/SourceContext.swift +++ b/Sources/Testing/SourceAttribution/SourceContext.swift @@ -26,11 +26,10 @@ public struct SourceContext: Sendable { /// source location. /// /// - Parameters: - /// - backtrace: The backtrace associated with the new instance. Defaults to - /// the current backtrace (obtained via - /// ``Backtrace/current(maximumAddressCount:)``). - /// - sourceLocation: The source location associated with the new instance. - public init(backtrace: Backtrace? = .current(), sourceLocation: SourceLocation? = nil) { + /// - backtrace: The backtrace associated with the new instance. + /// - sourceLocation: The source location associated with the new instance, + /// if available. + public init(backtrace: Backtrace?, sourceLocation: SourceLocation? = nil) { self.backtrace = backtrace self.sourceLocation = sourceLocation } diff --git a/Sources/Testing/Traits/ConditionTrait.swift b/Sources/Testing/Traits/ConditionTrait.swift index 55d2b0b60..09c8909dc 100644 --- a/Sources/Testing/Traits/ConditionTrait.swift +++ b/Sources/Testing/Traits/ConditionTrait.swift @@ -92,7 +92,12 @@ public struct ConditionTrait: TestTrait, SuiteTrait { } if !result { - let sourceContext = SourceContext(sourceLocation: sourceLocation) + // We don't need to consider including a backtrace here because it will + // primarily contain frames in the testing library, not user code. If an + // error was thrown by a condition evaluated above, the caller _should_ + // attempt to get the backtrace of the caught error when creating an issue + // for it, however. + let sourceContext = SourceContext(backtrace: nil, sourceLocation: sourceLocation) throw SkipInfo(comment: commentOverride ?? comments.first, sourceContext: sourceContext) } } diff --git a/Tests/TestingTests/RunnerTests.swift b/Tests/TestingTests/RunnerTests.swift index 7aacc2f61..bbc88949c 100644 --- a/Tests/TestingTests/RunnerTests.swift +++ b/Tests/TestingTests/RunnerTests.swift @@ -405,8 +405,9 @@ final class RunnerTests: XCTestCase { testFunction(named: "succeedsAsync()", in: SendableTests.self), testFunction(named: "succeeds()", in: SendableTests.NestedSendableTests.self), ].map { try XCTUnwrap($0) } + let skipInfo = SkipInfo(sourceContext: .init(backtrace: nil)) let steps: [Runner.Plan.Step] = tests - .map { .init(test: $0, action: .skip()) } + .map { .init(test: $0, action: .skip(skipInfo)) } let plan = Runner.Plan(steps: steps) let testStarted = expectation(description: "Test was skipped") diff --git a/Tests/TestingTests/TestSupport/TestingAdditions.swift b/Tests/TestingTests/TestSupport/TestingAdditions.swift index 668056b84..3d820900d 100644 --- a/Tests/TestingTests/TestSupport/TestingAdditions.swift +++ b/Tests/TestingTests/TestSupport/TestingAdditions.swift @@ -376,3 +376,13 @@ extension Issue { self.init(kind: kind, comments: [], sourceContext: sourceContext) } } + +extension SourceContext { + init() { + self.init(sourceLocation: nil) + } + + init(sourceLocation: SourceLocation?) { + self.init(backtrace: .current(), sourceLocation: sourceLocation) + } +} From ce3fb0588cc65efd187e2ef34b1b9f56d3ddb1ec Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 12 Sep 2024 16:54:04 -0500 Subject: [PATCH 5/7] Require 'sourceLocation:' be passed to SourceContext explicitly, too --- Sources/Testing/Running/Runner.Plan.swift | 9 +++++++++ Sources/Testing/Running/SkipInfo.swift | 11 ++++++++++- .../SourceAttribution/SourceContext.swift | 16 +++++++++++++++- Tests/TestingTests/SkipInfoTests.swift | 2 +- .../TestSupport/TestingAdditions.swift | 4 ++++ 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift index 72afef804..6d87443a3 100644 --- a/Sources/Testing/Running/Runner.Plan.swift +++ b/Sources/Testing/Running/Runner.Plan.swift @@ -433,3 +433,12 @@ extension Runner.Plan.Action { } } #endif + +// MARK: - Deprecated + +extension Runner.Plan.Action { + @available(*, deprecated, message: "Use .skip(_:) and pass a SkipInfo explicitly.") + public static func skip() -> Self { + .skip(SkipInfo()) + } +} diff --git a/Sources/Testing/Running/SkipInfo.swift b/Sources/Testing/Running/SkipInfo.swift index b54aaf656..8a512ddcf 100644 --- a/Sources/Testing/Running/SkipInfo.swift +++ b/Sources/Testing/Running/SkipInfo.swift @@ -38,7 +38,7 @@ public struct SkipInfo: Sendable { /// current backtrace. public init( comment: Comment? = nil, - sourceContext: SourceContext = .init(backtrace: .current()) + sourceContext: SourceContext ) { self.comment = comment self.sourceContext = sourceContext @@ -56,3 +56,12 @@ extension SkipInfo: Equatable, Hashable {} // MARK: - Codable extension SkipInfo: Codable {} + +// MARK: - Deprecated + +extension SkipInfo { + @available(*, deprecated, message: "Use init(comment:sourceContext:) and pass an explicit SourceContext.") + public init(comment: Comment? = nil) { + self.init(comment: comment, sourceContext: .init(backtrace: .current(), sourceLocation: nil)) + } +} diff --git a/Sources/Testing/SourceAttribution/SourceContext.swift b/Sources/Testing/SourceAttribution/SourceContext.swift index df3c5a386..8e6ff6b96 100644 --- a/Sources/Testing/SourceAttribution/SourceContext.swift +++ b/Sources/Testing/SourceAttribution/SourceContext.swift @@ -29,7 +29,7 @@ public struct SourceContext: Sendable { /// - backtrace: The backtrace associated with the new instance. /// - sourceLocation: The source location associated with the new instance, /// if available. - public init(backtrace: Backtrace?, sourceLocation: SourceLocation? = nil) { + public init(backtrace: Backtrace?, sourceLocation: SourceLocation?) { self.backtrace = backtrace self.sourceLocation = sourceLocation } @@ -40,3 +40,17 @@ extension SourceContext: Equatable, Hashable {} // MARK: - Codable extension SourceContext: Codable {} + +// MARK: - Deprecated + +extension SourceContext { + @available(*, deprecated, message: "Use init(backtrace:sourceLocation:) and pass both arguments explicitly instead.") + public init(backtrace: Backtrace?) { + self.init(backtrace: backtrace, sourceLocation: nil) + } + + @available(*, deprecated, message: "Use init(backtrace:sourceLocation:) and pass both arguments explicitly instead.") + public init(sourceLocation: SourceLocation? = nil) { + self.init(backtrace: nil, sourceLocation: sourceLocation) + } +} diff --git a/Tests/TestingTests/SkipInfoTests.swift b/Tests/TestingTests/SkipInfoTests.swift index 2bfa481b6..b89b899b6 100644 --- a/Tests/TestingTests/SkipInfoTests.swift +++ b/Tests/TestingTests/SkipInfoTests.swift @@ -13,7 +13,7 @@ @Suite("SkipInfo Tests") struct SkipInfoTests { @Test("comment property") func comment() { - var skipInfo = SkipInfo(comment: "abc123") + var skipInfo = SkipInfo(comment: "abc123", sourceContext: .init()) #expect(skipInfo.comment == "abc123") skipInfo.comment = .__line("// Foo") #expect(skipInfo.comment == .__line("// Foo")) diff --git a/Tests/TestingTests/TestSupport/TestingAdditions.swift b/Tests/TestingTests/TestSupport/TestingAdditions.swift index 3d820900d..f69d0f083 100644 --- a/Tests/TestingTests/TestSupport/TestingAdditions.swift +++ b/Tests/TestingTests/TestSupport/TestingAdditions.swift @@ -382,6 +382,10 @@ extension SourceContext { self.init(sourceLocation: nil) } + init(backtrace: Backtrace?) { + self.init(backtrace: backtrace, sourceLocation: nil) + } + init(sourceLocation: SourceLocation?) { self.init(backtrace: .current(), sourceLocation: sourceLocation) } From b9f6b8503b88509503d139327eb53161d6d785c5 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 12 Sep 2024 17:04:19 -0500 Subject: [PATCH 6/7] Fix DocC --- Sources/Testing/Running/SkipInfo.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/Sources/Testing/Running/SkipInfo.swift b/Sources/Testing/Running/SkipInfo.swift index 8a512ddcf..0c5a6923d 100644 --- a/Sources/Testing/Running/SkipInfo.swift +++ b/Sources/Testing/Running/SkipInfo.swift @@ -33,9 +33,6 @@ public struct SkipInfo: Sendable { /// - comment: A user-specified comment describing this skip, if any. /// Defaults to `nil`. /// - sourceContext: A source context indicating where this skip occurred. - /// Defaults to a source context returned by calling - /// ``SourceContext/init(backtrace:sourceLocation:)`` passing only the - /// current backtrace. public init( comment: Comment? = nil, sourceContext: SourceContext From f15aa15b69f39ab6d70745a4d03bf67c900b194a Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Fri, 13 Sep 2024 09:57:38 -0500 Subject: [PATCH 7/7] Consolidate two testing addition overloads --- Tests/TestingTests/TestSupport/TestingAdditions.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Tests/TestingTests/TestSupport/TestingAdditions.swift b/Tests/TestingTests/TestSupport/TestingAdditions.swift index f69d0f083..0f0d4641a 100644 --- a/Tests/TestingTests/TestSupport/TestingAdditions.swift +++ b/Tests/TestingTests/TestSupport/TestingAdditions.swift @@ -368,11 +368,7 @@ extension Trait where Self == TimeLimitTrait { } extension Issue { - init(kind: Kind) { - self.init(kind: kind, sourceContext: .init()) - } - - init(kind: Kind, sourceContext: SourceContext) { + init(kind: Kind, sourceContext: SourceContext = .init()) { self.init(kind: kind, comments: [], sourceContext: sourceContext) } }