From 8cad92554515b90d2257f8ee610d9874c8f6d8ad Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Fri, 9 Dec 2022 21:11:32 -1000 Subject: [PATCH 1/7] Add tests for Semaphore type --- Tests/AsyncQueueTests/SemaphoreTests.swift | 137 +++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 Tests/AsyncQueueTests/SemaphoreTests.swift diff --git a/Tests/AsyncQueueTests/SemaphoreTests.swift b/Tests/AsyncQueueTests/SemaphoreTests.swift new file mode 100644 index 0000000..f7303f0 --- /dev/null +++ b/Tests/AsyncQueueTests/SemaphoreTests.swift @@ -0,0 +1,137 @@ +// MIT License +// +// Copyright (c) 2022 Dan Federman +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +import XCTest + +final class SemaphoreTests: XCTestCase { + + // MARK: XCTestCase + + override func setUp() async throws { + try await super.setUp() + + systemUnderTest = Semaphore() + } + + override func tearDown() async throws { + try await super.tearDown() + + while await systemUnderTest.isWaiting { + await systemUnderTest.signal() + } + } + + // MARK: Behavior Tests + + func test_wait_suspendsUntilEqualNumberOfSignalCalls() async { + /* + This test is tricky to pull off! + Our requirements: + 1. We need to call `wait()` before `signal()` + 2. We need to ensure that the `wait()` call suspends _before_ we call `signal()` + 3. We can't `await` the `wait()` call before calling `signal()` since that would effectively deadlock the test. + + In order to ensure that we are executing the `wait()` calls before we call `signal()` _without awaiting a `wait()` call_, + we utilize an Actor (which has inherently ordered execution) to enqueue ordered `Task`s. We make calls to this actor + `await` the beginning of the `Task` to ensure that each `Task` has begun before resuming the test's execution. + */ + + actor CountingExecutor { + /// Enqueues an asynchronous task. This method suspends the caller until the asynchronous task has begun, ensuring ordered execution of enqueued tasks. + /// - Parameter task: A unit of work that returns work to execute after the task completes and the count is incremented. + func enqueueAndCount(_ task: @escaping @Sendable () async -> (() async -> Void)?) async { + // Await the start of the soon-to-be-enqueued `Task` with a continuation. + await withCheckedContinuation { continuation in + // Re-enter the actor context but don't wait for the result. + Task { + // Now that we're back in the actor context, resume the calling code. + continuation.resume() + let executeAfterIncrement = await task() + countedTasksCompleted += 1 + await executeAfterIncrement?() + } + } + } + + func execute(_ task: @Sendable () async throws -> Void) async rethrows { + try await task() + } + + var countedTasksCompleted = 0 + } + + let executor = CountingExecutor() + let iterationCount = 1_000 + + for _ in 1...iterationCount { + await executor.enqueueAndCount { + let didSuspend = await self.systemUnderTest.wait() + XCTAssertTrue(didSuspend) + + return { + // Signal that the suspended wait call above has resumed. + // This signal allows us to `wait()` for all of these enqueued `wait()` tasks to have completed later in this test. + await self.systemUnderTest.signal() + } + } + } + + // Loop one fewer than iterationCount. + for _ in 1.. Date: Sun, 8 Jan 2023 18:22:55 -0800 Subject: [PATCH 2/7] Utilize a single actor rather than two actors to increase determinism --- Tests/AsyncQueueTests/SemaphoreTests.swift | 80 ++++++++++++---------- 1 file changed, 44 insertions(+), 36 deletions(-) diff --git a/Tests/AsyncQueueTests/SemaphoreTests.swift b/Tests/AsyncQueueTests/SemaphoreTests.swift index f7303f0..239a55a 100644 --- a/Tests/AsyncQueueTests/SemaphoreTests.swift +++ b/Tests/AsyncQueueTests/SemaphoreTests.swift @@ -49,78 +49,56 @@ final class SemaphoreTests: XCTestCase { 1. We need to call `wait()` before `signal()` 2. We need to ensure that the `wait()` call suspends _before_ we call `signal()` 3. We can't `await` the `wait()` call before calling `signal()` since that would effectively deadlock the test. + 4. We must utilize a single actor's isolated context to avoid accidental interleaving when suspending to communicate across actor contexts. In order to ensure that we are executing the `wait()` calls before we call `signal()` _without awaiting a `wait()` call_, we utilize an Actor (which has inherently ordered execution) to enqueue ordered `Task`s. We make calls to this actor `await` the beginning of the `Task` to ensure that each `Task` has begun before resuming the test's execution. */ - actor CountingExecutor { - /// Enqueues an asynchronous task. This method suspends the caller until the asynchronous task has begun, ensuring ordered execution of enqueued tasks. - /// - Parameter task: A unit of work that returns work to execute after the task completes and the count is incremented. - func enqueueAndCount(_ task: @escaping @Sendable () async -> (() async -> Void)?) async { - // Await the start of the soon-to-be-enqueued `Task` with a continuation. - await withCheckedContinuation { continuation in - // Re-enter the actor context but don't wait for the result. - Task { - // Now that we're back in the actor context, resume the calling code. - continuation.resume() - let executeAfterIncrement = await task() - countedTasksCompleted += 1 - await executeAfterIncrement?() - } - } - } - - func execute(_ task: @Sendable () async throws -> Void) async rethrows { - try await task() - } - - var countedTasksCompleted = 0 - } - - let executor = CountingExecutor() let iterationCount = 1_000 + /// A counter that will only be accessed from within the `systemUnderTest`'s context + let unsafeCounter = UnsafeCounter() for _ in 1...iterationCount { - await executor.enqueueAndCount { - let didSuspend = await self.systemUnderTest.wait() + await systemUnderTest.enqueueAndCount(using: unsafeCounter) { systemUnderTest in + let didSuspend = await systemUnderTest.wait() XCTAssertTrue(didSuspend) - return { + return { systemUnderTest in // Signal that the suspended wait call above has resumed. // This signal allows us to `wait()` for all of these enqueued `wait()` tasks to have completed later in this test. - await self.systemUnderTest.signal() + systemUnderTest.signal() } } } // Loop one fewer than iterationCount. for _ in 1.. ((isolated Semaphore) -> Void)?) async { + // Await the start of the soon-to-be-enqueued `Task` with a continuation. + await withCheckedContinuation { continuation in + // Re-enter the actor context but don't wait for the result. + Task { + // Now that we're back in the actor context, resume the calling code. + continuation.resume() + let executeAfterIncrement = await task(self) + counter.countedTasksCompleted += 1 + executeAfterIncrement?(self) + } + } + } + + func execute(_ task: @Sendable (isolated Semaphore) async throws -> Void) async rethrows { + try await task(self) + } +} + +// MARK: - UnsafeCounter + +private final class UnsafeCounter: @unchecked Sendable { + var countedTasksCompleted = 0 +} From 8969a8bed06119895f51f80e0bc3aa098ff84017 Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Sun, 8 Jan 2023 18:34:15 -0800 Subject: [PATCH 3/7] Ensure 100% coverage of all code, including test code --- Tests/AsyncQueueTests/SemaphoreTests.swift | 5 ++--- codecov.yml | 4 ---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Tests/AsyncQueueTests/SemaphoreTests.swift b/Tests/AsyncQueueTests/SemaphoreTests.swift index 239a55a..8d1d0b3 100644 --- a/Tests/AsyncQueueTests/SemaphoreTests.swift +++ b/Tests/AsyncQueueTests/SemaphoreTests.swift @@ -35,9 +35,8 @@ final class SemaphoreTests: XCTestCase { override func tearDown() async throws { try await super.tearDown() - while await systemUnderTest.isWaiting { - await systemUnderTest.signal() - } + let isWaiting = await systemUnderTest.isWaiting + XCTAssertFalse(isWaiting) } // MARK: Behavior Tests diff --git a/codecov.yml b/codecov.yml index d563b48..e4647b5 100644 --- a/codecov.yml +++ b/codecov.yml @@ -12,7 +12,3 @@ coverage: default: threshold: 0.25% patch: off - -ignore: - # This package is not shipped. - - "Tests" From 1c8031fd330f5a8514b9411eaaf5979b8fd07ead Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Sun, 8 Jan 2023 18:40:03 -0800 Subject: [PATCH 4/7] Be explicit about wanting 100% coverage --- codecov.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codecov.yml b/codecov.yml index e4647b5..cc762d7 100644 --- a/codecov.yml +++ b/codecov.yml @@ -10,5 +10,5 @@ coverage: status: project: default: - threshold: 0.25% + target: 100% patch: off From ed28b2c3c09457dddb5914c84f88d40801ca0d8c Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 7 Feb 2023 19:12:11 -0800 Subject: [PATCH 5/7] Call super.tearDown() last Co-authored-by: Michael Bachand --- Tests/AsyncQueueTests/SemaphoreTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/AsyncQueueTests/SemaphoreTests.swift b/Tests/AsyncQueueTests/SemaphoreTests.swift index 8d1d0b3..ff88728 100644 --- a/Tests/AsyncQueueTests/SemaphoreTests.swift +++ b/Tests/AsyncQueueTests/SemaphoreTests.swift @@ -33,10 +33,10 @@ final class SemaphoreTests: XCTestCase { } override func tearDown() async throws { - try await super.tearDown() - let isWaiting = await systemUnderTest.isWaiting XCTAssertFalse(isWaiting) + + try await super.tearDown() } // MARK: Behavior Tests From 4a2848eafccdf35a31adaee20db7576db1da5d6f Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 7 Feb 2023 19:24:18 -0800 Subject: [PATCH 6/7] Clean up comment --- Tests/AsyncQueueTests/SemaphoreTests.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/AsyncQueueTests/SemaphoreTests.swift b/Tests/AsyncQueueTests/SemaphoreTests.swift index ff88728..e507780 100644 --- a/Tests/AsyncQueueTests/SemaphoreTests.swift +++ b/Tests/AsyncQueueTests/SemaphoreTests.swift @@ -51,8 +51,7 @@ final class SemaphoreTests: XCTestCase { 4. We must utilize a single actor's isolated context to avoid accidental interleaving when suspending to communicate across actor contexts. In order to ensure that we are executing the `wait()` calls before we call `signal()` _without awaiting a `wait()` call_, - we utilize an Actor (which has inherently ordered execution) to enqueue ordered `Task`s. We make calls to this actor - `await` the beginning of the `Task` to ensure that each `Task` has begun before resuming the test's execution. + we utilize the Sempahore's ordered execution context to enqueue ordered `Task`s similar to how an ActorQueue works. */ let iterationCount = 1_000 From 519e1ed927c7c66e9cc43c74278c78aeff7bb6ca Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Tue, 7 Feb 2023 19:27:51 -0800 Subject: [PATCH 7/7] clean up wording --- Tests/AsyncQueueTests/SemaphoreTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AsyncQueueTests/SemaphoreTests.swift b/Tests/AsyncQueueTests/SemaphoreTests.swift index e507780..a2c8ebc 100644 --- a/Tests/AsyncQueueTests/SemaphoreTests.swift +++ b/Tests/AsyncQueueTests/SemaphoreTests.swift @@ -47,7 +47,7 @@ final class SemaphoreTests: XCTestCase { Our requirements: 1. We need to call `wait()` before `signal()` 2. We need to ensure that the `wait()` call suspends _before_ we call `signal()` - 3. We can't `await` the `wait()` call before calling `signal()` since that would effectively deadlock the test. + 3. We can't `await` the `wait()` call on the test's queue before calling `signal()` since that would deadlock the test. 4. We must utilize a single actor's isolated context to avoid accidental interleaving when suspending to communicate across actor contexts. In order to ensure that we are executing the `wait()` calls before we call `signal()` _without awaiting a `wait()` call_,