…ledOrNotScheduledAnymore
# Motivation
This task was flaky and hit the assertion with the count being 1
# Modification
This changes the expectation to greater than 0 since we can always
guarantee that one scheduled task is run but due to timing windows it
might be that we shutdown the EL before the second task is enqueued
which meant it was immediately failed.
# Result
One less flaky test
Motivation:
NIOEmbedded is used all over NIO-land for testing various pieces of the
infrastructure, and so requires a substantial audit for strict
concurrency.
Modifications:
- Mark a few things Sendable.
- Fix the tests, which actually did have some nasty bugs
Result:
Sendable-clean NIOEmbedded
### Motivation
Code that tries to work with `NIODeadline` can be challenging to test
using `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` because they
have their own, fake clock.
These testing event loops do implement `scheduleTask(in:_)` to submit
work in the future, relative to the event loop clock, but there is no
way to get the event loop's current notion of "now", as a `NIODeadline`,
and users sometimes find that `NIODeadline.now`, which returns the time
of the real clock, to be surprising.
### Modifications
Add `EventLoop.now` to get the current time of the event loop clock.
### Result
New APIs to support writing code that's easier to test.
### Motivation:
We're seeing some very rare failures from this test in CI runs but I
cannot reproduce locally. The test relies on timing to some degree and
the interaction of scheduling tasks at shutdown.
### Modifications:
The `AsyncTestingEventLoop` has an `executeInContext` function which
puts the closure on the backing dispatch queue and blocks. In this
instance it might be a good way to make sure that all the previous work
has happened.
### Result:
(Hopefully) less flakey test.
Motivation:
NIOAsyncTestingEventLoopTests.testTasksScheduledDuringShutdownAreAutomaticallyCancelled
is flaky.
The recursivesly schedules tasks to run on the event loop and then,
after a small pause, shuts down the event loop. It then asserts that
more then 1 task was scheduled (i.e. at least 1 recursive task was run).
This assertion occasionally fails as exactly 1 task was run.
I haven't been able to reproduce this locally but I believe the root of
the flakiness is that the child tasks to shutdown the loop and advance
the time (to trigger the recursive scheduling) can race. If the shutdown
wins then only the root task will run.
Modifications:
- Remove the array of scheduled tasks, it wasn't being used
- Fix atomic ordering
- Increase sleep time before shutting down
- Advance time in the task group rather than a child task
Result:
Less flaky test (hopefully)
### Motivation
Some tests were making use of `Task.sleep(for:)` which isn't present on older platforms.
```
/workspace/Tests/NIOEmbeddedTests/AsyncTestingEventLoopTests.swift:505:32: error: 'sleep(for:tolerance:clock:)' is only available in tvOS 16.0 or newer
try await Task.sleep(for: .milliseconds(1))
```
### Modifications
Update tests to use `Task.sleep(nanoseconds:)` via `TimeAmount.nanoseconds` instead, which is available on those platforms.
### Result
Can build tests on older platforms.
### Motivation:
The two testing event loops—`EmbeddedEventLoop` and `NIOAsyncTestingEventLoop`—have different semantics for outstanding work during shutdown, which are both different from the production `SelectableEventLoop`, specifically with newly scheduled tasks that result from running existing scheduled work at the time of shutdown.
There are three axes to consider:
1. Scheduled tasks that are at or past their deadline.
2. Scheduled tasks with a deadline in the future.
3. Newly scheduled work that result from either running or cancelling (1) and (2).
| | (1) | (2) | (3) |
|-|-|-|-|
| `SelectableEventLoop` | Run | Cancel | Quiesce over 1000 ticks, repeatedly run resulting (1) and cancel resulting (2) |
| `EmbeddedEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
| `NIOAsyncTestingEventLoop` | Run | Run | Run resulting (1) and resulting (2) |
Note that `NIOAsyncTestingEventLoop` may never terminate because of this.
### Modifications:
This PR aligns `EmbeddedEventLoop` and `NIOTestingEventLoop` and makes them more similar to `SelectableEventLoop` and less surprising semantics.
### Result:
| | (1) | (2) | (3) |
|-|-|-|-|
| `SelectableEventLoop` | Run | Cancel | Quiesce over 1000 ticks, repeatedly run resulting (1) and cancel resulting (2) |
| `EmbeddedEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
| `NIOAsyncTestingEventLoop` | Run | Cancel | Cancel resulting (1) and resulting (2) |
### Future work:
Given both the `EmbeddedEventLoop` and `NIOAsyncTestingEventLoop` are used in tests of NIO applications that run with `SelectableEventLoop` in production, it might be worth extending both to also support quiescing to give a more representative shutdown behaviour in tests.
* Apply formatting
* Apply no block comments rule
* Apply OmitExplicitReturns
* Apple OnlyOneTrailingClosureArgument
* Apply NoAssignmentInExpressions
* Fix up DontRepeatTypeInStaticProperties lint errors
* Apply `OrderedImports`
* Apply `ReplaceForEachWithForLoop`
* format file
* Enable the formatting pipeline
* Adopt `AmbiguousTrailingClosureOverload`
* Fix license header
* Fix format check
* Fix `EndOfLineComment`
* Fix CI
* Adapt CI script to check if changes when running formatting
* Separate lint and format into to steps
* Fix format
* Adopt `UseEarlyExits`
* Revert "Adopt `UseEarlyExits`"
This reverts commit d1ac5bbe12.
# Motivation
We are currently missing a bunch of annotations in our tests which leads to compilation failures if you build against generic iOS/macOS/watchOS/tvOS
# Modification
This PR adds the missing availability annotations in the tests.
# Motivation
We introduced `XCTAsyncTest` to bridge the gap where some Swift versions did not have `async` test support.
# Modification
This PR removes `XCTAsyncTest` and migrates all the places where we used it to native `async` tests.
# Result
Less custom code.
Motivation:
swift- nio was failing builds that should pass
Modifications:
Adding available to the necessary sections
* Updating test OnToRunClosure
Motivation:
testCancelledScheduledTasksDoNotHoldOnToRunClosure() was not allowed enough time and timing off at moments, causing it to fail occasionally
Modifications:
Added a ConditionLock throughout the code to make sure it only unlocks when the code has waited enough time for it to not hit the precondition failure
* Enhance and rename AsyncEmbeddedEventLoop
Motivation
AsyncEmbeddedEventLoop is an important part of our ongoing testing story
for NIO. However, it suffers from two problems.
The first is a usability one. As we discovered during the original
implementation, following EmbeddedEventLoop's pattern of not having any
EventLoop "Tasks" execute until run() meant that simple constructs like
`EventLoopFuture.wait` and `EventLoopFuture.get` didn't work at all,
forcing us to add an annoying `awaitFuture` method.
When playing with implementing EmbeddedChannel, this got worse, as I/O
methods also don't run immediately if called from the testing thread. We
couldn't easily work around this issue, and it meant that common
patterns (like calling `channel.writeAndFlush`) would deadlock!
This is unacceptable, so a change had to be made.
While we're here, we received feedback that the name is unclear to
users. Given that this particular event loop is in no sense "embedded",
we no longer need the name, so we can take this opportunity to use a
better one.
Modifications
Changed `func execute` to immediately execute its task body, and to
dequeue all pending tasks at this time. Essentially, it's the equivalent
to run(). This is a major change in its behaviour.
Renamed the loop to `NIOAsyncTestingEventLoop`.
Result
Better names, easier to use.
* Make soundness happy
* Remove awaitFuture
Franz Busch
Cory Benfield
Si Beaumont
George Barnett
David Nadoba
carolinacass
Johannes Weiss