Skip to content
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

[SWT-NNNN] Introduce API allowing traits to customize test execution #733

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
100 changes: 52 additions & 48 deletions Documentation/Proposals/NNNN-custom-test-execution-traits.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
* Implementation: [swiftlang/swift-testing#733](https://github.com/swiftlang/swift-testing/pull/733), [swiftlang/swift-testing#86](https://github.com/swiftlang/swift-testing/pull/86)
* Review: ([pitch](https://forums.swift.org/...))
stmontgomery marked this conversation as resolved.
Show resolved Hide resolved

### Revision history

* **v1**: Initial pitch.
* **v2**: Dropped 'Custom' prefix from the proposed API names (although kept the
word in certain documentation passages where it clarified behavior).

## Introduction

This introduces API which enables a custom `Trait`-conforming type to customize
Expand Down Expand Up @@ -190,21 +196,19 @@ these scoped access calls to only the traits which require it.

I propose the following new APIs:

- A new protocol `CustomTestExecuting` with a single required `execute(...)`
method. This will be called to run a test, and allows the conforming type to
perform custom logic before or after.
- A new property `customTestExecutor` on the `Trait` protocol whose type is an
`Optional` value of a type conforming to `CustomTestExecuting`. A `nil` value
from this property will skip calling the `execute(...)` method.
- A default implementation of `Trait.customTestExecutor` whose value is `nil`.
- A conditional implementation of `Trait.customTestExecutor` whose value is
`self` in the common case where the trait type conforms to
`CustomTestExecuting` itself.

Since the `customTestExecutor` property is optional and `nil` by default, the
testing library cannot invoke the `execute(...)` method unless a trait
customizes test behavior. This avoids the "unnecessarily lengthy backtraces"
problem above.
- A new protocol `TestExecuting` with a single required `execute(...)` method.
This will be called to run a test, and allows the conforming type to perform
custom logic before or after.
- A new property `testExecutor` on the `Trait` protocol whose type is an
`Optional` value of a type conforming to `TestExecuting`. A `nil` value for
this property will skip calling the `execute(...)` method.
- A default implementation of `Trait.testExecutor` whose value is `nil`.
- A conditional implementation of `Trait.testExecutor` whose value is `self`
in the common case where the trait type conforms to `TestExecuting` itself.

Since the `testExecutor` property is optional and `nil` by default, the testing
library cannot invoke the `execute(...)` method unless a trait customizes test
behavior. This avoids the "unnecessarily lengthy backtraces" problem above.

Below are the proposed interfaces:

Expand All @@ -215,12 +219,12 @@ Below are the proposed interfaces:
///
/// Types conforming to this protocol may be used in conjunction with a
/// ``Trait``-conforming type by implementing the
/// ``Trait/customTestExecutor-1dwpt`` property, allowing custom traits to
/// ``Trait/testExecutor-714gp`` property, allowing custom traits to
/// customize the execution of tests. Consolidating common set-up and tear-down
/// logic for tests which have similar needs allows each test function to be
/// more succinct with less repetitive boilerplate so it can focus on what makes
/// it unique.
public protocol CustomTestExecuting: Sendable {
public protocol TestExecuting: Sendable {
/// Execute a function for the specified test and/or test case.
///
/// - Parameters:
Expand All @@ -241,8 +245,8 @@ public protocol CustomTestExecuting: Sendable {
/// When the testing library is preparing to run a test, it finds all traits
/// applied to that test (including those inherited from containing suites)
/// and asks each for the value of its
/// ``Trait/customTestExecutor-1dwpt`` property. It then calls this method on
/// all non-`nil` instances, giving each an opportunity to perform
/// ``Trait/testExecutor-714gp`` property. It then calls this method
/// on all non-`nil` instances, giving each an opportunity to perform
/// arbitrary work before or after invoking `function`.
///
/// This method should either invoke `function` once before returning or throw
Expand All @@ -259,42 +263,42 @@ public protocol CustomTestExecuting: Sendable {
public protocol Trait: Sendable {
// ...

/// The type of the custom test executor for this trait.
/// The type of the test executor for this trait.
///
/// The default type is `Never`, which cannot be instantiated. This means the
/// value of the ``customTestExecutor-1dwpt`` property for all traits with the
/// default custom executor type is `nil`, meaning such traits will not
/// value of the ``testExecutor-714gp`` property for all traits with
/// the default custom executor type is `nil`, meaning such traits will not
/// perform any custom execution for the tests they're applied to.
associatedtype CustomTestExecutor: CustomTestExecuting = Never
associatedtype TestExecutor: TestExecuting = Never

/// The custom test executor for this trait, if any.
/// The test executor for this trait, if any.
///
/// If this trait's type conforms to ``CustomTestExecuting``, the default
/// value of this property is `self` and this trait will be used to customize
/// test execution. This is the most straightforward way to implement a trait
/// which customizes the execution of tests.
/// If this trait's type conforms to ``TestExecuting``, the default value of
/// this property is `self` and this trait will be used to customize test
/// execution. This is the most straightforward way to implement a trait which
/// customizes the execution of tests.
///
/// If the value of this property is an instance of a different type
/// conforming to ``CustomTestExecuting``, that instance will be used to
/// perform custom test execution instead.
/// conforming to ``TestExecuting``, that instance will be used to perform
/// test execution instead.
///
/// The default value of this property is `nil` (with the default type
/// `Never?`), meaning that instances of this type will not perform any custom
/// test execution for tests they are applied to.
var customTestExecutor: CustomTestExecutor? { get }
var testExecutor: TestExecutor? { get }
}

extension Trait where Self: CustomTestExecuting {
extension Trait where Self: TestExecuting {
// Returns `self`.
public var customTestExecutor: Self? { get }
public var testExecutor: Self? {
}

extension Trait where CustomTestExecutor == Never {
extension Trait where TestExecutor == Never {
// Returns `nil`.
public var customTestExecutor: CustomTestExecutor? { get }
public var testExecutor: TestExecutor? {
}

extension Never: CustomTestExecuting {}
extension Never: TestExecuting {}
```

Here is a complete example of the usage scenario described earlier, showcasing
Expand All @@ -306,7 +310,7 @@ func example() {
// ...validate API usage, referencing `APICredentials.current`...
}

struct MockAPICredentialsTrait: TestTrait, CustomTestExecuting {
struct MockAPICredentialsTrait: TestTrait, TestExecuting {
func execute(_ function: @Sendable () async throws -> Void, for test: Test, testCase: Test.Case?) async throws {
let mockCredentials = APICredentials(apiKey: "...")
try await APICredentials.$current.withValue(mockCredentials) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We think task locals are going to be a particularly common use case, right? Perhaps we should build out a trait type that sets a task local:

@Test(.withValue(123, for: $foo)) func f() { ... }

Or even if we don't make it a trait, can we make it easier to write a trait that sets a task local with minimal boilerplate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that would be a very useful, built-in trait to have. Let's consider that separately, since it would ultimately leverage the API proposed here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to be part of this proposal, but I think it does affect this proposal. Might be worth coming up with an example of when you'd use this stuff other than to set a task local.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Briefly: not all work before/after a test is simply mutating state in that process. It may involve writing to the filesystem, or interacting with other system-wide resources. (Clearly, parallelism would need to be considered in such scenarios.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: Users can still use this facility to mutate global or static state, as long as it complies with Swift concurrency rules

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make sure to add this example to the discussion?

Expand Down Expand Up @@ -357,26 +361,26 @@ concurrency technique and reduces the potential for test parallelization.
### Add `execute(...)` directly to the `Trait` protocol

The proposed `execute(...)` method could be added as a requirement of the
`Trait` protocol instead of being part of a separate `CustomTestExecuting`
protocol, and it could have a default implementation which directly invokes the
passed-in closure. But this approach would suffer from the lengthy backtrace
problem described above.
`Trait` protocol instead of being part of a separate `TestExecuting` protocol,
and it could have a default implementation which directly invokes the passed-in
closure. But this approach would suffer from the lengthy backtrace problem
described above.

### Extend the `Trait` protocol

The original, experimental implementation of this feature included a protocol
named`CustomExecutionTrait` which extended `Trait` and had roughly the same
method requirement as the `CustomTestExecuting` protocol proposed above. This
design worked, provided scoped access, and avoided the lengthy backtrace problem.
method requirement as the `TestExecuting` protocol proposed above. This design
worked, provided scoped access, and avoided the lengthy backtrace problem.

After evaluating the design and usage of this SPI though, it seemed unfortunate
to structure it as a sub-protocol of `Trait` because it means that the full
capabilities of the trait system are spread across multiple protocols. In the
proposed design, the ability to provide a custom test executor value is exposed
via the main `Trait` protocol, and it relies on an associated type to
conditionally opt-in to custom test behavior. In other words, the proposed
design expresses custom test behavior as just a _capability_ that a trait may
have, rather than a distinct sub-type of trait.
proposed design, the ability to provide a test executor value is exposed via the
main `Trait` protocol, and it relies on an associated type to conditionally
opt-in to custom test behavior. In other words, the proposed design expresses
custom test behavior as just a _capability_ that a trait may have, rather than a
distinct sub-type of trait.

Also, the implementation of this approach within the testing library was not
ideal as it required a conditional `trait as? CustomExecutionTrait` downcast at
Expand Down
13 changes: 6 additions & 7 deletions Sources/Testing/Running/Runner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,19 @@ public struct Runner: Sendable {
// MARK: - Running tests

extension Runner {
/// Execute the ``CustomTestExecuting/execute(_:for:testCase:)`` functions of
/// any custom test executors for traits associated with the test in a plan
/// step.
/// Execute the ``TestExecuting/execute(_:for:testCase:)`` functions of any
/// test executors for traits associated with the test in a plan step.
///
/// - Parameters:
/// - step: The step being performed.
/// - testCase: The test case, if applicable, for which to execute the
/// function.
/// - body: A function to execute from within the
/// ``CustomTestExecuting/execute(_:for:testCase:)`` function of each
/// non-`nil` custom test executor of the traits applied to `step.test`.
/// ``TestExecuting/execute(_:for:testCase:)`` function of each non-`nil`
/// test executor of the traits applied to `step.test`.
///
/// - Throws: Whatever is thrown by `body` or by any of the
/// ``CustomTestExecuting/execute(_:for:testCase:)`` functions.
/// ``TestExecuting/execute(_:for:testCase:)`` functions.
private func _executeTraits(
for step: Plan.Step,
testCase: Test.Case?,
Expand All @@ -91,7 +90,7 @@ extension Runner {
// and ultimately the first trait is the first one to be invoked.
let executeAllTraits = step.test.traits.lazy
.reversed()
.compactMap { $0.customTestExecutor }
.compactMap { $0.testExecutor }
.map { $0.execute(_:for:testCase:) }
.reduce(body) { executeAllTraits, testExecutor in
{
Expand Down
6 changes: 3 additions & 3 deletions Sources/Testing/Testing.docc/Traits/Trait.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ See https://swift.org/CONTRIBUTORS.txt for Swift project authors

### Customizing the execution of tests

- ``CustomTestExecuting``
- ``Trait/customTestExecutor-1dwpt``
- ``Trait/CustomTestExecutor``
- ``TestExecuting``
- ``Trait/testExecutor-714gp``
- ``Trait/TestExecutor``
42 changes: 21 additions & 21 deletions Sources/Testing/Traits/Trait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,29 @@ public protocol Trait: Sendable {
/// By default, the value of this property is an empty array.
var comments: [Comment] { get }

/// The type of the custom test executor for this trait.
/// The type of the test executor for this trait.
///
/// The default type is `Never`, which cannot be instantiated. This means the
/// value of the ``customTestExecutor-1dwpt`` property for all traits with the
/// default custom executor type is `nil`, meaning such traits will not
/// value of the ``testExecutor-714gp`` property for all traits with
/// the default custom executor type is `nil`, meaning such traits will not
/// perform any custom execution for the tests they're applied to.
associatedtype CustomTestExecutor: CustomTestExecuting = Never
associatedtype TestExecutor: TestExecuting = Never

/// The custom test executor for this trait, if any.
/// The test executor for this trait, if any.
///
/// If this trait's type conforms to ``CustomTestExecuting``, the default
/// value of this property is `self` and this trait will be used to customize
/// test execution. This is the most straightforward way to implement a trait
/// which customizes the execution of tests.
/// If this trait's type conforms to ``TestExecuting``, the default value of
/// this property is `self` and this trait will be used to customize test
/// execution. This is the most straightforward way to implement a trait which
/// customizes the execution of tests.
///
/// If the value of this property is an instance of a different type
/// conforming to ``CustomTestExecuting``, that instance will be used to
/// perform custom test execution instead.
/// conforming to ``TestExecuting``, that instance will be used to perform
/// test execution instead.
///
/// The default value of this property is `nil` (with the default type
/// `Never?`), meaning that instances of this type will not perform any custom
/// test execution for tests they are applied to.
var customTestExecutor: CustomTestExecutor? { get }
var testExecutor: TestExecutor? { get }
}

/// A protocol that allows customizing the execution of a test function (and
stmontgomery marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -73,12 +73,12 @@ public protocol Trait: Sendable {
///
/// Types conforming to this protocol may be used in conjunction with a
/// ``Trait``-conforming type by implementing the
/// ``Trait/customTestExecutor-1dwpt`` property, allowing custom traits to
/// ``Trait/testExecutor-714gp`` property, allowing custom traits to
/// customize the execution of tests. Consolidating common set-up and tear-down
/// logic for tests which have similar needs allows each test function to be
/// more succinct with less repetitive boilerplate so it can focus on what makes
/// it unique.
public protocol CustomTestExecuting: Sendable {
public protocol TestExecuting: Sendable {
/// Execute a function for the specified test and/or test case.
///
/// - Parameters:
Expand All @@ -99,8 +99,8 @@ public protocol CustomTestExecuting: Sendable {
/// When the testing library is preparing to run a test, it finds all traits
/// applied to that test (including those inherited from containing suites)
/// and asks each for the value of its
/// ``Trait/customTestExecutor-1dwpt`` property. It then calls this method on
/// all non-`nil` instances, giving each an opportunity to perform
/// ``Trait/testExecutor-714gp`` property. It then calls this method
/// on all non-`nil` instances, giving each an opportunity to perform
/// arbitrary work before or after invoking `function`.
///
/// This method should either invoke `function` once before returning or throw
Expand All @@ -114,13 +114,13 @@ public protocol CustomTestExecuting: Sendable {
func execute(_ function: @Sendable () async throws -> Void, for test: Test, testCase: Test.Case?) async throws
}

extension Trait where Self: CustomTestExecuting {
public var customTestExecutor: Self? {
extension Trait where Self: TestExecuting {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want this to be TestTrait?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler doesn't allow that, because then it's ambiguous for types which conform to both TestTrait and SuiteTrait

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I have it now, the extension on Trait acts as the "default" implementation, and the SuiteTrait refines it conditionally.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it should be ambiguous for types that conform to both, or we should provide an additional default for them that executes once per suite or test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to break the current ability to have a trait type conform to both SuiteTrait and TestTrait. Do you think something is problematic about the current approach?

public var testExecutor: Self? {
self
}
}

extension Never: CustomTestExecuting {
extension Never: TestExecuting {
public func execute(_ function: @Sendable () async throws -> Void, for test: Test, testCase: Test.Case?) async throws {}
}

Expand Down Expand Up @@ -153,8 +153,8 @@ extension Trait {
}
}

extension Trait where CustomTestExecutor == Never {
public var customTestExecutor: CustomTestExecutor? {
extension Trait where TestExecutor == Never {
stmontgomery marked this conversation as resolved.
Show resolved Hide resolved
public var testExecutor: TestExecutor? {
nil
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

@testable @_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing

@Suite("CustomTestExecuting-conforming Trait Tests")
struct CustomTestExecutingTraitTests {
@Suite("TestExecuting-conforming Trait Tests")
struct TestExecutingTraitTests {
@Test("Execute code before and after a non-parameterized test.")
func executeCodeBeforeAndAfterNonParameterizedTest() async {
// `expectedCount` is 2 because we run it both for the test and the test case
Expand Down Expand Up @@ -65,7 +65,7 @@ struct CustomTestExecutingTraitTests {

// MARK: - Fixtures

private struct CustomTrait: TestTrait, CustomTestExecuting {
private struct CustomTrait: TestTrait, TestExecuting {
var before: Confirmation
var after: Confirmation
func execute(_ function: @Sendable () async throws -> Void, for test: Test, testCase: Test.Case?) async throws {
Expand All @@ -77,15 +77,15 @@ private struct CustomTrait: TestTrait, CustomTestExecuting {
}
}

private struct CustomThrowingErrorTrait: TestTrait, CustomTestExecuting {
private struct CustomThrowingErrorTrait: TestTrait, TestExecuting {
fileprivate struct CustomTraitError: Error {}

func execute(_ function: @Sendable () async throws -> Void, for test: Test, testCase: Test.Case?) async throws {
throw CustomTraitError()
}
}

struct DoSomethingBeforeAndAfterTrait: SuiteTrait, TestTrait, CustomTestExecuting {
struct DoSomethingBeforeAndAfterTrait: SuiteTrait, TestTrait, TestExecuting {
static let state = Locked(rawValue: 0)

func execute(_ function: @Sendable () async throws -> Void, for test: Test, testCase: Test.Case?) async throws {
Expand Down