Skip to content

Commit

Permalink
Improve testTasksScheduledDuringShutdownAreAutomaticallyCancelled (#2843
Browse files Browse the repository at this point in the history
)

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)
  • Loading branch information
glbrntt authored Aug 14, 2024
1 parent e0deef8 commit 286834d
Showing 1 changed file with 8 additions and 13 deletions.
21 changes: 8 additions & 13 deletions Tests/NIOEmbeddedTests/AsyncTestingEventLoopTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -481,19 +481,16 @@ final class NIOAsyncTestingEventLoopTests: XCTestCase {
func testTasksScheduledDuringShutdownAreAutomaticallyCancelled() async throws {
let eventLoop = NIOAsyncTestingEventLoop()
let tasksRun = ManagedAtomic(0)
var childTasks: [Scheduled<Void>] = []

func scheduleRecursiveTask(
at taskStartTime: NIODeadline,
andChildTaskAfter childTaskStartDelay: TimeAmount
) -> Scheduled<Void> {
eventLoop.scheduleTask(deadline: taskStartTime) {
tasksRun.wrappingIncrement(ordering: .relaxed)
childTasks.append(
scheduleRecursiveTask(
at: eventLoop.now + childTaskStartDelay,
andChildTaskAfter: childTaskStartDelay
)
tasksRun.wrappingIncrement(ordering: .releasing)
_ = scheduleRecursiveTask(
at: eventLoop.now + childTaskStartDelay,
andChildTaskAfter: childTaskStartDelay
)
}
}
Expand All @@ -502,17 +499,15 @@ final class NIOAsyncTestingEventLoopTests: XCTestCase {

try await withThrowingTaskGroup(of: Void.self) { group in
group.addTask {
try await Task.sleep(nanoseconds: 1_000_000)
try await Task.sleep(nanoseconds: 10_000_000)
await eventLoop.shutdownGracefully()
}
group.addTask {
await eventLoop.advanceTime(to: .uptimeNanoseconds(1))
}

await eventLoop.advanceTime(to: .uptimeNanoseconds(1))
try await group.waitForAll()
}

XCTAssertGreaterThan(tasksRun.load(ordering: .relaxed), 1)
XCTAssertEqual(childTasks.count, tasksRun.load(ordering: .relaxed))
XCTAssertGreaterThan(tasksRun.load(ordering: .acquiring), 1)
}

func testShutdownCancelsRemainingScheduledTasks() async {
Expand Down

0 comments on commit 286834d

Please sign in to comment.