-
Notifications
You must be signed in to change notification settings - Fork 120
[SWT-0005] Range-based confirmations #691
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
Merged
grynspan
merged 10 commits into
main
from
jgrynspan/range-based-confirmations-with-pitch
Oct 25, 2024
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
180629b
[SWT-NNNN] Range-based confirmations
grynspan fa2cf2f
Add pitch link
grynspan f5f6edf
Require Sequence conformance too
grynspan d3ffe03
Address feedback
grynspan 36c5d5b
Avoid needing _parameterizedProtocolsAPI
grynspan fd1f83d
Update Sources/Testing/Testing.docc/testing-asynchronous-code.md
grynspan dad1298
Add bug number
grynspan 13e7098
Fix a typo in a test
grynspan 5195fae
Update proposal number
grynspan 1eb2987
Add acceptance link
grynspan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,186 @@ | ||
# Range-based confirmations | ||
|
||
* Proposal: [SWT-0005](0005-ranged-confirmations.md) | ||
* Authors: [Jonathan Grynspan](https://github.com/grynspan) | ||
* Status: **Accepted** | ||
* Bug: rdar://138499457 | ||
* Implementation: [swiftlang/swift-testing#598](https://github.com/swiftlang/swift-testing/pull/598), [swiftlang/swift-testing#689](https://github.com/swiftlang/swift-testing/pull689) | ||
* Review: ([pitch](https://forums.swift.org/t/pitch-range-based-confirmations/74589)), | ||
([acceptance](https://forums.swift.org/t/pitch-range-based-confirmations/74589/7)) | ||
|
||
## Introduction | ||
|
||
Swift Testing includes [an interface](https://swiftpackageindex.com/swiftlang/swift-testing/main/documentation/testing/confirmation(_:expectedcount:isolation:sourcelocation:_:)) | ||
for checking that some asynchronous event occurs a given number of times | ||
(typically exactly once or never at all.) This proposal enhances that interface | ||
to allow arbitrary ranges of event counts so that a test can be written against | ||
code that may not always fire said event the exact same number of times. | ||
|
||
## Motivation | ||
|
||
Some tests rely on fixtures or external state that is not perfectly | ||
deterministic. For example, consider a test that checks that clicking the mouse | ||
button will generate a `.mouseClicked` event. Such a test might use the | ||
`confirmation()` interface: | ||
|
||
```swift | ||
await confirmation(expectedCount: 1) { mouseClicked in | ||
var eventLoop = EventLoop() | ||
eventLoop.eventHandler = { event in | ||
if event == .mouseClicked { | ||
mouseClicked() | ||
} | ||
} | ||
await eventLoop.simulate(.mouseClicked) | ||
} | ||
``` | ||
|
||
But what happens if the user _actually_ clicks a mouse button while this test is | ||
running? That might trigger a _second_ `.mouseClicked` event, and then the test | ||
will fail spuriously. | ||
|
||
## Proposed solution | ||
|
||
If the test author could instead indicate to Swift Testing that their test will | ||
generate _one or more_ events, they could avoid spurious failures: | ||
|
||
```swift | ||
await confirmation(expectedCount: 1...) { mouseClicked in | ||
... | ||
} | ||
``` | ||
|
||
With this proposal, we add an overload of `confirmation()` that takes any range | ||
expression instead of a single integer value (which is still accepted via the | ||
existing overload.) | ||
|
||
## Detailed design | ||
|
||
A new overload of `confirmation()` is added: | ||
|
||
```swift | ||
/// Confirm that some event occurs during the invocation of a function. | ||
/// | ||
/// - Parameters: | ||
/// - comment: An optional comment to apply to any issues generated by this | ||
/// function. | ||
/// - expectedCount: A range of integers indicating the number of times the | ||
/// expected event should occur when `body` is invoked. | ||
/// - isolation: The actor to which `body` is isolated, if any. | ||
/// - sourceLocation: The source location to which any recorded issues should | ||
/// be attributed. | ||
/// - body: The function to invoke. | ||
/// | ||
/// - Returns: Whatever is returned by `body`. | ||
/// | ||
/// - Throws: Whatever is thrown by `body`. | ||
/// | ||
/// Use confirmations to check that an event occurs while a test is running in | ||
/// complex scenarios where `#expect()` and `#require()` are insufficient. For | ||
/// example, a confirmation may be useful when an expected event occurs: | ||
/// | ||
/// - In a context that cannot be awaited by the calling function such as an | ||
/// event handler or delegate callback; | ||
/// - More than once, or never; or | ||
/// - As a callback that is invoked as part of a larger operation. | ||
/// | ||
/// To use a confirmation, pass a closure containing the work to be performed. | ||
/// The testing library will then pass an instance of ``Confirmation`` to the | ||
/// closure. Every time the event in question occurs, the closure should call | ||
/// the confirmation: | ||
/// | ||
/// ```swift | ||
/// let minBuns = 5 | ||
/// let maxBuns = 10 | ||
/// await confirmation( | ||
/// "Baked between \(minBuns) and \(maxBuns) buns", | ||
/// expectedCount: minBuns ... maxBuns | ||
/// ) { bunBaked in | ||
/// foodTruck.eventHandler = { event in | ||
/// if event == .baked(.cinnamonBun) { | ||
/// bunBaked() | ||
/// } | ||
/// } | ||
/// await foodTruck.bakeTray(of: .cinnamonBun) | ||
/// } | ||
/// ``` | ||
/// | ||
/// When the closure returns, the testing library checks if the confirmation's | ||
/// preconditions have been met, and records an issue if they have not. | ||
/// | ||
/// If an exact count is expected, use | ||
/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead. | ||
public func confirmation<R>( | ||
_ comment: Comment? = nil, | ||
expectedCount: some RangeExpression<Int> & Sequence<Int> Sendable, | ||
isolation: isolated (any Actor)? = #isolation, | ||
sourceLocation: SourceLocation = #_sourceLocation, | ||
_ body: (Confirmation) async throws -> sending R | ||
) async rethrows -> R | ||
``` | ||
|
||
### Ranges without lower bounds | ||
|
||
Certain types of range, specifically [`PartialRangeUpTo`](https://developer.apple.com/documentation/swift/partialrangeupto) | ||
and [`PartialRangeThrough`](https://developer.apple.com/documentation/swift/partialrangethrough), | ||
may have surprising behavior when used with this new interface because they | ||
implicitly include `0`. If a test author writes `...10`, do they mean "zero to | ||
ten" or "one to ten"? The programmatic meaning is the former, but some test | ||
authors might mean the latter. If an event does not occur, a test using | ||
`confirmation()` and this `expectedCount` value would pass when the test author | ||
meant for it to fail. | ||
|
||
The unbounded range (`...`) type `UnboundedRange` is effectively useless when | ||
used with this interface and any use of it here is almost certainly a programmer | ||
error. | ||
|
||
`PartialRangeUpTo` and `PartialRangeThrough` conform to `RangeExpression`, but | ||
not to `Sequence`, so they will be rejected at compile time. `UnboundedRange` is | ||
a non-nominal type and will not match either. We will provide unavailable | ||
overloads of `confirmation()` for these types with messages that explain why | ||
they are unavailable, e.g.: | ||
|
||
```swift | ||
@available(*, unavailable, message: "Unbounded range '...' has no effect when used with a confirmation.") | ||
public func confirmation<R>( | ||
_ comment: Comment? = nil, | ||
expectedCount: UnboundedRange, | ||
isolation: isolated (any Actor)? = #isolation, | ||
sourceLocation: SourceLocation = #_sourceLocation, | ||
_ body: (Confirmation) async throws -> R | ||
) async rethrows -> R | ||
``` | ||
|
||
## Source compatibility | ||
|
||
This change is additive. Existing tests are unaffected. | ||
|
||
Code that refers to `confirmation(_:expectedCount:isolation:sourceLocation:_:)` | ||
by symbol name may need to add a contextual type to disambiguate the two | ||
overloads at compile time. | ||
|
||
## Integration with supporting tools | ||
|
||
The type of the associated value `expected` for the `Issue.Kind` case | ||
`confirmationMiscounted(actual:expected:)` will change from `Int` to | ||
`any RangeExpression & Sendable`[^1]. Tools that implement event handlers and | ||
distinguish between `Issue.Kind` cases are advised not to assume the type of | ||
this value is `Int`. | ||
|
||
## Alternatives considered | ||
|
||
- Doing nothing. We have identified real-world use cases for this interface | ||
including in Swift Testing’s own test target. | ||
- Allowing the use of any value as the `expectedCount` argument so long as it | ||
conforms to a protocol `ExpectedCount` (we'd have range types and `Int` | ||
conform by default.) It was unclear what this sort of flexibility would let | ||
us do, and posed challenges for encoding and decoding events and issues when | ||
using the JSON event stream interface. | ||
|
||
## Acknowledgments | ||
|
||
Thanks to the testing team for their help preparing this pitch! | ||
|
||
[^1]: In the future, this type will change to | ||
`any RangeExpression<Int> & Sendable`. Compiler support is required | ||
([96960993](rdar://96960993)). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.