From c7cc45c8764efc941e539dd22cef5aa88a7ff85a Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Mon, 17 Feb 2025 08:22:47 -0600 Subject: [PATCH 1/2] Introduce issue handling trait (as SPI) Resolves rdar://140144041 --- Sources/Testing/CMakeLists.txt | 1 + Sources/Testing/Running/Runner.swift | 58 +++++- Sources/Testing/Testing.docc/Traits.md | 8 + .../Testing/Traits/IssueHandlingTrait.swift | 167 +++++++++++++++ .../Traits/IssueHandlingTraitTests.swift | 197 ++++++++++++++++++ 5 files changed, 426 insertions(+), 5 deletions(-) create mode 100644 Sources/Testing/Traits/IssueHandlingTrait.swift create mode 100644 Tests/TestingTests/Traits/IssueHandlingTraitTests.swift diff --git a/Sources/Testing/CMakeLists.txt b/Sources/Testing/CMakeLists.txt index b4e865427..6eca45dde 100644 --- a/Sources/Testing/CMakeLists.txt +++ b/Sources/Testing/CMakeLists.txt @@ -97,6 +97,7 @@ add_library(Testing Traits/ConditionTrait.swift Traits/ConditionTrait+Macro.swift Traits/HiddenTrait.swift + Traits/IssueHandlingTrait.swift Traits/ParallelizationTrait.swift Traits/Tags/Tag.Color.swift Traits/Tags/Tag.Color+Loading.swift diff --git a/Sources/Testing/Running/Runner.swift b/Sources/Testing/Running/Runner.swift index 16eff103f..8520d1aaf 100644 --- a/Sources/Testing/Running/Runner.swift +++ b/Sources/Testing/Running/Runner.swift @@ -91,10 +91,11 @@ extension Runner { return try await body() } - // Construct a recursive function that invokes each trait's ``execute(_:for:testCase:)`` - // function. The order of the sequence is reversed so that the last trait is - // the one that invokes body, then the second-to-last invokes the last, etc. - // and ultimately the first trait is the first one to be invoked. + // Construct a recursive function that invokes each scope provider's + // `provideScope(for:testCase:performing:)` function. The order of the + // sequence is reversed so that the last trait is the one that invokes body, + // then the second-to-last invokes the last, etc. and ultimately the first + // trait is the first one to be invoked. let executeAllTraits = test.traits.lazy .reversed() .compactMap { $0.scopeProvider(for: test, testCase: testCase) } @@ -108,6 +109,41 @@ extension Runner { try await executeAllTraits() } + /// Apply the custom scope from any issue handling traits for the specified + /// test. + /// + /// - Parameters: + /// - test: The test being run, for which to apply its issue handling traits. + /// - body: A function to execute within the scope provided by the test's + /// issue handling traits. + /// + /// - Throws: Whatever is thrown by `body` or by any of the traits' provide + /// scope function calls. + private static func _applyIssueHandlingTraits(for test: Test, _ body: @escaping @Sendable () async throws -> Void) async throws { + // If the test does not have any traits, exit early to avoid unnecessary + // heap allocations below. + if test.traits.isEmpty { + return try await body() + } + + // Construct a recursive function that invokes each issue handling trait's + // `provideScope(performing:)` function. The order of the sequence is + // reversed so that the last trait is the one that invokes body, then the + // second-to-last invokes the last, etc. and ultimately the first trait is + // the first one to be invoked. + let executeAllTraits = test.traits.lazy + .compactMap { $0 as? IssueHandlingTrait } + .reversed() + .map { $0.provideScope(performing:) } + .reduce(body) { executeAllTraits, provideScope in + { + try await provideScope(executeAllTraits) + } + } + + try await executeAllTraits() + } + /// Enumerate the elements of a sequence, parallelizing enumeration in a task /// group if a given plan step has parallelization enabled. /// @@ -177,7 +213,19 @@ extension Runner { Event.post(.testSkipped(skipInfo), for: (step.test, nil), configuration: configuration) shouldSendTestEnded = false case let .recordIssue(issue): - Event.post(.issueRecorded(issue), for: (step.test, nil), configuration: configuration) + // Scope posting the issue recorded event such that issue handling + // traits have the opportunity to handle it. This ensures that if a test + // has an issue handling trait _and_ some other trait which caused an + // issue to be recorded, the issue handling trait can process the issue + // even though it wasn't recorded by the test function. + try await Test.withCurrent(step.test) { + try await _applyIssueHandlingTraits(for: step.test) { + // Don't specify `configuration` when posting this issue so that + // traits can provide scope and potentially customize the + // configuration. + Event.post(.issueRecorded(issue), for: (step.test, nil)) + } + } shouldSendTestEnded = false } } else { diff --git a/Sources/Testing/Testing.docc/Traits.md b/Sources/Testing/Testing.docc/Traits.md index 413b4327c..e6d19c1b9 100644 --- a/Sources/Testing/Testing.docc/Traits.md +++ b/Sources/Testing/Testing.docc/Traits.md @@ -48,6 +48,13 @@ types that customize the behavior of your tests. - ``Trait/bug(_:id:_:)-10yf5`` - ``Trait/bug(_:id:_:)-3vtpl`` + + ### Creating custom traits - ``Trait`` @@ -64,3 +71,4 @@ types that customize the behavior of your tests. - ``Tag`` - ``Tag/List`` - ``TimeLimitTrait`` + diff --git a/Sources/Testing/Traits/IssueHandlingTrait.swift b/Sources/Testing/Traits/IssueHandlingTrait.swift new file mode 100644 index 000000000..5063d1571 --- /dev/null +++ b/Sources/Testing/Traits/IssueHandlingTrait.swift @@ -0,0 +1,167 @@ +// +// 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 +// + +/// A type that allows transforming or filtering the issues recorded by a test. +/// +/// Use this type to observe or customize the issue(s) recorded by the test this +/// trait is applied to. You can transform a recorded issue by copying it, +/// modifying one or more of its properties, and returning the copy. You can +/// observe recorded issues by returning them unmodified. Or you can suppress an +/// issue by either filtering it using ``Trait/filterIssues(_:)`` or returning +/// `nil` from the closure passed to ``Trait/transformIssues(_:)``. +/// +/// When an instance of this trait is applied to a suite, it is recursively +/// inherited by all child suites and tests. +/// +/// To add this trait to a test, use one of the following functions: +/// +/// - ``Trait/transformIssues(_:)`` +/// - ``Trait/filterIssues(_:)`` +@_spi(Experimental) +public struct IssueHandlingTrait: TestTrait, SuiteTrait { + /// A function which transforms an issue and returns an optional replacement. + /// + /// - Parameters: + /// - issue: The issue to transform. + /// + /// - Returns: An issue to replace `issue`, or else `nil` if the issue should + /// not be recorded. + fileprivate typealias Transformer = @Sendable (_ issue: Issue) -> Issue? + + /// This trait's transformer function. + private var _transformer: Transformer + + fileprivate init(transformer: @escaping Transformer) { + _transformer = transformer + } + + public var isRecursive: Bool { + true + } +} + +extension IssueHandlingTrait: TestScoping { + public func scopeProvider(for test: Test, testCase: Test.Case?) -> Self? { + // Provide scope for tests at both the suite and test case levels, but not + // for the test function level. This avoids redundantly invoking the closure + // twice, and potentially double-processing, issues recorded by test + // functions. + test.isSuite || testCase != nil ? self : nil + } + + public func provideScope(for test: Test, testCase: Test.Case?, performing function: @Sendable () async throws -> Void) async throws { + try await provideScope(performing: function) + } + + /// Provide scope for a specified function. + /// + /// - Parameters: + /// - function: The function to perform. + /// + /// This is a simplified version of ``provideScope(for:testCase:performing:)`` + /// which doesn't accept test or test case parameters. It's included so that + /// a runner can invoke this trait's closure even when there is no test case, + /// such as if a trait on a test function threw an error during `prepare(for:)` + /// and caused an issue to be recorded for the test function. In that scenario, + /// this trait still needs to be invoked, but its `scopeProvider(for:testCase:)` + /// intentionally returns `nil` (see the comment in that method), so this + /// function can be called instead to ensure this trait can still handle that + /// issue. + func provideScope(performing function: @Sendable () async throws -> Void) async throws { + guard var configuration = Configuration.current else { + preconditionFailure("Configuration.current is nil when calling \(#function). Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") + } + + configuration.eventHandler = { [oldConfiguration = configuration] event, context in + guard case let .issueRecorded(issue) = event.kind else { + oldConfiguration.eventHandler(event, context) + return + } + + // Use the original configuration's event handler when invoking the + // transformer to avoid infinite recursion if the transformer itself + // records new issues. This means only issue handling traits whose scope + // is outside this one will be allowed to handle such issues. + let newIssue = Configuration.withCurrent(oldConfiguration) { + _transformer(issue) + } + + if let newIssue { + var event = event + event.kind = .issueRecorded(newIssue) + oldConfiguration.eventHandler(event, context) + } + } + + try await Configuration.withCurrent(configuration, perform: function) + } +} + +@_spi(Experimental) +extension Trait where Self == IssueHandlingTrait { + /// Constructs an trait that transforms issues recorded by a test. + /// + /// - Parameters: + /// - transformer: The closure called for each issue recorded by the test + /// this trait is applied to. It is passed a recorded issue, and returns + /// an optional issue to replace the passed-in one. + /// + /// The `transformer` closure is called each time an issue is recorded by the + /// test this trait is applied to. The closure is passed the recorded issue, + /// and if it returns a non-`nil` value, that will be recorded instead of the + /// original. Otherwise, if the closure returns `nil`, the issue is suppressed + /// and will not be included in the results. + /// + /// The `transformer` closure may be called more than once if the test records + /// multiple issues. If more than one instance of this trait is applied to a + /// test (including via inheritance from a containing suite), the `transformer` + /// closure for each instance will be called in right-to-left, innermost-to- + /// outermost order, unless `nil` is returned, which will skip invoking the + /// remaining traits' closures. + /// + /// Within `transformer`, you may access the current test or test case (if any) + /// using ``Test/current`` ``Test/Case/current``, respectively. You may also + /// record new issues, although they will only be handled by issue handling + /// traits which precede this trait or were inherited from a containing suite. + public static func transformIssues(_ transformer: @escaping @Sendable (Issue) -> Issue?) -> Self { + Self(transformer: transformer) + } + + /// Constructs a trait that filters issues recorded by a test. + /// + /// - Parameters: + /// - isIncluded: The predicate with which to filter issues recorded by the + /// test this trait is applied to. It is passed a recorded issue, and + /// should return `true` if the issue should be included, or `false` if it + /// should be suppressed. + /// + /// The `isIncluded` closure is called each time an issue is recorded by the + /// test this trait is applied to. The closure is passed the recorded issue, + /// and if it returns `true`, the issue will be preserved in the test results. + /// Otherwise, if the closure returns `false`, the issue will not be included + /// in the test results. + /// + /// The `isIncluded` closure may be called more than once if the test records + /// multiple issues. If more than one instance of this trait is applied to a + /// test (including via inheritance from a containing suite), the `isIncluded` + /// closure for each instance will be called in right-to-left, innermost-to- + /// outermost order, unless `false` is returned, which will skip invoking the + /// remaining traits' closures. + /// + /// Within `isIncluded`, you may access the current test or test case (if any) + /// using ``Test/current`` ``Test/Case/current``, respectively. You may also + /// record new issues, although they will only be handled by issue handling + /// traits which precede this trait or were inherited from a containing suite. + public static func filterIssues(_ isIncluded: @escaping @Sendable (Issue) -> Bool) -> Self { + Self { issue in + isIncluded(issue) ? issue : nil + } + } +} diff --git a/Tests/TestingTests/Traits/IssueHandlingTraitTests.swift b/Tests/TestingTests/Traits/IssueHandlingTraitTests.swift new file mode 100644 index 000000000..4d749b07f --- /dev/null +++ b/Tests/TestingTests/Traits/IssueHandlingTraitTests.swift @@ -0,0 +1,197 @@ +// +// 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 +// + +@_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing + +@Suite("IssueHandlingTrait Tests") +struct IssueHandlingTraitTests { + @Test("Transforming an issue by appending a comment") + func addComment() async throws { + var configuration = Configuration() + configuration.eventHandler = { event, context in + guard case let .issueRecorded(issue) = event.kind, case .unconditional = issue.kind else { + return + } + + #expect(issue.comments == ["Foo", "Bar"]) + } + + let handler = IssueHandlingTrait.transformIssues { issue in + var issue = issue + issue.comments.append("Bar") + return issue + } + + await Test(handler) { + Issue.record("Foo") + }.run(configuration: configuration) + } + + @Test("Suppressing an issue by returning `nil` from the transform closure") + func suppressIssueUsingTransformer() async throws { + var configuration = Configuration() + configuration.eventHandler = { event, context in + if case .issueRecorded = event.kind { + Issue.record("Unexpected issue recorded event: \(event)") + } + } + + let handler = IssueHandlingTrait.transformIssues { _ in + // Return nil to suppress the issue. + nil + } + + await Test(handler) { + Issue.record("Foo") + }.run(configuration: configuration) + } + + @Test("Suppressing an issue by returning `false` from the filter closure") + func filterIssue() async throws { + var configuration = Configuration() + configuration.eventHandler = { event, context in + if case .issueRecorded = event.kind { + Issue.record("Unexpected issue recorded event: \(event)") + } + } + + await Test(.filterIssues { _ in false }) { + Issue.record("Foo") + }.run(configuration: configuration) + } + +#if !SWT_NO_UNSTRUCTURED_TASKS + @Test("Transforming an issue recorded from another trait on the test") + func skipIssue() async throws { + var configuration = Configuration() + configuration.eventHandler = { event, context in + guard case let .issueRecorded(issue) = event.kind, case .errorCaught = issue.kind else { + return + } + + #expect(issue.comments == ["Transformed!"]) + } + + struct MyError: Error {} + + try await confirmation("Transformer closure is called") { transformerCalled in + let transformer: @Sendable (Issue) -> Issue? = { issue in + defer { + transformerCalled() + } + + #expect(Test.Case.current == nil) + + var issue = issue + issue.comments = ["Transformed!"] + return issue + } + + let test = Test( + .enabled(if: try { throw MyError() }()), + .transformIssues(transformer) + ) {} + + // Use a detached task to intentionally clear task local values for the + // current test and test case, since this test validates their value. + await Task.detached { [configuration] in + await test.run(configuration: configuration) + }.value + } + } +#endif + + @Test("Accessing the current Test and Test.Case from a transformer closure") + func currentTestAndCase() async throws { + await confirmation("Transformer closure is called") { transformerCalled in + let handler = IssueHandlingTrait.transformIssues { issue in + defer { + transformerCalled() + } + #expect(Test.current?.name == "fixture()") + #expect(Test.Case.current != nil) + return issue + } + + var test = Test(handler) { + Issue.record("Foo") + } + test.name = "fixture()" + await test.run() + } + } + + @Test("Validate the relative execution order of multiple issue handling traits") + func traitOrder() async throws { + var configuration = Configuration() + configuration.eventHandler = { event, context in + guard case let .issueRecorded(issue) = event.kind, case .unconditional = issue.kind else { + return + } + + // Ordering is intentional + #expect(issue.comments == ["Foo", "Bar", "Baz"]) + } + + let outerHandler = IssueHandlingTrait.transformIssues { issue in + var issue = issue + issue.comments.append("Baz") + return issue + } + let innerHandler = IssueHandlingTrait.transformIssues { issue in + var issue = issue + issue.comments.append("Bar") + return issue + } + + await Test(outerHandler, innerHandler) { + Issue.record("Foo") + }.run(configuration: configuration) + } + + @Test("Secondary issue recorded from a transformer closure") + func issueRecordedFromClosure() async throws { + await confirmation("Original issue recorded") { originalIssueRecorded in + await confirmation("Secondary issue recorded") { secondaryIssueRecorded in + var configuration = Configuration() + configuration.eventHandler = { event, context in + guard case let .issueRecorded(issue) = event.kind, case .unconditional = issue.kind else { + return + } + + if issue.comments.contains("Foo") { + originalIssueRecorded() + } else if issue.comments.contains("Something else") { + secondaryIssueRecorded() + } else { + Issue.record("Unexpected issue recorded: \(issue)") + } + } + + let handler1 = IssueHandlingTrait.transformIssues { issue in + return issue + } + let handler2 = IssueHandlingTrait.transformIssues { issue in + Issue.record("Something else") + return issue + } + let handler3 = IssueHandlingTrait.transformIssues { issue in + // The "Something else" issue should not be passed to this closure. + #expect(issue.comments.contains("Foo")) + return issue + } + + await Test(handler1, handler2, handler3) { + Issue.record("Foo") + }.run(configuration: configuration) + } + } + } +} From a3c1c650501855495acf6607bab4002bca7ac410 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 17 Apr 2025 10:07:10 -0500 Subject: [PATCH 2/2] Note that the closures are invoked synchronously (not at the end of the test) --- .../Testing/Traits/IssueHandlingTrait.swift | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Sources/Testing/Traits/IssueHandlingTrait.swift b/Sources/Testing/Traits/IssueHandlingTrait.swift index 5063d1571..d70d68e93 100644 --- a/Sources/Testing/Traits/IssueHandlingTrait.swift +++ b/Sources/Testing/Traits/IssueHandlingTrait.swift @@ -113,11 +113,11 @@ extension Trait where Self == IssueHandlingTrait { /// this trait is applied to. It is passed a recorded issue, and returns /// an optional issue to replace the passed-in one. /// - /// The `transformer` closure is called each time an issue is recorded by the - /// test this trait is applied to. The closure is passed the recorded issue, - /// and if it returns a non-`nil` value, that will be recorded instead of the - /// original. Otherwise, if the closure returns `nil`, the issue is suppressed - /// and will not be included in the results. + /// The `transformer` closure is called synchronously each time an issue is + /// recorded by the test this trait is applied to. The closure is passed the + /// recorded issue, and if it returns a non-`nil` value, that will be recorded + /// instead of the original. Otherwise, if the closure returns `nil`, the + /// issue is suppressed and will not be included in the results. /// /// The `transformer` closure may be called more than once if the test records /// multiple issues. If more than one instance of this trait is applied to a @@ -142,11 +142,11 @@ extension Trait where Self == IssueHandlingTrait { /// should return `true` if the issue should be included, or `false` if it /// should be suppressed. /// - /// The `isIncluded` closure is called each time an issue is recorded by the - /// test this trait is applied to. The closure is passed the recorded issue, - /// and if it returns `true`, the issue will be preserved in the test results. - /// Otherwise, if the closure returns `false`, the issue will not be included - /// in the test results. + /// The `isIncluded` closure is called synchronously each time an issue is + /// recorded by the test this trait is applied to. The closure is passed the + /// recorded issue, and if it returns `true`, the issue will be preserved in + /// the test results. Otherwise, if the closure returns `false`, the issue + /// will not be included in the test results. /// /// The `isIncluded` closure may be called more than once if the test records /// multiple issues. If more than one instance of this trait is applied to a