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

Add ConditionTrait.evaluate() #909

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions Documentation/Proposals/NNNN-evaluate-condition.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Public API to evaluate ConditionTrait

* Proposal: [SWT-NNNN](NNNN-evaluate-condition.md)
* Authors: [David Catmull](https://github.com/Uncommon)
* Status: **Awaiting review**
* Implementation: [swiftlang/swift-testing#909](https://github.com/swiftlang/swift-testing/pull/909)
* Review: ([pitch](https://forums.swift.org/t/pitch-introduce-conditiontrait-evaluate/77242))

## Introduction

This adds an `evaluate()` method to `ConditionTrait` to evaluate the condition
without requiring a `Test` instance.

## Motivation

Currently, the only way a `ConditionTrait` is evaluated is inside the
`prepare(for:)` method. This makes it difficult for third-party libraries to
utilize these traits because evaluating a condition would require creating a
dummy `Test` to pass to that method.

## Proposed solution

The proposal is to add a `ConditionTrait.evaluate()` method which returns the
result of the evaluation. The existing `prepare(for:)` method is updated to call
`evaluate()` so that the logic is not duplicated.

## Detailed design

The `evaluate()` method is as follows, containing essentially the same logic
as was in `prepare(for:)`:

```swift
public func evaluate() async throws -> EvaluationResult {
switch kind {
case let .conditional(condition):
try await condition()
case let .unconditional(unconditionalValue):
(unconditionalValue, nil)
}
}
```

`EvaluationResult` is a `typealias` for the tuple already used as the result
of the callback in `Kind.conditional`:

```swift
public typealias EvaluationResult = (wasMet: Bool, comment: Comment?)
```

## Source compatibility

This change is purely additive.

## Integration with supporting tools

This change allows third-party libraries to apply condition traits at other
levels than suites or whole test functions, for example if tests are broken up
into smaller sections.

## Future directions

This change seems sufficient for third party libraries to make use of
`ConditionTrait`. Changes for other traits can be tackled in separate proposals.

## Alternatives considered

Exposing `ConditionTrait.Kind` and `.kind` was also considered, but it seemed
unnecessary to go that far, and it would encourage duplicating the logic that
already exists in `prepare(for:)`.

In the first draft implementation, the `EvaluationResult` type was an enum that
only contained the comment in the failure case. It was changed to match the
existing tuple to allow for potentially including comments for the success case
without requiring a change to the API.
36 changes: 26 additions & 10 deletions Sources/Testing/Traits/ConditionTrait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
/// - ``Trait/disabled(if:_:sourceLocation:)``
/// - ``Trait/disabled(_:sourceLocation:_:)``
public struct ConditionTrait: TestTrait, SuiteTrait {
/// The result of evaluating the condition.
///
/// - Parameters:
/// - wasMet: Whether or not the condition was met.
/// - comment: Optionally, a comment describing the result of evaluating the condition.
@_spi(Experimental)
public typealias EvaluationResult = (wasMet: Bool, comment: Comment?)

/// An enumeration describing the kinds of conditions that can be represented
/// by an instance of this type.
enum Kind: Sendable {
Expand All @@ -30,7 +38,7 @@ public struct ConditionTrait: TestTrait, SuiteTrait {
/// `false` and a comment is also returned, it is used in place of the
/// value of the associated trait's ``ConditionTrait/comment`` property.
/// If this function returns `true`, the returned comment is ignored.
case conditional(_ body: @Sendable () async throws -> (Bool, comment: Comment?))
case conditional(_ body: @Sendable () async throws -> EvaluationResult)

/// Create an instance of this type associated with a trait that is
/// conditional on the result of calling a function.
Expand All @@ -41,7 +49,7 @@ public struct ConditionTrait: TestTrait, SuiteTrait {
///
/// - Returns: An instance of this type.
static func conditional(_ body: @escaping @Sendable () async throws -> Bool) -> Self {
conditional { () -> (Bool, comment: Comment?) in
conditional { () -> EvaluationResult in
return (try await body(), nil)
}
}
Expand Down Expand Up @@ -79,19 +87,27 @@ public struct ConditionTrait: TestTrait, SuiteTrait {

/// The source location where this trait was specified.
public var sourceLocation: SourceLocation

public func prepare(for test: Test) async throws {
let result: Bool
var commentOverride: Comment?


/// Evaluate this instance's underlying condition.
///
/// - Returns: The result of evaluating this instance's underlying condition.
///
/// The evaluation is performed each time this function is called, and is not
/// cached.
@_spi(Experimental)
public func evaluate() async throws -> EvaluationResult {
switch kind {
case let .conditional(condition):
(result, commentOverride) = try await condition()
try await condition()
case let .unconditional(unconditionalValue):
result = unconditionalValue
(unconditionalValue, nil)
}
}

public func prepare(for test: Test) async throws {
let (isEnabled, commentOverride) = try await evaluate()

if !result {
if !isEnabled {
// 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_
Expand Down
26 changes: 26 additions & 0 deletions Tests/TestingTests/RunnerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,32 @@ final class RunnerTests: XCTestCase {
let test2 = Test(.disabled(if: Bool.random())) { }
XCTAssertTrue(test2.traits.compactMap { $0 as? ConditionTrait }.allSatisfy { !$0.isConstant })
}

func testEvaluateConditionTrait() async throws {
let comment: Comment = "comment"
let trueUnconditional = ConditionTrait(kind: .unconditional(true), comments: [], sourceLocation: #_sourceLocation)
let falseUnconditional = ConditionTrait.disabled()
let enabledTrue = ConditionTrait.enabled(if: true)
let enabledFalse = ConditionTrait.enabled(if: false)
let enabledTrueComment = ConditionTrait(kind: .conditional { (true, comment) }, comments: [], sourceLocation: #_sourceLocation)
let enabledFalseComment = ConditionTrait(kind: .conditional { (false, comment) }, comments: [], sourceLocation: #_sourceLocation)
var result: ConditionTrait.EvaluationResult

result = try await trueUnconditional.evaluate()
XCTAssertTrue(result.wasMet)
result = try await falseUnconditional.evaluate()
XCTAssertFalse(result.wasMet)
result = try await enabledTrue.evaluate()
XCTAssertTrue(result.wasMet)
result = try await enabledFalse.evaluate()
XCTAssertFalse(result.wasMet)
result = try await enabledTrueComment.evaluate()
XCTAssertTrue(result.wasMet)
XCTAssertEqual(result.comment, comment)
result = try await enabledFalseComment.evaluate()
XCTAssertFalse(result.wasMet)
XCTAssertEqual(result.comment, comment)
}

func testGeneratedPlan() async throws {
let tests: [(Any.Type, String)] = [
Expand Down