Skip to content

Commit 3765b11

Browse files
committed
PR Feedback
1 parent 7bb41c8 commit 3765b11

File tree

1 file changed

+88
-100
lines changed

1 file changed

+88
-100
lines changed

proposals/testing/NNNN-polling-confirmations.md

Lines changed: 88 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -93,17 +93,15 @@ testing library:
9393
/// This value may not correspond to the wall-clock time that polling lasts
9494
/// for, especially on highly-loaded systems with a lot of tests running.
9595
/// If nil, this uses whatever value is specified under the last
96-
/// ``PollingUntilFirstPassConfigurationTrait`` or
97-
/// ``PollingUntilStopsPassingConfigurationTrait`` added to the test or
98-
/// suite.
96+
/// ``PollingConfirmationConfigurationTrait`` added to the test or suite
97+
/// with a matching stopCondition.
9998
/// If no such trait has been added, then polling will be attempted for
10099
/// about 1 second before recording an issue.
101100
/// `duration` must be greater than 0.
102101
/// - interval: The minimum amount of time to wait between polling attempts.
103102
/// If nil, this uses whatever value is specified under the last
104-
/// ``PollingUntilFirstPassConfigurationTrait`` or
105-
/// ``PollingUntilStopsPassingConfigurationTrait`` added to the test or
106-
/// suite.
103+
/// ``PollingConfirmationConfigurationTrait`` added to the test or suite
104+
/// with a matching stopCondition.
107105
/// If no such trait has been added, then polling will wait at least
108106
/// 1 millisecond between polling attempts.
109107
/// `interval` must be greater than 0.
@@ -119,7 +117,7 @@ testing library:
119117
/// complex scenarios where other forms of confirmation are insufficient. For
120118
/// example, waiting on some state to change that cannot be easily confirmed
121119
/// through other forms of `confirmation`.
122-
@available(_clockAPI, *)
120+
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
123121
public func confirmation(
124122
_ comment: Comment? = nil,
125123
until stopCondition: PollingStopCondition,
@@ -140,17 +138,15 @@ public func confirmation(
140138
/// This value may not correspond to the wall-clock time that polling lasts
141139
/// for, especially on highly-loaded systems with a lot of tests running.
142140
/// If nil, this uses whatever value is specified under the last
143-
/// ``PollingUntilFirstPassConfigurationTrait`` or
144-
/// ``PollingUntilStopsPassingConfigurationTrait`` added to the test or
145-
/// suite.
141+
/// ``PollingConfirmationConfigurationTrait`` added to the test or suite
142+
/// with a matching stopCondition.
146143
/// If no such trait has been added, then polling will be attempted for
147144
/// about 1 second before recording an issue.
148145
/// `duration` must be greater than 0.
149146
/// - interval: The minimum amount of time to wait between polling attempts.
150147
/// If nil, this uses whatever value is specified under the last
151-
/// ``PollingUntilFirstPassConfigurationTrait`` or
152-
/// ``PollingUntilStopsPassingConfigurationTrait`` added to the test or
153-
/// suite.
148+
/// ``PollingConfirmationConfigurationTrait`` added to the test or suite
149+
/// with a matching stopCondition.
154150
/// If no such trait has been added, then polling will wait at least
155151
/// 1 millisecond between polling attempts.
156152
/// `interval` must be greater than 0.
@@ -168,7 +164,7 @@ public func confirmation(
168164
/// complex scenarios where other forms of confirmation are insufficient. For
169165
/// example, waiting on some state to change that cannot be easily confirmed
170166
/// through other forms of `confirmation`.
171-
@available(_clockAPI, *)
167+
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
172168
@discardableResult
173169
public func confirmation<R>(
174170
_ comment: Comment? = nil,
@@ -191,7 +187,7 @@ how the confirmation should be handled.
191187
```swift
192188
/// A type defining when to stop polling early.
193189
/// This also determines what happens if the duration elapses during polling.
194-
public enum PollingStopCondition: Sendable {
190+
public enum PollingStopCondition: Sendable, Equatable {
195191
/// Evaluates the expression until the first time it returns true.
196192
/// If it does not pass once by the time the timeout is reached, then a
197193
/// failure will be reported.
@@ -214,7 +210,7 @@ confirmation doesn't pass:
214210

215211
```swift
216212
/// A type describing an error thrown when polling fails.
217-
public struct PollingFailedError: Error, Sendable, CustomIssueRepresentable {}
213+
public struct PollingFailedError: Error, Sendable {}
218214
```
219215

220216
### New `Issue.Kind` case
@@ -224,70 +220,67 @@ a failed polling confirmation.
224220

225221
```swift
226222
public struct Issue {
223+
// ...
227224
public enum Kind {
225+
// ...
226+
228227
/// An issue due to a polling confirmation having failed.
229228
///
230229
/// This issue can occur when calling ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-455gr``
231230
/// or
232231
/// ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-5tnlk``
233232
/// whenever the polling fails, as described in ``PollingStopCondition``.
234233
case pollingConfirmationFailed
234+
235+
// ...
235236
}
237+
238+
// ...
236239
}
237240
```
238241

239-
### New Traits
242+
### New Trait
240243

241-
Two new traits will be added to change the default values for the
242-
`duration` and `interval` arguments. Test authors will often want to poll for
243-
the `firstPass` stop condition for longer than they poll for the
244-
`stopsPassing` stop condition, which is why there are separate traits for
245-
configuring defaults for these functions.
244+
A new trait will be added to change the default values for the
245+
`duration` and `interval` arguments for matching `PollingStopCondition`s.
246+
Test authors will often want to poll for the `firstPass` stop condition for
247+
longer than they poll for the `stopsPassing` stop condition, which is why there
248+
are separate traits for configuring defaults for these functions.
246249

247250
```swift
248251
/// A trait to provide a default polling configuration to all usages of
249252
/// ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-455gr``
250253
/// and
251254
/// ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-5tnlk``
252-
/// within a test or suite for the ``PollingStopCondition.firstPass``
253-
/// stop condition.
254-
///
255-
/// To add this trait to a test, use the
256-
/// ``Trait/pollingUntilFirstPassDefaults`` function.
257-
@available(_clockAPI, *)
258-
public struct PollingUntilFirstPassConfigurationTrait: TestTrait, SuiteTrait {
259-
/// How long to continue polling for
260-
public var duration: Duration?
261-
/// The minimum amount of time to wait between polling attempts
262-
public var interval: Duration?
263-
264-
public var isRecursive: Bool { true }
265-
}
266-
267-
/// A trait to provide a default polling configuration to all usages of
268-
/// ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-455gr``
269-
/// and
270-
/// ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-5tnlk``
271-
/// within a test or suite for the ``PollingStopCondition.stopsPassing``
272-
/// stop condition.
255+
/// within a test or suite using the specified stop condition.
273256
///
274-
/// To add this trait to a test, use the ``Trait/pollingUntilStopsPassingDefaults``
257+
/// To add this trait to a test, use the ``Trait/pollingConfirmationDefaults``
275258
/// function.
276-
@available(_clockAPI, *)
277-
public struct PollingUntilStopsPassingConfigurationTrait: TestTrait, SuiteTrait {
278-
/// How long to continue polling for
279-
public var duration: Duration?
280-
/// The minimum amount of time to wait between polling attempts
281-
public var interval: Duration?
282-
283-
public var isRecursive: Bool { true }
259+
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
260+
public struct PollingConfirmationConfigurationTrait: TestTrait, SuiteTrait {
261+
/// The stop condition to this configuration is valid for
262+
public var stopCondition: PollingStopCondition { get }
263+
264+
/// How long to continue polling for. If nil, this will fall back to the next
265+
/// inner-most `PollingUntilStopsPassingConfigurationTrait.duration` value.
266+
/// If no non-nil values are found, then it will use 1 second.
267+
public var duration: Duration? { get }
268+
269+
/// The minimum amount of time to wait between polling attempts. If nil, this
270+
/// will fall back to earlier `PollingUntilStopsPassingConfigurationTrait.interval`
271+
/// values. If no non-nil values are found, then it will use 1 millisecond.
272+
public var interval: Duration? { get }
273+
274+
/// This trait will be recursively applied to all children.
275+
public var isRecursive: Bool { get }
284276
}
285277

286-
@available(_clockAPI, *)
287-
extension Trait where Self == PollingUntilFirstPassConfigurationTrait {
288-
/// Specifies defaults for ``confirmPassesEventually`` in the test or suite.
278+
@available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *)
279+
extension Trait where Self == PollingConfirmationConfigurationTrait {
280+
/// Specifies defaults for polling confirmations in the test or suite.
289281
///
290282
/// - Parameters:
283+
/// - stopCondition: The `PollingStopCondition` this trait applies to.
291284
/// - duration: The expected length of time to continue polling for.
292285
/// This value may not correspond to the wall-clock time that polling
293286
/// lasts for, especially on highly-loaded systems with a lot of tests
@@ -299,50 +292,31 @@ extension Trait where Self == PollingUntilFirstPassConfigurationTrait {
299292
/// If nil, polling will wait at least 1 millisecond between polling
300293
/// attempts.
301294
/// `interval` must be greater than 0.
302-
public static func pollingUntilFirstPassDefaults(
303-
until duration: Duration? = nil,
304-
pollingEvery interval: Duration? = nil
305-
) -> Self
306-
}
307-
308-
@available(_clockAPI, *)
309-
extension Trait where Self == PollingUntilStopsPassingConfigurationTrait {
310-
/// Specifies defaults for ``confirmPassesAlways`` in the test or suite.
311-
///
312-
/// - Parameters:
313-
/// - duration: The expected length of time to continue polling for.
314-
/// This value may not correspond to the wall-clock time that polling
315-
/// lasts for, especially on highly-loaded systems with a lot of tests
316-
/// running.
317-
/// if nil, polling will be attempted for approximately 1 second.
318-
/// `duration` must be greater than 0.
319-
/// - interval: The minimum amount of time to wait between polling
320-
/// attempts.
321-
/// If nil, polling will wait at least 1 millisecond between polling
322-
/// attempts.
323-
/// `interval` must be greater than 0.
324-
public static func pollingUntilStopsPassingDefaults(
325-
until duration: Duration? = nil,
295+
public static func pollingConfirmationDefaults(
296+
until stopCondition: PollingStopCondition,
297+
within duration: Duration? = nil,
326298
pollingEvery interval: Duration? = nil
327299
) -> Self
328300
}
329301
```
330302

331303
Specifying `duration` or `interval` directly on either new `confirmation`
332304
function will override any value provided by the relevant trait. Additionally,
333-
when multiple of these configuration traits are specified, the innermost or
334-
last trait will be applied.
335-
336-
### Default Polling Configuration
337-
338-
For all polling confirmations, the Testing library will default `duration` to
339-
1 second, and `interval` to 1 millisecond.
305+
when multiple of these configuration traits with matching stop conditions are
306+
specified, the innermost or last trait will be applied. When no trait with a
307+
matching stop condition is found and no `duration` or `interval` values are
308+
specified at the callsite, then the Testing library will use some default
309+
values.
340310

341311
### Platform Availability
342312

343313
Polling confirmations will not be available on platforms that do not support
344314
Swift Concurrency.
345315

316+
Polling confirmations will also not be available on platforms that do not
317+
have the `Clock`, `Duration`, and related types. For Apple platforms, this
318+
requires macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0 and visionOS 1.0.
319+
346320
### Duration and Concurrent Execution
347321

348322
It is an unfortunate side effect that directly using the `duration` to determine
@@ -383,19 +357,16 @@ These functions can be used with an async test function:
383357
}
384358
```
385359

386-
With the definition of `Aquarium` above, the closure will only need to be
360+
With the definition of `Aquarium` above, the closure may only need to be
387361
evaluated a few times before it starts returning true. At which point polling
388362
will end, and no failure will be reported.
389363

390-
Polling will be stopped in the following cases:
364+
Polling will be stopped when either:
391365

392-
- The specified `duration` has elapsed.
393-
- If the task that started the polling is cancelled.
394-
- For `PollingStopCondition.firstPass`: The first time the closure returns true
395-
or a non-nil value
396-
- For `PollingStopCondition.stopsPassing`: The first time the closure returns
397-
false or nil.
398-
- The first time the closure throws an error.
366+
- the specified `duration` has elapsed,
367+
- the task that started the polling is cancelled,
368+
- the closure returns a value that satisfies the stopping condition, or
369+
- the closure throws an error.
399370

400371
## Source compatibility
401372

@@ -431,6 +402,21 @@ test authors to define their own stop conditions.
431402
In order to keep this proposal focused, I chose not to add them yet. They may
432403
be added as part of future proposals.
433404

405+
### Curved polling rates
406+
407+
As initially specified, the polling rate is flat: poll, sleep for the
408+
specified polling interval, repeat until the stop condition or timeout is
409+
reached.
410+
411+
Instead, polling could be implemented as a curve. For example, poll very
412+
frequently at first, but progressively wait longer and longer between poll
413+
attempts. Or the opposite: poll sporadically at first, increasing in frequency
414+
as polling continues. We could even offer custom curve options.
415+
416+
For this initial implementation, I wanted to keep this simple. As such, while
417+
a curve is promising, I think it is better considered on its own as a separate
418+
proposal.
419+
434420
## Alternatives considered
435421

436422
### Use separate functions instead of the `PollingStopCondition` enum
@@ -473,11 +459,13 @@ unreliable the more tests in the test suite.
473459
Another option considered was using polling iterations, either solely or
474460
combined with the interval value.
475461

476-
However, while this works and is resistant to many of the issues timeouts face
477-
in concurrent testing environments, it is extremely difficult for test authors
478-
to predict a good-enough polling iterations value. Most test authors will think
479-
in terms of a duration, and we would expect nearly all test authors to
480-
add helpers to compute a polling iteration for them.
462+
While this works and is resistant to many of the issues timeouts face
463+
in concurrent testing environments - which is why polling is implemented using
464+
iterations & sleep intervals - it is extremely difficult for test authors to
465+
predict a good-enough polling iterations value, reducing the utility of this
466+
feature. Most test authors think in terms of a duration, and I would expect test
467+
authors to either not use this feature, or to add helpers to compute a polling
468+
iteration count from a duration value anyway.
481469

482470
### Take in a `Clock` instance
483471

0 commit comments

Comments
 (0)