From 4e8dbf7f3cd1a0a3056c88b06ef9c9df1dac5117 Mon Sep 17 00:00:00 2001 From: David Catmull Date: Sat, 15 Mar 2025 13:29:58 -0600 Subject: [PATCH 1/6] Proposal for ConditionTrait.evaluate(), moved from swift-testing --- proposals/testing/NNNN-evaluate-condition.md | 74 ++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 proposals/testing/NNNN-evaluate-condition.md diff --git a/proposals/testing/NNNN-evaluate-condition.md b/proposals/testing/NNNN-evaluate-condition.md new file mode 100644 index 0000000000..3fc8b9ad4d --- /dev/null +++ b/proposals/testing/NNNN-evaluate-condition.md @@ -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. From d617bd718f59442a92f0af79d17f6a7003741417 Mon Sep 17 00:00:00 2001 From: David Catmull Date: Wed, 19 Mar 2025 08:26:27 -0600 Subject: [PATCH 2/6] Include "extension ConditionTrait" in code exerpts --- proposals/testing/NNNN-evaluate-condition.md | 24 +++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/proposals/testing/NNNN-evaluate-condition.md b/proposals/testing/NNNN-evaluate-condition.md index 3fc8b9ad4d..2041d1c80a 100644 --- a/proposals/testing/NNNN-evaluate-condition.md +++ b/proposals/testing/NNNN-evaluate-condition.md @@ -30,13 +30,14 @@ 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) - } +extension ConditionTrait { + /// 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. + public func evaluate() async throws -> EvaluationResult } ``` @@ -44,7 +45,14 @@ public func evaluate() async throws -> EvaluationResult { of the callback in `Kind.conditional`: ```swift -public typealias EvaluationResult = (wasMet: Bool, comment: Comment?) +extension ConditionTrait { + /// 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. + public typealias EvaluationResult = (wasMet: Bool, comment: Comment?) +} ``` ## Source compatibility From 9299ebaa600e84f252de4b3a035a5baf432d119f Mon Sep 17 00:00:00 2001 From: David Catmull Date: Fri, 28 Mar 2025 08:36:11 -0600 Subject: [PATCH 3/6] Remove mentions of EvaluationResult, since it now uses Bool --- proposals/testing/NNNN-evaluate-condition.md | 21 +------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/proposals/testing/NNNN-evaluate-condition.md b/proposals/testing/NNNN-evaluate-condition.md index 2041d1c80a..4cd1cd806c 100644 --- a/proposals/testing/NNNN-evaluate-condition.md +++ b/proposals/testing/NNNN-evaluate-condition.md @@ -37,21 +37,7 @@ extension ConditionTrait { /// /// The evaluation is performed each time this function is called, and is not /// cached. - public func evaluate() async throws -> EvaluationResult -} -``` - -`EvaluationResult` is a `typealias` for the tuple already used as the result -of the callback in `Kind.conditional`: - -```swift -extension ConditionTrait { - /// 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. - public typealias EvaluationResult = (wasMet: Bool, comment: Comment?) + public func evaluate() async throws -> Bool } ``` @@ -75,8 +61,3 @@ This change seems sufficient for third party libraries to make use of 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. From 9b0c0627a58d0cd271b572d28076011975fd4a95 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Fri, 28 Mar 2025 10:47:36 -0500 Subject: [PATCH 4/6] Add myself as review manager --- proposals/testing/NNNN-evaluate-condition.md | 1 + 1 file changed, 1 insertion(+) diff --git a/proposals/testing/NNNN-evaluate-condition.md b/proposals/testing/NNNN-evaluate-condition.md index 4cd1cd806c..dcb9f44cf2 100644 --- a/proposals/testing/NNNN-evaluate-condition.md +++ b/proposals/testing/NNNN-evaluate-condition.md @@ -2,6 +2,7 @@ * Proposal: [SWT-NNNN](NNNN-evaluate-condition.md) * Authors: [David Catmull](https://github.com/Uncommon) +* Review Manager: [Stuart Montgomery](https://github.com/stmontgomery) * 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)) From 6b5eb54ab55aa52bbc156ead23e55f5b369076fc Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Fri, 28 Mar 2025 13:58:58 -0500 Subject: [PATCH 5/6] Note the associated issue --- proposals/testing/NNNN-evaluate-condition.md | 1 + 1 file changed, 1 insertion(+) diff --git a/proposals/testing/NNNN-evaluate-condition.md b/proposals/testing/NNNN-evaluate-condition.md index dcb9f44cf2..0ac7d9706f 100644 --- a/proposals/testing/NNNN-evaluate-condition.md +++ b/proposals/testing/NNNN-evaluate-condition.md @@ -4,6 +4,7 @@ * Authors: [David Catmull](https://github.com/Uncommon) * Review Manager: [Stuart Montgomery](https://github.com/stmontgomery) * Status: **Awaiting review** +* Bug: [swiftlang/swift-testing#903](https://github.com/swiftlang/swift-testing/issues/903) * 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)) From 7f981651effa8d5b5fb5455cd2d8736714fae69c Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Fri, 11 Apr 2025 10:05:25 -0500 Subject: [PATCH 6/6] Assign proposal ST number 0010 and update state to Active Review --- ...{NNNN-evaluate-condition.md => 0010-evaluate-condition.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename proposals/testing/{NNNN-evaluate-condition.md => 0010-evaluate-condition.md} (95%) diff --git a/proposals/testing/NNNN-evaluate-condition.md b/proposals/testing/0010-evaluate-condition.md similarity index 95% rename from proposals/testing/NNNN-evaluate-condition.md rename to proposals/testing/0010-evaluate-condition.md index 0ac7d9706f..d490a4b5ba 100644 --- a/proposals/testing/NNNN-evaluate-condition.md +++ b/proposals/testing/0010-evaluate-condition.md @@ -1,9 +1,9 @@ # Public API to evaluate ConditionTrait -* Proposal: [SWT-NNNN](NNNN-evaluate-condition.md) +* Proposal: [ST-0010](0010-evaluate-condition.md) * Authors: [David Catmull](https://github.com/Uncommon) * Review Manager: [Stuart Montgomery](https://github.com/stmontgomery) -* Status: **Awaiting review** +* Status: **Active review (April 11...April 22, 2025)** * Bug: [swiftlang/swift-testing#903](https://github.com/swiftlang/swift-testing/issues/903) * 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))