Polling confirmations (under Experimental spi) #1115
Conversation
younata
requested review from
stmontgomery,
grynspan,
briancroom and
suzannaratcliff
as code owners
grynspan
added
enhancement
younata
changed the title
younata
force-pushed
the
polling-expectations
branch
from
74262d2 to
3469f50
Compare
Sources/Testing/Issues/Issue.swift
Outdated
| /// ``confirmation(_:until:within:pollingEvery:isolation:sourceLocation:_:)-5tnlk`` | ||
| /// whenever the polling fails, as described in ``PollingStopCondition``. | ||
| @_spi(Experimental) | ||
| case pollingConfirmationFailed |
There was a problem hiding this comment.
Should we include any associated values with this case? e.g. to inform the console output for this issue kind.
There was a problem hiding this comment.
That seems reasonable to do. Maybe for if an error was thrown, and if the stopping condition fails?
There was a problem hiding this comment.
Here's what I'm thinking right now:
/// A type defining why a Polling Confirmation failed
@_spi(Experimental)
public enum PollingConfirmationFailureReason: Sendable {
/// The polling confirmation failed because the stop condition failed.
///
/// If the stop condition is ``PollingStopCondition.firstPass``, then this
/// this means that the polling confirmation exceeded the number of polling
/// iterations without the `body` closure having returned true or a non-nil
/// value.
///
/// If the stop condition is ``PollingStopCondition.stopsPassing``, then this
/// means that the `body` closure returned false or nil before the polling
/// confirmation reached the number of polling iterations.
case stopConditionFailed(PollingStopCondition)
/// The polling confirmation failed because the `body` closure threw an error.
indirect case errorThrown(_ error: any Error)
}Thoughts?
There was a problem hiding this comment.
If an error is thrown, do we want to treat that as the polling confirmation failing per se, or just a separate issue of kind .errorCaught() and the polling confirmation just doesn't record any sort of result?
Preliminary implementation of polling expectations Make Polling into a function along the lines of Confirmation Additionally, make PollingBehavior an implementation detail of polling, instead of exposed publicly Removes any timeouts involved for polling, as they become increasingly unreliable as the system runs more and more tests Take in configuration arguments, add polling interval Add traits for configuring polling Use consistent naming between confirmAlwaysPasses and the related configuration trait Stop unnecessarily waiting after the last polling attempt has finished. Allow for subsequent polling configuration traits which specified nil for a value to fall back to earlier polling configuration traits before falling back to the default. Add requirePassesEventually and requireAlwaysPasses These two mirror their confirm counterparts, only throwing an error (instead of recording an issue) when they fail. Rewrite confirmPassesEventually when returning an optional to remove the PollingRecorder actor. Now, this uses a separate method for evaluating polling to remove that actor. Clean up the duplicate Poller.evaluate/Poller.poll methods Removed the duplicate poll method, and made evaluate-returning-bool into a wrapper for evaluate-returning-optional Configure polling confirmations as timeout & polling interval This is less direct, but much more intuitive for test authors. Also add exit tests confirming that these values are non-negative Rename to actually use the confirmation name Follow more english-sentence-like guidance for function naming Simplify the polling confirmation API down to just 2 public functions, 1 enum, and 1 error type. Always throw an error when polling fails, get rid of the separate issue recording. Use a single polling confirmation configuration trait Instead of mulitple traits per stop condition, just have a single trait per stop condition.
… confirmation failed
younata
force-pushed
the
polling-expectations
branch
from
3469f50 to
d2f979a
Compare
|
|
||
| /// Default values for polling confirmations. | ||
| @available(_clockAPI, *) | ||
| internal let defaultPollingConfiguration = ( |
There was a problem hiding this comment.
Can this be private?
| internal let defaultPollingConfiguration = ( | |
| private let _defaultPollingConfiguration = ( |
There was a problem hiding this comment.
New files, such as this one and PollingConfigurationTrait.swift, need to be added to the CMake scripts. Can you be sure to add them there? See the file list in Sources/Testing/CMakeLists.txt.
There was a problem hiding this comment.
In terms of the placement of this new file, I'd like to suggest instead of creating a new Polling/ subdirectory we create a new Confirmations/ subdirectory, add move the existing Confirmation.swift to that new directory, and place this new file there too.
(And if you do take this suggestion and move some files around, be sure to adjust the CMake files accordingly.)
|
|
||
| /// A type describing why polling failed | ||
| @_spi(Experimental) | ||
| public enum PollingFailureReason: Sendable, Codable { |
There was a problem hiding this comment.
I think this PollingFailureReason enum would feel more natural nested within PollingFailedError, and then its name could be simplified to Reason (or fully-qualified, PollingFailedError.Reason).
This implements polling confirmations, as described in ST-NNNN.
Motivation:
Being able to monitor changes in the background is something of immense value. Swift Testing already provides an API for this in the
confirmationapi. However, I've found theconfirmationto be hard to work with at times - it requires you to set up a callback for when something changes, which is not always possible or even the right thing to do. Polling provides a very general approach to monitoring all kinds of changes.Modifications:
This adds a new set of macros for handling polling. A new public enum for the 2 separate polling behaviors, and new types to actually implement polling. All under the experimental spi.
Checklist: