From 9bdff6ea9d8a8f0edaccbc6a8290371244db2c84 Mon Sep 17 00:00:00 2001 From: Brandon Williams <135203+mbrandonw@users.noreply.github.com> Date: Mon, 7 Aug 2023 16:21:22 -0400 Subject: [PATCH 1/6] Fix problem in readme. (#5) --- README.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5922503..ac43d7c 100644 --- a/README.md +++ b/README.md @@ -198,7 +198,9 @@ flips to `true` and then `false`. You might hope that it is as easy as this: ```swift func testIsLoading() async { - let model = NumberFactModel(getFact: { "\($0) is a good number" }) + let model = NumberFactModel(getFact: { + "\($0) is a good number." + }) let task = Task { await model.getFactButtonTapped() } XCTAssertEqual(model.isLoading, true) @@ -219,7 +221,9 @@ called and the moment the request finishes by using a `Task.yield`: ```diff func testIsLoading() async { - let model = NumberFactModel(getFact: { "\($0) is a good number" }) + let model = NumberFactModel(getFact: { + "\($0) is a good number." + }) let task = Task { await model.getFactButtonTapped() } + await Task.yield() @@ -234,12 +238,17 @@ called and the moment the request finishes by using a `Task.yield`: But that still fails the vast majority of times. -These problems, and more, can be fixed by running this entire test on the main serial executor: +These problems, and more, can be fixed by running this entire test on the main serial executor. +You will also have insert a small yield in the `getFact` endpoint due to Swift's ability to +inline async closures that do not actually perform async work: ```diff func testIsLoading() async { + await withMainSerialExecutor { - let model = NumberFactModel(getFact: { "\($0) is a good number" }) + let model = NumberFactModel(getFact: { ++ await Task.yield() + return "\($0) is a good number." + }) let task = Task { await model.getFactButtonTapped() } await Task.yield() @@ -253,7 +262,7 @@ These problems, and more, can be fixed by running this entire test on the main s } ``` -That one change makes this test pass deterministically, 100% of the time. +That small change makes this test pass deterministically, 100% of the time. ## Documentation From efa7d76bca9f0dad4b0633b3a7bf41c851970ffa Mon Sep 17 00:00:00 2001 From: Brandon Williams <135203+mbrandonw@users.noreply.github.com> Date: Wed, 16 Aug 2023 15:24:45 -0400 Subject: [PATCH 2/6] Article on reliably testing async code. (#6) --- README.md | 1 - .../Documentation.docc/ConcurrencyExtras.md | 1 + .../ReliablyTestingAsync.md | 265 ++++++++++++++++++ .../MainSerialExecutor.swift | 3 + 4 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 Sources/ConcurrencyExtras/Documentation.docc/ReliablyTestingAsync.md diff --git a/README.md b/README.md index ac43d7c..70ff43d 100644 --- a/README.md +++ b/README.md @@ -264,7 +264,6 @@ inline async closures that do not actually perform async work: That small change makes this test pass deterministically, 100% of the time. - ## Documentation The latest documentation for this library is available [here][concurrency-extras-docs]. diff --git a/Sources/ConcurrencyExtras/Documentation.docc/ConcurrencyExtras.md b/Sources/ConcurrencyExtras/Documentation.docc/ConcurrencyExtras.md index c14f657..d4da447 100644 --- a/Sources/ConcurrencyExtras/Documentation.docc/ConcurrencyExtras.md +++ b/Sources/ConcurrencyExtras/Documentation.docc/ConcurrencyExtras.md @@ -243,6 +243,7 @@ need to make weaker assertions due to non-determinism, but can still assert on s ### Serial execution +- - ``withMainSerialExecutor(operation:)-79jpc`` ### Preconcurrency diff --git a/Sources/ConcurrencyExtras/Documentation.docc/ReliablyTestingAsync.md b/Sources/ConcurrencyExtras/Documentation.docc/ReliablyTestingAsync.md new file mode 100644 index 0000000..036b47c --- /dev/null +++ b/Sources/ConcurrencyExtras/Documentation.docc/ReliablyTestingAsync.md @@ -0,0 +1,265 @@ +# Reliably testing async code + +Learn how to use the tools of this library to write reliable, deterministic tests for your async +Swift code. + +## Overview + +Swift 5.5 brought first class support for concurrency to the language, including lightweight syntax +for describing when functions and methods need to perform async work, a new data type for +isolating mutable data, and all new APIs for performing non-blocking asynchronous work. This made it +far easier to write async code than ever before, but it also made testing asynchronous code quite +a bit more complicated. + +Join us for a quick overview of what tools Swift gives us today for testing asynchronous code, as +well as examples of how these tools can fall short, and then how to fix them. + +> This article is a brief recap of a [series of +episodes](https://www.pointfree.co/collections/concurrency/testing-async-code) on Point-Free that +goes very deep into how to reliably test async code in Swift. + +## Async testing tools of today + +The primary tool for testing async code today is XCTest's support for async test cases. Simply mark +your test method as `async` and then you are free to perform any async work you want: + +```swift +class FeatureTests: XCTestCase { + func testBasics() async { + … + } +} +``` + +This makes it easy to invoke an async function or method and then assert on what changed after +the work finished. + +For example, suppose we had a very simple observable object for encapsulating a number that could +be incremented and decremented from the UI, as well as the ability to fetch a fact about the +number. The mechanism for fetching the fact should be hidden behind some kind of interface, like +a protocol, but for now we will pass it as an explicit closure to the model. + +Further, to make things interesting, we will also manage a piece of boolean state that tracks +whether or not the fact is currently loading so that we can display a progress indicator in the +view: + +```swift +@MainActor +@Observable +class FeatureModel { + var count = 0 + var fact: String? + var isLoadingFact = false + + // NB: Can hide this closure behind an interface and use some sort of dependency + // injection to provide it. + let numberFact: (Int) async throws -> String + init(numberFact: @escaping (Int) async throws -> String) { + self.numberFact = numberFact + } + + func getFactButtonTapped() async { + self.isLoadingFact = true + defer { self.isLoadingFact = false } + + do { + self.fact = try await self.numberFact(self.count) + } catch { + // TODO: Handle error + } + } +} +``` + +This model seems simple enough, yet it can be surprisingly tricky to test all aspects of it. + +The easiest part of the model to test is that the `fact` state is populated eventually after the +"Get fact" button is tapped. That can be done simply thanks to the support for async in tests, and +by using some kind of "mock" version of the number fact closure that returns a response immediately +rather than making a network request: + +```swift +@MainActor +func testGetFact() async { + let model = FeatureModel(numberFact: { number in + "\(number) is a good number!" + }) + + await model.getFactButtonTapped() + XCAssertEqual(model.fact, "0 is a good number!") +} +``` + +This test will pass 100% of the time, and do so very quickly. And that's great! + +What's not so great is that it's not really possible to test that the `isLoadingFact` state flips +from `false` to `true` and then back to `false`. At least when using the tools that Swift gives us +today for testing async code. + +First of all, naively asserting on `isLoadingFact` right after invoking `getFactButtonTapped` can't +possibly work because the async work has already finished by that point: + +```swift +await model.getFactButtonTapped() +XCTAssertEqual(model.isLoadingFact, true) // ❌ +XCAssertEqual(model.fact, "0 is a good number!") +XCTAssertEqual(model.isLoadingFact, false) +``` + +So what we need to do is run `getFactButtonTapped` in an unstructured `Task` so that it can run +in parallel with the rest of the test. That should allow us to wiggle ourselves in between the +moment the boolean state flips to `true` and then `false`: + +```swift +let task = Task { await model.getFactButtonTapped() } +XCTAssertEqual(model.isLoadingFact, true) // ❌ +await task.value +XCAssertEqual(model.fact, "0 is a good number!") +XCTAssertEqual(model.isLoadingFact, false) +``` + +However this fails the vast majority of times. Over 99% of the time. It seems that _every_ once in +awhile the `Task` starts up fast enough to flip the boolean to `true`, but that is a rare exception +rather than the rule. + +What we really need to do is wait a _little_ bit of time for the `Task` to start executing its code, +but not _too_ much time so that it finishes. Perhaps a single `Task.yield` will help: + +```swift +let task = Task { await model.getFactButtonTapped() } +await Task.yield() +XCTAssertEqual(model.isLoadingFact, true) // ❌ +await task.value +XCAssertEqual(model.fact, "0 is a good number!") +XCTAssertEqual(model.isLoadingFact, false) +``` + +Unfortunately this fails too, and it does so the vast majority of the time. + +And this is only one small example of async code that is difficult to test. If your async code +tries to implement cancellation, or makes use of time-based asynchrony (such as clocks), or +uses async sequences, or any number of things, then you will come across similar test failures that +are essentially impossible to fix. You may be able to even get the tests to seemingly pass +consistently, but almost alwasy if you run them enough times (thousands or millions of times), you +_will_ eventually get a test failure, and that breeds uncertainty in your test suite. + +## Looking to Async Algorithms for inspiration + +So, what are we to do? + +The problem with testing this kind of async code in Swift is that we have no way to predict how +the runtime will schedule and execute work. And that is fine when running the code in production, +but we don't need that complexity for tests. Most tests are verifying a very simple state machine +of actions: the user performs a few actions, one after another, and we assert at each step of the +way how the state of our feature changes. + +In such situations we don't need the full power of a complex scheduling machine that manages a small +pool of threads. It would be completely sufficient to serialize all async work to a single thread. +That does not mean that multiple concurrent tasks are not able to interleave. Suspension of async +tasks can still work as you expect, but all actual work is run serially on a single thread. + +And interestingly, there is even a precendent for this in one of Apple's open source Swift +libraries! The [Async Algorithms][async-algos-gh] package comes with an +[`AsyncSequenceValidation`][async-algos-validate-library] library with tools specifically designed +to make testing async code a deterministic process. It needs this tool in order to write reliable, +deterministic tests for its various operators, such as `debounce`, `throttle`, and more. + +The way it accomplishes this is by [overriding the global enqueue hook][async-algos-hook-override] +that Swift uses when new asynchronous tasks are created. And that hook is publicly exposed to us +from Swift's actual C++ codebase, which we can see by looking at its [headers][swift-global-hook]. +The async algorithms package uses that global hook to serialize all async work to a single queue +rather than let the test be susceptible to the vagaries of the global concurrent executor, allowing +it to write tests that pass deterministically, 100% of the time. + +## How to test async code reliably? + +And so if Apple can write tests like this, why can't we? + +Well, now we can thanks to the ``withMainSerialExecutor(operation:)-79jpc`` tool that ships with +this library. It temporarily alters the manner in which Swift enqueues asynchronous work in order +to serialize it to the main thread. This allows you to test every facet of the async code, including +what happens between each suspension point, in a manner that is 100% deterministic. + +For example, the previous test we wrote, which passed sometimes but failed most of the times, can +now be written in a way that passes 100% of the time: + +```swift +func testGetFact() async { + await withMainSerialExecutor { + let model = FeatureModel(numberFact: { number in + await Task.yield() + return "\(number) is a good number!" + }) + + let task = Task { await model.getFactButtonTapped() } + await Task.yield() + XCTAssertEqual(model.isLoadingFact, true) // ✅ + await task.value + XCAssertEqual(model.fact, "0 is a good number!") // ✅ + XCTAssertEqual(model.isLoadingFact, false) // ✅ + } +} +``` + +Because all enqueueing of work has been serialized we can be guaranteed that when we +`await Task.yield()` in the test, all work that is suspended will have an opportunity to execute +before execution is returned to our test. This means this test is guaranteed to always pass. + +You can even override the `invokeTest` method in your test case to force every test to run on the +main serial executor: + +```swift +override func invokeTest() { + withMainSerialExecutor { + super.invokeTest() + } +} +``` + +This tool allows you to finally write tests against complex and subtle async code that you can be +confident in. No more seeing mysterious test failures on CI and wasting hours of CI time re-running +tests or hours of developer time investigating if they are true errors or simply flakiness in the +async scheduling. + +## Testing reality + +Note that by using ``withMainSerialExecutor(operation:)-79jpc`` you are technically making your +tests behave in a manner that is different from how they would run in production. However, many +tests written on a day-to-day basis do not invoke the full-blown vagaries of concurrency. Instead, +tests often want to assert that when some user action happens, an async unit of work is executed, +and that causes some state to change. Such tests should be written in a way that is 100% +deterministic. And even Apple agrees in their [documentation of +`AsyncSequenceValidation`][async-validation-md] where they justify why they think their manner of +testing async sequences truly does test reality even though they are altering the runtime that +schedules async work (emphasis ours): + +> Quote from AsyncAlgorithms: _Testing is a critical area of focus for any package to make it +robust, catch bugs, and explain the expected behaviors in a documented manner. Testing things that +are asynchronous can be difficult, testing things that are asynchronous multiple times can be even +more difficult._ +> +> _Types that implement AsyncSequence **can often be described in deterministic actions given +particular inputs**. For the inputs, the events can be described as a discrete set: values, errors +being thrown, the terminal state of returning a nil value from the iterator, or advancing in time +and not doing anything. Likewise, the expected output has a discrete set of events: values, errors +being caught, the terminal state of receiving a nil value from the iterator, or advancing in time +and not doing anything._ + +Just as async sequences can often be described with a determinstic sequences of inputs that lead to +a deterministic sequence of outputs, the same is true of user actions in an application. And so we +too feel that many of the tests we write on a daily basis can be run inside +``withMainSerialExecutor(operation:)-79jpc`` and that we are not weakening the strength of those +tests in the least. + +However, if your code has truly complex asynchronous and concurrent operations, then it may be handy +to write two sets of tests: one set that targets the main executor (using ``withMainSerialExecutor(operation:)-79jpc``) +so that you can deterministically assert how the core system behaves, and then another set that +targets the default, global executor. The latter tests will probably need to make weaker assertions +due to non-determinism, but can still assert on some things. + + +[async-algos-gh]: https://github.com/apple/swift-async-algorithms +[async-algos-validate-library]: https://github.com/apple/swift-async-algorithms/tree/07a0c1ee08e90dd15b05d45a3ead10929c0b7ec5/Sources/AsyncSequenceValidation +[async-validation-md]: https://github.com/apple/swift-async-algorithms/blob/07a0c1ee08e90dd15b05d45a3ead10929c0b7ec5/Sources/AsyncSequenceValidation/AsyncSequenceValidation.docc/AsyncSequenceValidation.md +[async-algos-hook-override]: https://github.com/apple/swift-async-algorithms/blob/07a0c1ee08e90dd15b05d45a3ead10929c0b7ec5/Sources/AsyncSequenceValidation/Test.swift#L319-L321 +[swift-global-hook]: https://github.com/apple/swift/blob/e89de6e7e0952c3d0485cc07129ec17f2763c12f/include/swift/Runtime/Concurrency.h#L734-L738 diff --git a/Sources/ConcurrencyExtras/MainSerialExecutor.swift b/Sources/ConcurrencyExtras/MainSerialExecutor.swift index b86eba7..bee4475 100644 --- a/Sources/ConcurrencyExtras/MainSerialExecutor.swift +++ b/Sources/ConcurrencyExtras/MainSerialExecutor.swift @@ -15,6 +15,9 @@ /// } /// ``` /// + /// See for more information on why this tool is needed to test + /// async code and how to use it. + /// /// > Warning: This API is only intended to be used from tests to make them more reliable. Please do /// > not use it from application code. /// > From 0b9d2b5768bdacac17f3201bdb9f570e9a9ebedc Mon Sep 17 00:00:00 2001 From: mbrandonw Date: Wed, 16 Aug 2023 19:32:03 +0000 Subject: [PATCH 3/6] Run swift-format --- Sources/ConcurrencyExtras/MainSerialExecutor.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ConcurrencyExtras/MainSerialExecutor.swift b/Sources/ConcurrencyExtras/MainSerialExecutor.swift index bee4475..006e025 100644 --- a/Sources/ConcurrencyExtras/MainSerialExecutor.swift +++ b/Sources/ConcurrencyExtras/MainSerialExecutor.swift @@ -17,7 +17,7 @@ /// /// See for more information on why this tool is needed to test /// async code and how to use it. - /// + /// /// > Warning: This API is only intended to be used from tests to make them more reliable. Please do /// > not use it from application code. /// > From 15c0b09d3d383052cecf610d81a88f73d3e16204 Mon Sep 17 00:00:00 2001 From: John Flanagan <91548783+jflan-dd@users.noreply.github.com> Date: Tue, 19 Sep 2023 10:10:50 -0500 Subject: [PATCH 4/6] Fix parameter doc comments (#8) --- Sources/ConcurrencyExtras/ActorIsolated.swift | 2 +- Sources/ConcurrencyExtras/LockIsolated.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/ConcurrencyExtras/ActorIsolated.swift b/Sources/ConcurrencyExtras/ActorIsolated.swift index 94aac7f..7609cbd 100644 --- a/Sources/ConcurrencyExtras/ActorIsolated.swift +++ b/Sources/ConcurrencyExtras/ActorIsolated.swift @@ -79,7 +79,7 @@ public final actor ActorIsolated { /// > await didOpenSettings.withValue { XCTAssertTrue($0) } /// > ``` /// - /// - Parameters: operation: An operation to be performed on the actor with the underlying value. + /// - Parameter operation: An operation to be performed on the actor with the underlying value. /// - Returns: The result of the operation. public func withValue( _ operation: @Sendable (inout Value) throws -> T diff --git a/Sources/ConcurrencyExtras/LockIsolated.swift b/Sources/ConcurrencyExtras/LockIsolated.swift index cce26be..8b53bc9 100644 --- a/Sources/ConcurrencyExtras/LockIsolated.swift +++ b/Sources/ConcurrencyExtras/LockIsolated.swift @@ -36,7 +36,7 @@ public final class LockIsolated: @unchecked Sendable { /// } /// ``` /// - /// - Parameters: operation: An operation to be performed on the the underlying value with a lock. + /// - Parameter operation: An operation to be performed on the the underlying value with a lock. /// - Returns: The result of the operation. public func withValue( _ operation: (inout Value) throws -> T From eadb1cbcca4fd6f5c817bffc56e15b9c98a0fe47 Mon Sep 17 00:00:00 2001 From: Raphael Silva <5387088+peagasilva@users.noreply.github.com> Date: Mon, 16 Oct 2023 14:26:47 +0200 Subject: [PATCH 5/6] Add missing word to README.md sentence (#9) While reading it, I noticed a missing `be` in the sentence. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 70ff43d..516794e 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ The library comes with numerous helper APIs spread across the two Swift stream t Use `eraseToThrowingStream()` to propagate failures from throwing async sequences. - * Swift 5.9's `makeStream(of:)` functions have been back-ported. It can handy in tests that need + * Swift 5.9's `makeStream(of:)` functions have been back-ported. It can be handy in tests that need to override a dependency endpoint that returns a stream: ```swift From 6155400cb15b0d99b2c257d57603d61d03a817a8 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Thu, 26 Oct 2023 14:34:18 -0700 Subject: [PATCH 6/6] Lock async stream inits (#11) * Lock async stream inits Fixes #10. * wip --- Sources/ConcurrencyExtras/AsyncStream.swift | 9 +++++++-- Sources/ConcurrencyExtras/AsyncThrowingStream.swift | 9 +++++++-- Sources/ConcurrencyExtras/Internal/Locking.swift | 11 +++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 Sources/ConcurrencyExtras/Internal/Locking.swift diff --git a/Sources/ConcurrencyExtras/AsyncStream.swift b/Sources/ConcurrencyExtras/AsyncStream.swift index b8d4ad0..6dfa2ea 100644 --- a/Sources/ConcurrencyExtras/AsyncStream.swift +++ b/Sources/ConcurrencyExtras/AsyncStream.swift @@ -1,3 +1,5 @@ +import Foundation + extension AsyncStream { /// Produces an `AsyncStream` from an `AsyncSequence` by consuming the sequence till it /// terminates, ignoring any failure. @@ -51,10 +53,13 @@ extension AsyncStream { /// /// - Parameter sequence: An async sequence. public init(_ sequence: S) where S.Element == Element { + let lock = NSLock() var iterator: S.AsyncIterator? self.init { - if iterator == nil { - iterator = sequence.makeAsyncIterator() + lock.withLock { + if iterator == nil { + iterator = sequence.makeAsyncIterator() + } } return try? await iterator?.next() } diff --git a/Sources/ConcurrencyExtras/AsyncThrowingStream.swift b/Sources/ConcurrencyExtras/AsyncThrowingStream.swift index 14fd40c..829b0ae 100644 --- a/Sources/ConcurrencyExtras/AsyncThrowingStream.swift +++ b/Sources/ConcurrencyExtras/AsyncThrowingStream.swift @@ -1,13 +1,18 @@ +import Foundation + extension AsyncThrowingStream where Failure == Error { /// Produces an `AsyncThrowingStream` from an `AsyncSequence` by consuming the sequence till it /// terminates, rethrowing any failure. /// /// - Parameter sequence: An async sequence. public init(_ sequence: S) where S.Element == Element { + let lock = NSLock() var iterator: S.AsyncIterator? self.init { - if iterator == nil { - iterator = sequence.makeAsyncIterator() + lock.withLock { + if iterator == nil { + iterator = sequence.makeAsyncIterator() + } } return try await iterator?.next() } diff --git a/Sources/ConcurrencyExtras/Internal/Locking.swift b/Sources/ConcurrencyExtras/Internal/Locking.swift new file mode 100644 index 0000000..a97a9d0 --- /dev/null +++ b/Sources/ConcurrencyExtras/Internal/Locking.swift @@ -0,0 +1,11 @@ +import Foundation + +#if !(os(iOS) || os(macOS) || os(tvOS) || os(watchOS)) + extension NSLock { + func withLock(_ body: () throws -> R) rethrows -> R { + self.lock() + defer { self.unlock() } + return try body() + } + } +#endif