From 966d5f2ad6c54b6914d3744571610d5e0a3a4cb5 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Wed, 9 Oct 2024 11:19:03 -0700 Subject: [PATCH 1/2] Infrastructure: Address concurrency warnings in tests --- Sources/ComposableArchitecture/Effect.swift | 4 +- .../BindingLocalTests.swift | 1 + .../CompatibilityTests.swift | 2 + .../CurrentValueRelayTests.swift | 2 +- .../EffectTests.swift | 8 ++-- .../Reducers/BindingReducerTests.swift | 1 + .../ScopeCacheTests.swift | 5 +++ .../SharedTests.swift | 15 +++---- .../StoreFilterTests.swift | 1 + .../StoreLifetimeTests.swift | 3 ++ .../StoreTests.swift | 15 ++++++- .../TestStoreTests.swift | 8 ++-- .../ThrottleTests.swift | 12 +++--- .../ViewStoreTests.swift | 40 +++++++++++-------- 14 files changed, 75 insertions(+), 42 deletions(-) diff --git a/Sources/ComposableArchitecture/Effect.swift b/Sources/ComposableArchitecture/Effect.swift index 2afaf5b526f1..f31eb3571ee3 100644 --- a/Sources/ComposableArchitecture/Effect.swift +++ b/Sources/ComposableArchitecture/Effect.swift @@ -2,9 +2,9 @@ import Combine import Foundation import SwiftUI -public struct Effect { +public struct Effect: Sendable { @usableFromInline - enum Operation { + enum Operation: @unchecked Sendable { case none case publisher(AnyPublisher) case run(TaskPriority? = nil, @Sendable (_ send: Send) async -> Void) diff --git a/Tests/ComposableArchitectureTests/BindingLocalTests.swift b/Tests/ComposableArchitectureTests/BindingLocalTests.swift index 4f7dd6f673d8..0eb72a1681e4 100644 --- a/Tests/ComposableArchitectureTests/BindingLocalTests.swift +++ b/Tests/ComposableArchitectureTests/BindingLocalTests.swift @@ -4,6 +4,7 @@ @testable import ComposableArchitecture final class BindingLocalTests: BaseTCATestCase { + @MainActor public func testBindingLocalIsActive() { XCTAssertFalse(BindingLocal.isActive) diff --git a/Tests/ComposableArchitectureTests/CompatibilityTests.swift b/Tests/ComposableArchitectureTests/CompatibilityTests.swift index 6bec1258240d..781051c5b250 100644 --- a/Tests/ComposableArchitectureTests/CompatibilityTests.swift +++ b/Tests/ComposableArchitectureTests/CompatibilityTests.swift @@ -6,6 +6,7 @@ final class CompatibilityTests: BaseTCATestCase { // Actions can be re-entrantly sent into the store if an action is sent that holds an object // which sends an action on deinit. In order to prevent a simultaneous access exception for this // case we need to use `withExtendedLifetime` on the buffered actions when clearing them out. + @MainActor func testCaseStudy_ActionReentranceFromClearedBufferCausingDeinitAction() { let cancelID = UUID() @@ -78,6 +79,7 @@ final class CompatibilityTests: BaseTCATestCase { // In particular, this means that in the implementation of `Store.send` we need to flip // `isSending` to false _after_ the store's state mutation is made so that re-entrant actions // are buffered rather than immediately handled. + @MainActor func testCaseStudy_ActionReentranceFromStateObservation() { var cancellables: Set = [] defer { _ = cancellables } diff --git a/Tests/ComposableArchitectureTests/CurrentValueRelayTests.swift b/Tests/ComposableArchitectureTests/CurrentValueRelayTests.swift index 56c0708e653f..4bf45d8a2ea8 100644 --- a/Tests/ComposableArchitectureTests/CurrentValueRelayTests.swift +++ b/Tests/ComposableArchitectureTests/CurrentValueRelayTests.swift @@ -15,7 +15,7 @@ await withTaskGroup(of: Void.self) { group in for index in 1...1_000 { - group.addTask { + group.addTask { @Sendable in subject.send(index) } } diff --git a/Tests/ComposableArchitectureTests/EffectTests.swift b/Tests/ComposableArchitectureTests/EffectTests.swift index d821a5bb352b..a277d446998c 100644 --- a/Tests/ComposableArchitectureTests/EffectTests.swift +++ b/Tests/ComposableArchitectureTests/EffectTests.swift @@ -47,11 +47,11 @@ final class EffectTests: BaseTCATestCase { } func testConcatenateOneEffect() async { - await withMainSerialExecutor { + await withMainSerialExecutor { [mainQueue] in let values = LockIsolated<[Int]>([]) let effect = Effect.concatenate( - .publisher { Just(1).delay(for: 1, scheduler: self.mainQueue) } + .publisher { Just(1).delay(for: 1, scheduler: mainQueue) } ) let task = Task { @@ -62,10 +62,10 @@ final class EffectTests: BaseTCATestCase { XCTAssertEqual(values.value, []) - await self.mainQueue.advance(by: 1) + await mainQueue.advance(by: 1) XCTAssertEqual(values.value, [1]) - await self.mainQueue.run() + await mainQueue.run() XCTAssertEqual(values.value, [1]) await task.value diff --git a/Tests/ComposableArchitectureTests/Reducers/BindingReducerTests.swift b/Tests/ComposableArchitectureTests/Reducers/BindingReducerTests.swift index 77f1722074b4..0e8c63c75850 100644 --- a/Tests/ComposableArchitectureTests/Reducers/BindingReducerTests.swift +++ b/Tests/ComposableArchitectureTests/Reducers/BindingReducerTests.swift @@ -47,6 +47,7 @@ final class BindingTests: BaseTCATestCase { ) } + @MainActor func testViewEquality() { struct Feature: Reducer { struct State: Equatable { diff --git a/Tests/ComposableArchitectureTests/ScopeCacheTests.swift b/Tests/ComposableArchitectureTests/ScopeCacheTests.swift index 4ccebb742855..e4b288a95b65 100644 --- a/Tests/ComposableArchitectureTests/ScopeCacheTests.swift +++ b/Tests/ComposableArchitectureTests/ScopeCacheTests.swift @@ -35,6 +35,7 @@ final class ScopeCacheTests: BaseTCATestCase { store.send(.child(.dismiss)) } + @MainActor func testOptionalScope_CachedStore() { #if DEBUG let store = StoreOf(initialState: Feature.State(child: Feature.State())) { @@ -46,6 +47,7 @@ final class ScopeCacheTests: BaseTCATestCase { #endif } + @MainActor func testOptionalScope_StoreIfLet() { #if DEBUG let store = StoreOf(initialState: Feature.State(child: Feature.State())) { @@ -62,6 +64,7 @@ final class ScopeCacheTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testOptionalScope_StoreIfLet_UncachedStore() { let store = StoreOf(initialState: Feature.State(child: Feature.State())) { } @@ -93,6 +96,7 @@ final class ScopeCacheTests: BaseTCATestCase { } } + @MainActor func testIdentifiedArrayScope_CachedStore() { #if DEBUG let store = StoreOf(initialState: Feature.State(rows: [Feature.State()])) { @@ -108,6 +112,7 @@ final class ScopeCacheTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testIdentifiedArrayScope_UncachedStore() { let store = StoreOf(initialState: Feature.State(rows: [Feature.State()])) { Feature() diff --git a/Tests/ComposableArchitectureTests/SharedTests.swift b/Tests/ComposableArchitectureTests/SharedTests.swift index 9622697ca12e..adf7ca243d39 100644 --- a/Tests/ComposableArchitectureTests/SharedTests.swift +++ b/Tests/ComposableArchitectureTests/SharedTests.swift @@ -96,10 +96,11 @@ final class SharedTests: XCTestCase { XCTAssertEqual(store.state.sharedCount, 2) } + @MainActor func testMultiSharing() async { @Shared(Stats()) var stats - let store = await TestStore( + let store = TestStore( initialState: SharedFeature.State( profile: Shared(Profile(stats: $stats)), sharedCount: Shared(0), @@ -721,7 +722,7 @@ final class SharedTests: XCTestCase { func testSharedDefaults_Used() { let didAccess = LockIsolated(false) - let logDefault: () -> Bool = { + let logDefault: @Sendable () -> Bool = { didAccess.setValue(true) return true } @@ -732,7 +733,7 @@ final class SharedTests: XCTestCase { func testSharedDefaults_Unused() { let didAccess = LockIsolated(false) - let logDefault: () -> Bool = { + let logDefault: @Sendable () -> Bool = { didAccess.setValue(true) return true } @@ -761,7 +762,7 @@ final class SharedTests: XCTestCase { func testSharedOverrideDefault() { let accessedActive1 = LockIsolated(false) let accessedDefault = LockIsolated(false) - let logDefault: () -> Bool = { + let logDefault: @Sendable () -> Bool = { accessedDefault.setValue(true) return true } @@ -796,7 +797,7 @@ final class SharedTests: XCTestCase { func testSharedReaderOverrideDefault() { let accessedActive1 = LockIsolated(false) let accessedDefault = LockIsolated(false) - let logDefault: () -> Bool = { + let logDefault: @Sendable () -> Bool = { accessedDefault.setValue(true) return true } @@ -1141,7 +1142,7 @@ private struct SimpleFeature { } @Perceptible -class SharedObject { +class SharedObject: @unchecked Sendable { var count = 0 } @@ -1244,7 +1245,7 @@ extension PersistenceReaderKey where Self == PersistenceKeyDefault Bool) -> Self { + static func isActive(default keyDefault: @escaping @Sendable () -> Bool) -> Self { PersistenceKeyDefault(.appStorage("isActive"), keyDefault()) } } diff --git a/Tests/ComposableArchitectureTests/StoreFilterTests.swift b/Tests/ComposableArchitectureTests/StoreFilterTests.swift index 66c5a271061e..5781116a6cc3 100644 --- a/Tests/ComposableArchitectureTests/StoreFilterTests.swift +++ b/Tests/ComposableArchitectureTests/StoreFilterTests.swift @@ -3,6 +3,7 @@ import Combine import XCTest final class StoreInvalidationTests: BaseTCATestCase { + @MainActor func testInvalidation() { var cancellables: Set = [] diff --git a/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift b/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift index 79a6add3c1b9..065f94a74dcb 100644 --- a/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift +++ b/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift @@ -4,6 +4,7 @@ import XCTest final class StoreLifetimeTests: BaseTCATestCase { @available(*, deprecated) + @MainActor func testStoreCaching() { let grandparentStore = Store(initialState: Grandparent.State()) { Grandparent() @@ -21,6 +22,7 @@ final class StoreLifetimeTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testStoreInvalidation() { let grandparentStore = Store(initialState: Grandparent.State()) { Grandparent() @@ -48,6 +50,7 @@ final class StoreLifetimeTests: BaseTCATestCase { } #if DEBUG + @MainActor func testStoreDeinit() { Logger.shared.isEnabled = true do { diff --git a/Tests/ComposableArchitectureTests/StoreTests.swift b/Tests/ComposableArchitectureTests/StoreTests.swift index 2dd3f0182031..50037cd87877 100644 --- a/Tests/ComposableArchitectureTests/StoreTests.swift +++ b/Tests/ComposableArchitectureTests/StoreTests.swift @@ -1,4 +1,4 @@ -import Combine +@preconcurrency import Combine @_spi(Internals) import ComposableArchitecture import XCTest @@ -47,6 +47,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testScopedStoreReceivesUpdatesFromParent() { let counterReducer = Reduce({ state, _ in state += 1 @@ -71,6 +72,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testParentStoreReceivesUpdatesFromChild() { let counterReducer = Reduce({ state, _ in state += 1 @@ -95,6 +97,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testScopeCallCount_OneLevel_NoSubscription() { var numCalls1 = 0 let store = Store(initialState: 0) {} @@ -112,6 +115,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testScopeCallCount_OneLevel_Subscribing() { var numCalls1 = 0 let store = Store(initialState: 0) {} @@ -130,6 +134,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testScopeCallCount_TwoLevels_Subscribing() { var numCalls1 = 0 var numCalls2 = 0 @@ -158,6 +163,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testScopeCallCount_ThreeLevels_ViewStoreSubscribing() { var numCalls1 = 0 var numCalls2 = 0 @@ -280,6 +286,7 @@ final class StoreTests: BaseTCATestCase { XCTAssertEqual(values, [1, 2, 3, 4]) } + @MainActor func testLotsOfSynchronousActions() { enum Action { case incr, noop } let reducer = Reduce({ state, action in @@ -350,6 +357,7 @@ final class StoreTests: BaseTCATestCase { XCTAssertEqual(outputs, [nil, 1, nil, 1, nil, 1, nil]) } + @MainActor func testIfLetTwo() { let parentStore = Store(initialState: 0) { Reduce { state, action in @@ -382,6 +390,7 @@ final class StoreTests: BaseTCATestCase { .store(in: &self.cancellables) } + @MainActor func testActionQueuing() async { let subject = PassthroughSubject() @@ -391,7 +400,7 @@ final class StoreTests: BaseTCATestCase { case doIncrement } - let store = await TestStore(initialState: 0) { + let store = TestStore(initialState: 0) { Reduce { state, action in switch action { case .incrementTapped: @@ -420,6 +429,7 @@ final class StoreTests: BaseTCATestCase { subject.send(completion: .finished) } + @MainActor func testCoalesceSynchronousActions() { let store = Store(initialState: 0) { Reduce { state, action in @@ -451,6 +461,7 @@ final class StoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testBufferedActionProcessing() { struct ChildState: Equatable { var count: Int? diff --git a/Tests/ComposableArchitectureTests/TestStoreTests.swift b/Tests/ComposableArchitectureTests/TestStoreTests.swift index 78cc900fb268..d919a1350028 100644 --- a/Tests/ComposableArchitectureTests/TestStoreTests.swift +++ b/Tests/ComposableArchitectureTests/TestStoreTests.swift @@ -1,4 +1,4 @@ -import Combine +@preconcurrency import Combine import ComposableArchitecture import XCTest @@ -396,6 +396,7 @@ final class TestStoreTests: BaseTCATestCase { } } + @MainActor func testPrepareDependenciesCalledOnce() { var count = 0 let store = TestStore(initialState: 0) { @@ -469,6 +470,7 @@ final class TestStoreTests: BaseTCATestCase { } } + @MainActor func testSubscribeReceiveCombineScheduler() async { let subject = PassthroughSubject() let scheduler = DispatchQueue.test @@ -482,7 +484,7 @@ final class TestStoreTests: BaseTCATestCase { case start } - let store = await TestStore(initialState: State()) { + let store = TestStore(initialState: State()) { Reduce { state, action in switch action { case .start: @@ -659,7 +661,7 @@ final class TestStoreTests: BaseTCATestCase { } private struct Client: DependencyKey { - var fetch: () -> Int + var fetch: @Sendable () -> Int static let liveValue = Client(fetch: { 42 }) } extension DependencyValues { diff --git a/Tests/ComposableArchitectureTests/ThrottleTests.swift b/Tests/ComposableArchitectureTests/ThrottleTests.swift index 6415ebe60f75..519a9115c751 100644 --- a/Tests/ComposableArchitectureTests/ThrottleTests.swift +++ b/Tests/ComposableArchitectureTests/ThrottleTests.swift @@ -1,6 +1,6 @@ import Combine import ComposableArchitecture -import XCTest +@preconcurrency import XCTest final class EffectThrottleTests: BaseTCATestCase { let mainQueue = DispatchQueue.test @@ -8,7 +8,7 @@ final class EffectThrottleTests: BaseTCATestCase { func testThrottleLatest_Publisher() async { let store = await TestStore(initialState: ThrottleFeature.State()) { ThrottleFeature(id: #function, latest: true) - } withDependencies: { + } withDependencies: { [mainQueue] in $0.mainQueue = mainQueue.eraseToAnyScheduler() } @@ -51,7 +51,7 @@ final class EffectThrottleTests: BaseTCATestCase { func testThrottleLatest_Async() async { let store = await TestStore(initialState: ThrottleFeature.State()) { ThrottleFeature(id: #function, latest: true) - } withDependencies: { + } withDependencies: { [mainQueue] in $0.mainQueue = mainQueue.eraseToAnyScheduler() } @@ -94,7 +94,7 @@ final class EffectThrottleTests: BaseTCATestCase { func testThrottleFirst_Publisher() async { let store = await TestStore(initialState: ThrottleFeature.State()) { ThrottleFeature(id: #function, latest: false) - } withDependencies: { + } withDependencies: { [mainQueue] in $0.mainQueue = mainQueue.eraseToAnyScheduler() } @@ -137,7 +137,7 @@ final class EffectThrottleTests: BaseTCATestCase { func testThrottleAfterInterval_Publisher() async { let store = await TestStore(initialState: ThrottleFeature.State()) { ThrottleFeature(id: #function, latest: true) - } withDependencies: { + } withDependencies: { [mainQueue] in $0.mainQueue = mainQueue.eraseToAnyScheduler() } @@ -158,7 +158,7 @@ final class EffectThrottleTests: BaseTCATestCase { func testThrottleEmitsFirstValueOnce_Publisher() async { let store = await TestStore(initialState: ThrottleFeature.State()) { ThrottleFeature(id: #function, latest: true) - } withDependencies: { + } withDependencies: { [mainQueue] in $0.mainQueue = mainQueue.eraseToAnyScheduler() } diff --git a/Tests/ComposableArchitectureTests/ViewStoreTests.swift b/Tests/ComposableArchitectureTests/ViewStoreTests.swift index 1d9956d999db..f699997f69bb 100644 --- a/Tests/ComposableArchitectureTests/ViewStoreTests.swift +++ b/Tests/ComposableArchitectureTests/ViewStoreTests.swift @@ -1,4 +1,4 @@ -import Combine +@preconcurrency import Combine import ComposableArchitecture import XCTest @@ -7,10 +7,11 @@ final class ViewStoreTests: BaseTCATestCase { override func setUpWithError() throws { try super.setUpWithError() - equalityChecks = 0 - subEqualityChecks = 0 + equalityChecks.setValue(0) + subEqualityChecks.setValue(0) } + @MainActor func testPublisherFirehose() { let store = Store(initialState: 0) {} let viewStore = ViewStore(store, observe: { $0 }) @@ -30,6 +31,7 @@ final class ViewStoreTests: BaseTCATestCase { } @available(*, deprecated) + @MainActor func testEqualityChecks() { let store = Store(initialState: State()) {} @@ -52,22 +54,23 @@ final class ViewStoreTests: BaseTCATestCase { viewStore3.publisher.substate.sink { _ in }.store(in: &self.cancellables) viewStore4.publisher.substate.sink { _ in }.store(in: &self.cancellables) - XCTAssertEqual(0, equalityChecks) - XCTAssertEqual(0, subEqualityChecks) + XCTAssertEqual(0, equalityChecks.value) + XCTAssertEqual(0, subEqualityChecks.value) viewStore4.send(()) - XCTAssertEqual(4, equalityChecks) - XCTAssertEqual(4, subEqualityChecks) + XCTAssertEqual(4, equalityChecks.value) + XCTAssertEqual(4, subEqualityChecks.value) viewStore4.send(()) - XCTAssertEqual(8, equalityChecks) - XCTAssertEqual(8, subEqualityChecks) + XCTAssertEqual(8, equalityChecks.value) + XCTAssertEqual(8, subEqualityChecks.value) viewStore4.send(()) - XCTAssertEqual(12, equalityChecks) - XCTAssertEqual(12, subEqualityChecks) + XCTAssertEqual(12, equalityChecks.value) + XCTAssertEqual(12, subEqualityChecks.value) viewStore4.send(()) - XCTAssertEqual(16, equalityChecks) - XCTAssertEqual(16, subEqualityChecks) + XCTAssertEqual(16, equalityChecks.value) + XCTAssertEqual(16, subEqualityChecks.value) } + @MainActor func testAccessViewStoreStateInPublisherSink() { let reducer = Reduce { count, _ in count += 1 @@ -90,6 +93,7 @@ final class ViewStoreTests: BaseTCATestCase { XCTAssertEqual([0, 1, 2, 3], results) } + @MainActor func testWillSet() { let reducer = Reduce { count, _ in count += 1 @@ -114,6 +118,7 @@ final class ViewStoreTests: BaseTCATestCase { XCTAssertEqual([0, 1, 2], results) } + @MainActor func testPublisherOwnsViewStore() { let reducer = Reduce { count, _ in count += 1 @@ -131,6 +136,7 @@ final class ViewStoreTests: BaseTCATestCase { XCTAssertEqual(results, [0, 1]) } + @MainActor func testStorePublisherSubscriptionOrder() { let store = Store(initialState: 0) { Reduce { state, _ in @@ -299,7 +305,7 @@ private struct State: Equatable { var substate = Substate() static func == (lhs: Self, rhs: Self) -> Bool { - equalityChecks += 1 + equalityChecks.withValue { $0 += 1 } return lhs.substate == rhs.substate } } @@ -308,10 +314,10 @@ private struct Substate: Equatable { var name = "Blob" static func == (lhs: Self, rhs: Self) -> Bool { - subEqualityChecks += 1 + subEqualityChecks.withValue { $0 += 1 } return lhs.name == rhs.name } } -private var equalityChecks = 0 -private var subEqualityChecks = 0 +private let equalityChecks = LockIsolated(0) +private let subEqualityChecks = LockIsolated(0) From 09156ca5caed28f8f4c8a5189ce02ee41b2f8054 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 14 Oct 2024 12:22:20 -0400 Subject: [PATCH 2/2] wip --- Sources/ComposableArchitecture/Effect.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/ComposableArchitecture/Effect.swift b/Sources/ComposableArchitecture/Effect.swift index f31eb3571ee3..a7c898fe2e20 100644 --- a/Sources/ComposableArchitecture/Effect.swift +++ b/Sources/ComposableArchitecture/Effect.swift @@ -1,10 +1,10 @@ -import Combine +@preconcurrency import Combine import Foundation import SwiftUI public struct Effect: Sendable { @usableFromInline - enum Operation: @unchecked Sendable { + enum Operation: Sendable { case none case publisher(AnyPublisher) case run(TaskPriority? = nil, @Sendable (_ send: Send) async -> Void)