Skip to content

Commit 7fd7f32

Browse files
mbrandonwmluisbrown
authored andcommitted
Simplify re-entrant tests (#1370)
* wip * wip * wip (cherry picked from commit 7940588a9e413004a6efd4c34a2d7f34a8852420) # Conflicts: # Sources/ComposableArchitecture/Store.swift # Tests/ComposableArchitectureTests/CompatibilityTests.swift
1 parent 7b053fa commit 7fd7f32

File tree

2 files changed

+27
-53
lines changed

2 files changed

+27
-53
lines changed

Sources/ComposableArchitecture/Store.swift

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -338,26 +338,11 @@ public final class Store<State, Action> {
338338
var currentState = self.state
339339
let tasks = Box<[Task<Void, Never>]>(wrappedValue: [])
340340
defer {
341-
// NB: Handle any potential re-entrant actions.
342-
//
343-
// 1. After all buffered actions have been processed, we clear them out, but this can lead to
344-
// re-entrant actions if a cleared action holds onto an object that sends an additional
345-
// action during "deinit".
346-
//
347-
// We use "withExtendedLifetime" to prevent simultaneous access exceptions for this case,
348-
// which otherwise would try to append an action while removal is still in-process.
349341
withExtendedLifetime(self.bufferedActions) {
350342
self.bufferedActions.removeAll()
351343
}
352-
// 2. Updating state can also lead to re-entrant actions if the emission of a downstream store
353-
// publisher sends an action back into the store.
354-
//
355-
// We update "state" _before_ flipping "isSending" to false to ensure these actions are
356-
// appended to the buffer and not processed immediately.
357344
self.state = currentState
358345
self.isSending = false
359-
// Should either of the above steps send re-entrant actions back into the store, we handle
360-
// them recursively.
361346
if !self.bufferedActions.isEmpty {
362347
if let task = self.send(
363348
self.bufferedActions.removeLast(), originatingFrom: originatingAction

Tests/ComposableArchitectureTests/CompatibilityTests.swift

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import XCTest
44

55
@MainActor
66
final class CompatibilityTests: XCTestCase {
7-
func testCaseStudy_ReentrantActionsFromBuffer() {
7+
// Actions can be re-entrantly sent into the store if an action is sent that holds an object
8+
// which sends an action on deinit. In order to prevent a simultaneous access exception for this
9+
// case we need to use `withExtendedLifetime` on the buffered actions when clearing them out.
10+
func testCaseStudy_ActionReentranceFromClearedBufferCausingDeinitAction() {
811
let cancelID = UUID()
912

1013
struct State: Equatable {}
@@ -73,51 +76,37 @@ final class CompatibilityTests: XCTestCase {
7376
)
7477
}
7578

76-
func testCaseStudy_ReentrantActionsFromProducer() {
77-
struct State: Equatable {
78-
var city: String
79-
var country: String
80-
}
81-
82-
enum Action: Equatable {
83-
case updateCity(String)
84-
case updateCountry(String)
85-
}
86-
87-
let reducer = Reducer<State, Action, Void> { state, action, _ in
88-
switch action {
89-
case let .updateCity(city):
90-
state.city = city
79+
// Actions can be re-entrantly sent into the store while observing changes to the store's state.
80+
// In such cases we need to take special care that those re-entrant actions are handled _after_
81+
// the original action.
82+
//
83+
// In particular, this means that in the implementation of `Store.send` we need to flip
84+
// `isSending` to false _after_ the store's state mutation is made so that re-entrant actions
85+
// are buffered rather than immediately handled.
86+
func testCaseStudy_ActionReentranceFromStateObservation() {
87+
let store = Store<Int, Int>(
88+
initialState: 0,
89+
reducer: .init { state, action, _ in
90+
state = action
9191
return .none
92-
case let .updateCountry(country):
93-
state.country = country
94-
return .none
95-
}
96-
}
97-
98-
let store = Store(
99-
initialState: State(city: "New York", country: "USA"),
100-
reducer: reducer,
92+
},
10193
environment: ()
10294
)
103-
let viewStore = ViewStore(store)
10495

105-
viewStore.produced.city
106-
.startWithValues { city in
107-
if city == "London" {
108-
viewStore.send(.updateCountry("UK"))
109-
}
110-
}
96+
let viewStore = ViewStore(store)
11197

112-
var countryUpdates = [String]()
113-
viewStore.produced.country
114-
.startWithValues { country in
115-
countryUpdates.append(country)
98+
viewStore.produced.producer
99+
.startWithValues { value in
100+
if value == 1 { viewStore.send(0) }
116101
}
117102

118-
viewStore.send(.updateCity("London"))
103+
var stateChanges: [Int] = []
104+
viewStore.produced.producer
105+
.startWithValues { stateChanges.append($0) }
119106

120-
XCTAssertEqual(countryUpdates, ["USA", "UK"])
107+
XCTAssertEqual(stateChanges, [0])
108+
viewStore.send(1)
109+
XCTAssertEqual(stateChanges, [0, 1, 0])
121110
}
122111
}
123112

0 commit comments

Comments
 (0)