From 3a61fd7dda765ff4b5775bbf84158de19f2379c7 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 7 Feb 2025 16:16:31 -0500 Subject: [PATCH 1/3] Add a (internal-only) `CustomIssueRepresentable` protocol. This PR adds a (for now internal-only) protocol that lets us transform arbitrary errors to arbitrary issues (rather than always emitting them as `.errorCaught`). This protocol replaces the special-casing we were doing for `SystemError` and `APIMisuseError`. This change also causes us to emit an API misuse issue if a developer passes an error of type `ExpectationFailedError` to `Issue.record()` as we can reasonably assume that such an error was already recorded correctly as `.expectationFailed`. --- Sources/Testing/CMakeLists.txt | 2 +- Sources/Testing/Issues/Issue+Recording.swift | 16 +--- .../Support/CustomIssueRepresentable.swift | 86 +++++++++++++++++++ Sources/Testing/Support/SystemError.swift | 36 -------- Tests/TestingTests/IssueTests.swift | 78 +++++++++++++++++ 5 files changed, 169 insertions(+), 49 deletions(-) create mode 100644 Sources/Testing/Support/CustomIssueRepresentable.swift delete mode 100644 Sources/Testing/Support/SystemError.swift diff --git a/Sources/Testing/CMakeLists.txt b/Sources/Testing/CMakeLists.txt index cd9983c07..16efd0ee6 100644 --- a/Sources/Testing/CMakeLists.txt +++ b/Sources/Testing/CMakeLists.txt @@ -73,6 +73,7 @@ add_library(Testing Support/Additions/WinSDKAdditions.swift Support/CartesianProduct.swift Support/CError.swift + Support/CustomIssueRepresentable.swift Support/Environment.swift Support/FileHandle.swift Support/GetSymbol.swift @@ -80,7 +81,6 @@ add_library(Testing Support/JSON.swift Support/Locked.swift Support/Locked+Platform.swift - Support/SystemError.swift Support/Versions.swift Discovery.swift Discovery+Platform.swift diff --git a/Sources/Testing/Issues/Issue+Recording.swift b/Sources/Testing/Issues/Issue+Recording.swift index 8a80e4467..aaf721c6a 100644 --- a/Sources/Testing/Issues/Issue+Recording.swift +++ b/Sources/Testing/Issues/Issue+Recording.swift @@ -27,19 +27,11 @@ extension Issue { /// - Returns: The issue that was recorded (`self` or a modified copy of it.) @discardableResult func record(configuration: Configuration? = nil) -> Self { - // If this issue is a caught error of kind SystemError, reinterpret it as a - // testing system issue instead (per the documentation for SystemError.) + // If this issue is a caught error that has a custom issue representation, + // perform that customization now. if case let .errorCaught(error) = kind { - // TODO: consider factoring this logic out into a protocol - if let error = error as? SystemError { - var selfCopy = self - selfCopy.kind = .system - selfCopy.comments.append(Comment(rawValue: String(describingForTest: error))) - return selfCopy.record(configuration: configuration) - } else if let error = error as? APIMisuseError { - var selfCopy = self - selfCopy.kind = .apiMisused - selfCopy.comments.append(Comment(rawValue: String(describingForTest: error))) + if let error = error as? any CustomIssueRepresentable { + let selfCopy = error.customize(self) return selfCopy.record(configuration: configuration) } } diff --git a/Sources/Testing/Support/CustomIssueRepresentable.swift b/Sources/Testing/Support/CustomIssueRepresentable.swift new file mode 100644 index 000000000..f4bef7740 --- /dev/null +++ b/Sources/Testing/Support/CustomIssueRepresentable.swift @@ -0,0 +1,86 @@ +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2024–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 +// + +/// A protocol that provides instances of conforming types with the ability to +/// record themselves as test issues. +/// +/// When a type conforms to this protocol, values of that type can be passed to +/// ``Issue/record(_:_:)``. The testing library then calls the +/// ``represent(as:)`` function and passes it an instance of ``Issue`` that will +/// be used to represent the value. The function can then reconfigure or replace +/// the issue as needed. +/// +/// This protocol may become part of the testing library's public interface in +/// the future. There's not really anything _requiring_ it to conform to `Error` +/// but all our current use cases are error types. If we want to allow other +/// types to be represented as issues, we will need to add an overload of +/// `Issue.record()` that takes `some CustomIssueRepresentable`. +protocol CustomIssueRepresentable: Error { + /// Customize the issue that will represent this value. + /// + /// - Parameters: + /// - issue: The issue to customize. The function consumes this value. + /// + /// - Returns: A customized copy of `issue`. + func customize(_ issue: consuming Issue) -> Issue +} + +// MARK: - + +/// A type representing an error in the testing library or its underlying +/// infrastructure. +/// +/// When an error of this type is thrown and caught by the testing library, it +/// is recorded as an issue of kind ``Issue/Kind/system`` rather than +/// ``Issue/Kind/errorCaught(_:)``. +/// +/// This type is not part of the public interface of the testing library. +/// External callers should generally record issues by throwing their own errors +/// or by calling ``Issue/record(_:sourceLocation:)``. +struct SystemError: Error, CustomStringConvertible, CustomIssueRepresentable { + var description: String + + func customize(_ issue: consuming Issue) -> Issue { + issue.kind = .system + issue.comments.append("\(self)") + return issue + } +} + +/// A type representing misuse of testing library API. +/// +/// When an error of this type is thrown and caught by the testing library, it +/// is recorded as an issue of kind ``Issue/Kind/apiMisused`` rather than +/// ``Issue/Kind/errorCaught(_:)``. +/// +/// This type is not part of the public interface of the testing library. +/// External callers should generally record issues by throwing their own errors +/// or by calling ``Issue/record(_:sourceLocation:)``. +struct APIMisuseError: Error, CustomStringConvertible, CustomIssueRepresentable { + var description: String + + func customize(_ issue: consuming Issue) -> Issue { + issue.kind = .apiMisused + issue.comments.append("\(self)") + return issue + } +} + +extension ExpectationFailedError: CustomIssueRepresentable { + func customize(_ issue: consuming Issue) -> Issue { + // Instances of this error type are thrown by `#require()` after an issue of + // kind `.expectationFailed` has already been recorded. Code that rethrows + // this error does not generate a new issue, but code that passes this error + // to Issue.record() is misbehaving. + issue.kind = .apiMisused + issue.comments.append("Recorded an error of type \(Self.self) representing an expectation that failed and was already recorded: \(expectation)") + return issue + } +} diff --git a/Sources/Testing/Support/SystemError.swift b/Sources/Testing/Support/SystemError.swift deleted file mode 100644 index d2d4809e3..000000000 --- a/Sources/Testing/Support/SystemError.swift +++ /dev/null @@ -1,36 +0,0 @@ -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2024 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 -// - -/// A type representing an error in the testing library or its underlying -/// infrastructure. -/// -/// When an error of this type is thrown and caught by the testing library, it -/// is recorded as an issue of kind ``Issue/Kind/system`` rather than -/// ``Issue/Kind/errorCaught(_:)``. -/// -/// This type is not part of the public interface of the testing library. -/// External callers should generally record issues by throwing their own errors -/// or by calling ``Issue/record(_:sourceLocation:)``. -struct SystemError: Error, CustomStringConvertible { - var description: String -} - -/// A type representing misuse of testing library API. -/// -/// When an error of this type is thrown and caught by the testing library, it -/// is recorded as an issue of kind ``Issue/Kind/apiMisused`` rather than -/// ``Issue/Kind/errorCaught(_:)``. -/// -/// This type is not part of the public interface of the testing library. -/// External callers should generally record issues by throwing their own errors -/// or by calling ``Issue/record(_:sourceLocation:)``. -struct APIMisuseError: Error, CustomStringConvertible { - var description: String -} diff --git a/Tests/TestingTests/IssueTests.swift b/Tests/TestingTests/IssueTests.swift index 4a4fda631..8b7d86358 100644 --- a/Tests/TestingTests/IssueTests.swift +++ b/Tests/TestingTests/IssueTests.swift @@ -1515,6 +1515,84 @@ final class IssueTests: XCTestCase { await fulfillment(of: [expectationFailed], timeout: 0.0) } + + func testThrowing(_ error: some Error, producesIssueMatching issueMatcher: @escaping @Sendable (Issue) -> Bool) async { + let issueRecorded = expectation(description: "Issue recorded") + + var configuration = Configuration() + configuration.eventHandler = { event, _ in + guard case let .issueRecorded(issue) = event.kind else { + return + } + if issueMatcher(issue) { + issueRecorded.fulfill() + let description = String(describing: error) + #expect(issue.comments.map(String.init(describing:)).contains(description)) + } else { + Issue.record("Unexpected issue \(issue)") + } + } + + await Test { + throw error + }.run(configuration: configuration) + + await fulfillment(of: [issueRecorded], timeout: 0.0) + } + + func testThrowingSystemErrorProducesSystemIssue() async { + await testThrowing( + SystemError(description: "flinging excuses"), + producesIssueMatching: { issue in + if case .system = issue.kind { + return true + } + return false + } + ) + } + + func testThrowingAPIMisuseErrorProducesAPIMisuseIssue() async { + await testThrowing( + APIMisuseError(description: "you did it wrong"), + producesIssueMatching: { issue in + if case .apiMisused = issue.kind { + return true + } + return false + } + ) + } + + func testRethrowingExpectationFailedErrorCausesAPIMisuseError() async { + let expectationFailed = expectation(description: "Expectation failed (issue recorded)") + let apiMisused = expectation(description: "API misused (issue recorded)") + + var configuration = Configuration() + configuration.eventHandler = { event, _ in + guard case let .issueRecorded(issue) = event.kind else { + return + } + switch issue.kind { + case .expectationFailed: + expectationFailed.fulfill() + case .apiMisused: + apiMisused.fulfill() + default: + Issue.record("Unexpected issue \(issue)") + } + } + + await Test { + do { + try #require(Bool(false)) + } catch { + Issue.record(error) + } + }.run(configuration: configuration) + + await fulfillment(of: [expectationFailed, apiMisused], timeout: 0.0) + } } #endif From f31cf6e1df7898356966a384ece9cf260b386827 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 11 Feb 2025 12:39:13 -0500 Subject: [PATCH 2/3] Update Sources/Testing/Support/CustomIssueRepresentable.swift Co-authored-by: Stuart Montgomery --- Sources/Testing/Support/CustomIssueRepresentable.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Support/CustomIssueRepresentable.swift b/Sources/Testing/Support/CustomIssueRepresentable.swift index f4bef7740..f726f3bfc 100644 --- a/Sources/Testing/Support/CustomIssueRepresentable.swift +++ b/Sources/Testing/Support/CustomIssueRepresentable.swift @@ -13,7 +13,7 @@ /// /// When a type conforms to this protocol, values of that type can be passed to /// ``Issue/record(_:_:)``. The testing library then calls the -/// ``represent(as:)`` function and passes it an instance of ``Issue`` that will +/// ``customize(_:)`` function and passes it an instance of ``Issue`` that will /// be used to represent the value. The function can then reconfigure or replace /// the issue as needed. /// From 60cda42ec87a0ddcee7ddfd350829579f066933c Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 11 Feb 2025 12:39:20 -0500 Subject: [PATCH 3/3] Update Sources/Testing/Support/CustomIssueRepresentable.swift Co-authored-by: Stuart Montgomery --- Sources/Testing/Support/CustomIssueRepresentable.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Support/CustomIssueRepresentable.swift b/Sources/Testing/Support/CustomIssueRepresentable.swift index f726f3bfc..d76739d1f 100644 --- a/Sources/Testing/Support/CustomIssueRepresentable.swift +++ b/Sources/Testing/Support/CustomIssueRepresentable.swift @@ -32,7 +32,7 @@ protocol CustomIssueRepresentable: Error { func customize(_ issue: consuming Issue) -> Issue } -// MARK: - +// MARK: - Internal error types /// A type representing an error in the testing library or its underlying /// infrastructure.