From f90aa058732ff9f1d88341d647c8792fe04a4f31 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Tue, 7 May 2024 11:08:20 -0700 Subject: [PATCH 1/3] Have `store.finish()` assert no received actions `store.finish()` should do much of the same work as `store.deinit`, but asynchronously with a timeout. This PR updates things so that folks with long-living test stores (_e.g._ held onto by the test case) have the ability to assert that there are no unreceived actions. --- .../ComposableArchitecture/TestStore.swift | 69 ++++++++++++------- .../Reducers/PresentationReducerTests.swift | 16 +++-- .../Reducers/StackReducerTests.swift | 6 +- .../SharedTests.swift | 2 +- .../TestStoreFailureTests.swift | 24 ++++--- 5 files changed, 76 insertions(+), 41 deletions(-) diff --git a/Sources/ComposableArchitecture/TestStore.swift b/Sources/ComposableArchitecture/TestStore.swift index edf1290bbd3d..c25edf2646de 100644 --- a/Sources/ComposableArchitecture/TestStore.swift +++ b/Sources/ComposableArchitecture/TestStore.swift @@ -573,8 +573,8 @@ public final class TestStore { file: StaticString = #file, line: UInt = #line ) async { + self.assertNoReceivedActions(file: file, line: line) Task.cancel(id: OnFirstAppearID()) - let nanoseconds = nanoseconds ?? self.timeout let start = DispatchTime.now().uptimeNanoseconds await Task.megaYield() @@ -609,6 +609,7 @@ public final class TestStore { } await Task.yield() } + self.assertNoSharedChanges(file: file, line: line) } deinit { @@ -617,23 +618,7 @@ public final class TestStore { } func completed() { - if !self.reducer.receivedActions.isEmpty { - let actions = self.reducer.receivedActions - .map(\.action) - .map { " • " + debugCaseOutput($0, abbreviated: true) } - .joined(separator: "\n") - XCTFailHelper( - """ - The store received \(self.reducer.receivedActions.count) unexpected \ - action\(self.reducer.receivedActions.count == 1 ? "" : "s") by the end of this test: … - - Unhandled actions: - \(actions) - """, - file: self.file, - line: self.line - ) - } + self.assertNoReceivedActions(file: self.file, line: self.line) Task.cancel(id: OnFirstAppearID()) for effect in self.reducer.inFlightEffects { XCTFailHelper( @@ -648,27 +633,59 @@ public final class TestStore { • If using async/await in your effect, it may need a little bit of time to properly \ finish. To fix you can simply perform "await store.finish()" at the end of your test. - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", etc.), \ - make sure that you wait enough time for it to perform the effect. If you are using \ - a test clock/scheduler, advance it so that the effects may complete, or consider \ - using an immediate clock/scheduler to immediately perform the effect instead. + • If an effect uses a clock (or scheduler, via "receive(on:)", "delay", "debounce", etc.), \ + make sure that you wait enough time for it to perform the effect. If you are using a test \ + clock/scheduler, advance it so that the effects may complete, or consider using an \ + immediate clock/scheduler to immediately perform the effect instead. • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ then make sure those effects are torn down by marking the effect ".cancellable" and \ returning a corresponding cancellation effect ("Effect.cancel") from another action, or, \ if your effect is driven by a Combine subject, send it a completion. + + • If you do not wish to assert on these effects, perform "await \ + store.skipInFlightEffects()", or consider using a non-exhaustive test store: \ + "store.exhaustivity = .off". """, file: effect.action.file, line: effect.action.line ) } + self.assertNoSharedChanges(file: self.file, line: self.line) + } + + private func assertNoReceivedActions(file: StaticString, line: UInt) { + if !self.reducer.receivedActions.isEmpty { + let actions = self.reducer.receivedActions + .map(\.action) + .map { " • " + debugCaseOutput($0, abbreviated: true) } + .joined(separator: "\n") + XCTFailHelper( + """ + The store received \(self.reducer.receivedActions.count) unexpected \ + action\(self.reducer.receivedActions.count == 1 ? "" : "s"): … + + Unhandled actions: + \(actions) + + To fix, explicitly assert against these actions using "store.receive", skip these actions \ + by performing "await store.skipReceivedActions()", or consider using a non-exhaustive test \ + store: "store.exhaustivity = .off". + """, + file: file, + line: line + ) + } + } + + private func assertNoSharedChanges(file: StaticString, line: UInt) { // NB: This existential opening can go away if we can constrain 'State: Equatable' at the // 'TestStore' level, but for some reason this breaks DocC. if self.sharedChangeTracker.hasChanges, let stateType = State.self as? any Equatable.Type { func open(_: EquatableState.Type) { let store = self as! TestStore try? store.expectedStateShouldMatch( - preamble: "Test store completed before asserting against changes to shared state", + preamble: "Test store finished before asserting against changes to shared state", postamble: """ Invoke "TestStore.assert" at the end of this test to assert against changes to shared \ state. @@ -677,11 +694,12 @@ public final class TestStore { actual: store.state, updateStateToExpectedResult: nil, skipUnnecessaryModifyFailure: true, - file: store.file, - line: store.line + file: file, + line: line ) } open(stateType) + self.sharedChangeTracker.resetChanges() } } @@ -2594,5 +2612,6 @@ extension TestStore { file: StaticString = #file, line: UInt = #line ) async { + fatalError() } } diff --git a/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift b/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift index 18c6e4367788..dd10124942f8 100644 --- a/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift +++ b/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift @@ -2298,22 +2298,26 @@ final class PresentationReducerTests: BaseTCATestCase { An effect returned for this action is still running. It must complete before the end \ of the test. … - To fix, inspect any effects the reducer returns for this action and ensure that all \ - of them complete by the end of the test. There are a few reasons why an effect may \ - not have completed: + To fix, inspect any effects the reducer returns for this action and ensure that all of \ + them complete by the end of the test. There are a few reasons why an effect may not \ + have completed: • If using async/await in your effect, it may need a little bit of time to properly \ finish. To fix you can simply perform "await store.finish()" at the end of your test. - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ + • If an effect uses a clock (or scheduler, via "receive(on:)", "delay", "debounce", \ etc.), make sure that you wait enough time for it to perform the effect. If you are \ - using a test clock/scheduler, advance it so that the effects may complete, or \ - consider using an immediate clock/scheduler to immediately perform the effect instead. + using a test clock/scheduler, advance it so that the effects may complete, or consider \ + using an immediate clock/scheduler to immediately perform the effect instead. • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ then make sure those effects are torn down by marking the effect ".cancellable" and \ returning a corresponding cancellation effect ("Effect.cancel") from another action, \ or, if your effect is driven by a Combine subject, send it a completion. + + • If you do not wish to assert on these effects, perform "await \ + store.skipInFlightEffects()", or consider using a non-exhaustive test store: \ + "store.exhaustivity = .off". """ } } diff --git a/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift b/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift index dc558aaabd1b..e656ccf58c56 100644 --- a/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift +++ b/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift @@ -899,7 +899,7 @@ • If using async/await in your effect, it may need a little bit of time to properly \ finish. To fix you can simply perform "await store.finish()" at the end of your test. - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ + • If an effect uses a clock (or scheduler, via "receive(on:)", "delay", "debounce", \ etc.), make sure that you wait enough time for it to perform the effect. If you are \ using a test clock/scheduler, advance it so that the effects may complete, or \ consider using an immediate clock/scheduler to immediately perform the effect instead. @@ -908,6 +908,10 @@ then make sure those effects are torn down by marking the effect ".cancellable" and \ returning a corresponding cancellation effect ("Effect.cancel") from another action, \ or, if your effect is driven by a Combine subject, send it a completion. + + • If you do not wish to assert on these effects, perform "await \ + store.skipInFlightEffects()", or consider using a non-exhaustive test store: \ + "store.exhaustivity = .off". """ } } diff --git a/Tests/ComposableArchitectureTests/SharedTests.swift b/Tests/ComposableArchitectureTests/SharedTests.swift index ddb4fad04d9d..d309cd391dec 100644 --- a/Tests/ComposableArchitectureTests/SharedTests.swift +++ b/Tests/ComposableArchitectureTests/SharedTests.swift @@ -245,7 +245,7 @@ final class SharedTests: XCTestCase { } XCTExpectFailure { $0.compactDescription == """ - Test store completed before asserting against changes to shared state: … + Test store finished before asserting against changes to shared state: …   SharedFeature.State(   _count: 0, diff --git a/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift b/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift index 6de5bbf74ae0..fb8073ef8083 100644 --- a/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift +++ b/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift @@ -124,10 +124,14 @@ final class TestStoreFailureTests: BaseTCATestCase { XCTExpectFailure { $0.compactDescription == """ - The store received 1 unexpected action by the end of this test: … + The store received 1 unexpected action: … Unhandled actions: • .second + + To fix, explicitly assert against these actions using "store.receive", skip these actions \ + by performing "await store.skipReceivedActions()", or consider using a non-exhaustive test \ + store: "store.exhaustivity = .off". """ } await store.send(.first) @@ -153,15 +157,19 @@ final class TestStoreFailureTests: BaseTCATestCase { • If using async/await in your effect, it may need a little bit of time to properly \ finish. To fix you can simply perform "await store.finish()" at the end of your test. - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", etc.), \ - make sure that you wait enough time for it to perform the effect. If you are using a \ - test clock/scheduler, advance it so that the effects may complete, or consider using an \ + • If an effect uses a clock (or scheduler, via "receive(on:)", "delay", "debounce", etc.), \ + make sure that you wait enough time for it to perform the effect. If you are using a test \ + clock/scheduler, advance it so that the effects may complete, or consider using an \ immediate clock/scheduler to immediately perform the effect instead. - • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ - then make sure those effects are torn down by marking the effect ".cancellable" and \ - returning a corresponding cancellation effect ("Effect.cancel") from another action, or, \ - if your effect is driven by a Combine subject, send it a completion. + • If you are returning a long-living effect (timers, notifications, subjects, etc.), then \ + make sure those effects are torn down by marking the effect ".cancellable" and returning a \ + corresponding cancellation effect ("Effect.cancel") from another action, or, if your \ + effect is driven by a Combine subject, send it a completion. + + • If you do not wish to assert on these effects, perform "await \ + store.skipInFlightEffects()", or consider using a non-exhaustive test store: \ + "store.exhaustivity = .off". """ } await store.send(()) From d3182c944119af35cefee9043a880c4e96640878 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Tue, 7 May 2024 11:19:43 -0700 Subject: [PATCH 2/3] test --- .../TestStoreFailureTests.swift | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift b/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift index fb8073ef8083..af02290f114a 100644 --- a/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift +++ b/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift @@ -137,6 +137,36 @@ final class TestStoreFailureTests: BaseTCATestCase { await store.send(.first) } + @MainActor + func testReceivedActionBeforeFinish() async { + enum Action { case first, second } + let store = TestStore(initialState: 0) { + Reduce { state, action in + switch action { + case .first: return .send(.second) + case .second: return .none + } + } + } + self.longLivingStore = store + + await store.send(.first) + XCTExpectFailure { + $0.compactDescription == """ + The store received 1 unexpected action: … + + Unhandled actions: + • .second + + To fix, explicitly assert against these actions using "store.receive", skip these actions \ + by performing "await store.skipReceivedActions()", or consider using a non-exhaustive test \ + store: "store.exhaustivity = .off". + """ + } + await store.finish() + } + private var longLivingStore: AnyObject? + @MainActor func testEffectInFlightAfterDeinit() async { let store = TestStore(initialState: 0) { From 46865cf38326878cf27ad16aa9dcd68f1b37e10d Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Tue, 7 May 2024 11:54:56 -0700 Subject: [PATCH 3/3] Add docs about long-living stores --- .../Documentation.docc/Articles/Testing.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/Sources/ComposableArchitecture/Documentation.docc/Articles/Testing.md b/Sources/ComposableArchitecture/Documentation.docc/Articles/Testing.md index 1ebc780476ac..691dd7a67d4d 100644 --- a/Sources/ComposableArchitecture/Documentation.docc/Articles/Testing.md +++ b/Sources/ComposableArchitecture/Documentation.docc/Articles/Testing.md @@ -873,6 +873,43 @@ transitively get access to it through the app itself. In Xcode, go to "Build Pha "ComposableArchitecture" from the "Link Binary With Libraries" section. When using SwiftPM, remove the "ComposableArchitecture" entry from the `testTarget`'s' `dependencies` array in `Package.swift`. +### Long-living test stores + +Test stores should always be created in individual tests when possible, rather than as a shared +instance variable on the test class: + +```diff + final class FeatureTests: XCTestCase { + // 👎 Don't do this: +- let store = TestStore(initialState: Feature.State()) { +- Feature() +- } + + func testBasics() async { + // 👍 Do this: ++ let store = TestStore(initialState: Feature.State()) { ++ Feature() ++ } + // ... + } + } +``` + +This allows you to be very precise in each test: you can start the store in a very specific state, +and override just the dependencies a test cares about. + +More crucially, test stores that are held onto by the test class will not be deinitialized during a +test run, and so various exhaustive assertions made during deinitialization will not be made, +_e.g._ that the test store has unreceived actions that should be asserted against, or in-flight +effects that should complete. + +If a test store does _not_ deinitialize at the end of a test, you must explicitly call +``TestStore/finish(timeout:file:line:)-53gi5`` at the end of the test to retain exhaustive coverage: + +```swift +await store.finish() +``` + [xctest-dynamic-overlay-gh]: http://github.com/pointfreeco/xctest-dynamic-overlay [Testing-state-changes]: #Testing-state-changes [Testing-effects]: #Testing-effects