Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have store.finish() assert no received actions #3054

Merged
merged 3 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 44 additions & 25 deletions Sources/ComposableArchitecture/TestStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,8 @@ public final class TestStore<State, Action> {
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()
Expand Down Expand Up @@ -609,6 +609,7 @@ public final class TestStore<State, Action> {
}
await Task.yield()
}
self.assertNoSharedChanges(file: file, line: line)
}

deinit {
Expand All @@ -617,23 +618,7 @@ public final class TestStore<State, Action> {
}

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(
Expand All @@ -648,27 +633,59 @@ public final class TestStore<State, Action> {
• 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: Equatable>(_: EquatableState.Type) {
let store = self as! TestStore<EquatableState, Action>
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.
Expand All @@ -677,11 +694,12 @@ public final class TestStore<State, Action> {
actual: store.state,
updateStateToExpectedResult: nil,
skipUnnecessaryModifyFailure: true,
file: store.file,
line: store.line
file: file,
line: line
)
}
open(stateType)
self.sharedChangeTracker.resetChanges()
}
}

Expand Down Expand Up @@ -2594,5 +2612,6 @@ extension TestStore {
file: StaticString = #file,
line: UInt = #line
) async {
fatalError()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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".
"""
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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".
"""
}
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/ComposableArchitectureTests/SharedTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
54 changes: 46 additions & 8 deletions Tests/ComposableArchitectureTests/TestStoreFailureTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,49 @@ 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)
}

@MainActor
func testReceivedActionBeforeFinish() async {
enum Action { case first, second }
let store = TestStore(initialState: 0) {
Reduce<Int, Action> { 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) {
Expand All @@ -153,15 +187,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(())
Expand Down