Swift Testing Proposal: Polling Confirmations #2926
Conversation
…to evaluate the expression after it passes
I'm working on it, but macros are hard.
- Change the default timeout. - Change how unexpected thrown errors are treated. - Add future direction to add change monitoring via Observation - Add alternative considered of Just Use A While Loop - Add alternative considered for shorter default timeouts
- Include commenting on why not pure counts (i.e. why counts + interval) - Specify the configuration traits - Specify callsite configuration of counts + interval
… do not have a configurable clock
stmontgomery
added
the
TWG
| /// complex scenarios where other forms of confirmation are insufficient. For | ||
| /// example, waiting on some state to change that cannot be easily confirmed | ||
| /// through other forms of `confirmation`. | ||
| @available(_clockAPI, *) |
There was a problem hiding this comment.
_clockAPI is an availability definition specific to the Swift Testing codebase, and at build time it expands to actual OS version numbers. For the purpose of an API proposal I recommend showing the actual, expanded OS versions so people understand what they'll be in practice.
(This comment applies everywhere in the document which references _clockAPI.)
| Polling confirmations will not be available on platforms that do not support | ||
| Swift Concurrency. |
There was a problem hiding this comment.
The availability will also be constrained to OS versions which have the Clock, Duration, and related types. You may want to mention the specific Apple OS versions in particular.
| /// The minimum amount of time to wait between polling attempts | ||
| public var interval: Duration? | ||
|
|
||
| public var isRecursive: Bool { true } |
There was a problem hiding this comment.
You can omit the { true } implementation and only show that it's a get-able property, e.g.
| public var isRecursive: Bool { true } | |
| public var isRecursive: Bool { get } |
But I would recommend documenting that this trait type is recursively applied to children. For other trait types we've done this in the DocC for the type itself.
| /// To add this trait to a test, use the | ||
| /// ``Trait/pollingUntilFirstPassDefaults`` function. | ||
| @available(_clockAPI, *) | ||
| public struct PollingUntilFirstPassConfigurationTrait: TestTrait, SuiteTrait { |
There was a problem hiding this comment.
Assuming you do want to include the ability to influence this feature's behavior via a trait, I would suggest consolidating these two trait types into one if possible. For other traits we've tried to have the types be broader and use the static functions which initialize those types be the point where we differentiate their behavior. For example, the .enabled(if:) and .disabled(if:) functions both return a type of the same type: ConditionTrait.
But to accomplish this consolidation you'll likely need to store the PollingStopCondition in the trait instance. I think these changes would make for a simpler trait story for this feature: a single trait with a simpler name like PollingConfigurationTrait, with a property specifying its stopping condition.
|
|
||
| ```swift | ||
| public struct Issue { | ||
| public enum Kind { |
There was a problem hiding this comment.
Small style suggestion: When proposing a new member of an existing type, such as a new case in an enum here, I often add a placeholder comment like // ... above the additions to reflect that there are existing members and you're just adding things.
| public var duration: Duration? | ||
| /// The minimum amount of time to wait between polling attempts | ||
| public var interval: Duration? |
There was a problem hiding this comment.
Since both of these properties are optional, can you document (and decide, if it hasn't been settled yet) what the behavior will be if either or both of these are nil?
|
|
||
| @available(_clockAPI, *) | ||
| extension Trait where Self == PollingUntilFirstPassConfigurationTrait { | ||
| /// Specifies defaults for ``confirmPassesEventually`` in the test or suite. |
There was a problem hiding this comment.
This documentation still references the (older, I think?) API name confirmPassesEventually. (Similar comment for the other trait below, referencing confirmPassesAlways.)
| ### Default Polling Configuration | ||
|
|
||
| For all polling confirmations, the Testing library will default `duration` to | ||
| 1 second, and `interval` to 1 millisecond. |
There was a problem hiding this comment.
I'm somewhat hesitant to formally document and "promise" particular values for the duration and polling interval. Moreso for the latter; those are effectively policy details and I could imagine in the future having those be platform- or environment-specific. Or we could get fancier and decide to make the polling interval a curve rather than a fixed value: for example it could poll very frequently and aggressively at the beginning, but over time gradually back off if it seems like the condition is taking a while to become satisfied.
These are just speculative ideas and I'm not suggesting you actually pursue them right now. (Maybe they'd be good to mention in Alternatives considered?) But for now, I'm only suggesting that if we formally specify and commit to those numbers in this proposal, we might lose the flexibility to refine the behavior later. So maybe it would be better to be a little more vague, and say that the default interval will be short, but the testing library may adjust it to behave well in each runtime environment.
| } | ||
| ``` | ||
|
|
||
| With the definition of `Aquarium` above, the closure will only need to be |
There was a problem hiding this comment.
nit: "may" only need? Since technically you can't predict or guarantee any particular timing; the system could be under heavy load
| With the definition of `Aquarium` above, the closure will only need to be | |
| With the definition of `Aquarium` above, the closure may only need to be |
| Polling will be stopped in the following cases: | ||
|
|
||
| - The specified `duration` has elapsed. | ||
| - If the task that started the polling is cancelled. | ||
| - For `PollingStopCondition.firstPass`: The first time the closure returns true | ||
| or a non-nil value | ||
| - For `PollingStopCondition.stopsPassing`: The first time the closure returns | ||
| false or nil. | ||
| - The first time the closure throws an error. |
There was a problem hiding this comment.
It might not be clear to a reader that this is an "OR" list not "AND". I find it helpful when proposals clarify that kind of thing by adding "…, or" to the second-to-last bullet. For example:
| Polling will be stopped in the following cases: | |
| - The specified `duration` has elapsed. | |
| - If the task that started the polling is cancelled. | |
| - For `PollingStopCondition.firstPass`: The first time the closure returns true | |
| or a non-nil value | |
| - For `PollingStopCondition.stopsPassing`: The first time the closure returns | |
| false or nil. | |
| - The first time the closure throws an error. | |
| Polling will be stopped when either: | |
| - the specified duration has elapsed, | |
| - the task that started the polling is cancelled, | |
| - the closure returns a value that satisfies the stopping condition, or | |
| - the closure throws an error. |
(And in that suggestion I shortened the third bullet, instead of reiterating the stopping condition behaviors, since those are specified elsewhere in the document.)
stmontgomery
added
the
workgroup: needs development
New proposal for Swift Testing: Polling Confirmations.
These allow test authors to check that state updates over the course of some amount of time, in a very brute-force kind of way. Very helpful when there aren't other means of checking this available.