Skip to content

Commit 7940588

Browse files
authored
Simplify re-entrant tests (#1370)
* wip * wip * wip
1 parent 26f9ed2 commit 7940588

File tree

2 files changed

+27
-54
lines changed

2 files changed

+27
-54
lines changed

Sources/ComposableArchitecture/Store.swift

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -329,26 +329,11 @@ public final class Store<State, Action> {
329329
var currentState = self.state.value
330330
let tasks = Box<[Task<Void, Never>]>(wrappedValue: [])
331331
defer {
332-
// NB: Handle any potential re-entrant actions.
333-
//
334-
// 1. After all buffered actions have been processed, we clear them out, but this can lead to
335-
// re-entrant actions if a cleared action holds onto an object that sends an additional
336-
// action during "deinit".
337-
//
338-
// We use "withExtendedLifetime" to prevent simultaneous access exceptions for this case,
339-
// which otherwise would try to append an action while removal is still in-process.
340332
withExtendedLifetime(self.bufferedActions) {
341333
self.bufferedActions.removeAll()
342334
}
343-
// 2. Updating state can also lead to re-entrant actions if the emission of a downstream store
344-
// publisher sends an action back into the store.
345-
//
346-
// We update "state" _before_ flipping "isSending" to false to ensure these actions are
347-
// appended to the buffer and not processed immediately.
348335
self.state.value = currentState
349336
self.isSending = false
350-
// Should either of the above steps send re-entrant actions back into the store, we handle
351-
// them recursively.
352337
if !self.bufferedActions.isEmpty {
353338
if let task = self.send(
354339
self.bufferedActions.removeLast(), originatingFrom: originatingAction

Tests/ComposableArchitectureTests/CompatibilityTests.swift

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import XCTest
55

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

1114
struct State: Equatable {}
@@ -75,55 +78,40 @@ final class CompatibilityTests: XCTestCase {
7578
)
7679
}
7780

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

107100
var cancellables: Set<AnyCancellable> = []
108-
109-
viewStore.publisher.city
110-
.sink { city in
111-
if city == "London" {
112-
viewStore.send(.updateCountry("UK"))
113-
}
101+
viewStore.publisher
102+
.sink { value in
103+
if value == 1 { viewStore.send(0) }
114104
}
115105
.store(in: &cancellables)
116106

117-
var countryUpdates = [String]()
118-
viewStore.publisher.country
119-
.sink { country in
120-
countryUpdates.append(country)
121-
}
107+
var stateChanges: [Int] = []
108+
viewStore.publisher
109+
.sink { stateChanges.append($0) }
122110
.store(in: &cancellables)
123111

124-
viewStore.send(.updateCity("London"))
125-
126-
XCTAssertEqual(countryUpdates, ["USA", "UK"])
112+
XCTAssertEqual(stateChanges, [0])
113+
viewStore.send(1)
114+
XCTAssertEqual(stateChanges, [0, 1, 0])
127115
}
128116
}
129117

0 commit comments

Comments
 (0)