Dependency in test uses live value when used with combine in reducer instead of mock value specified in test

[dvosscricut]

Description

Dependency in test uses live value when used with combine in reducer instead of mock value specified in test

Checklist

• I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

bug is in testDemoFeature even though testGetValuesClient is assigned to return mockValues in unit test, it's actually using liveValue instead! So instead of succeeding with mock values of [1776, 1778, 1896], it fails with live value of [42]

Actual behavior

bug is in testDemoFeature even though testGetValuesClient is assigned to return mockValues in unit test, it's actually using liveValue instead! So instead of succeeding with mock values of [1776, 1778, 1896], it fails with live value of [42]

Steps to reproduce

TcaDependencyTestBugV0.50.0.zip

run testDemoFeature unit test and observe test failure when it should succeed

The Composable Architecture version information

0.50.0

Destination operating system

iOS 16.2 (also occurs with project target iOS 15.0)

Xcode version information

14.2

Swift Compiler version information

xcrun swiftc --version
swift-driver version: 1.62.15 Apple Swift version 5.7.2 (swiftlang-5.7.2.135.5 clang-1400.0.29.51)
Target: x86_64-apple-macosx12.0

[mbrandonw]

Hi @dvosscricut, this is not a bug with the library, but rather an intended consequence of using escaping closures. The use of schedule here:

schedulerClient.operation.schedule {
  let values = testGetValuesClient.getValues()
  completion(.success(.scheduledAction(values)))
}

…is using an @escaping closure, which causes you to lose all overridden dependencies and so they revert back to their defaults.

This is happening because dependencies are built on top of @TaskLocals, which has some well-defined ways of propagating dependencies across escaping boundaries, but it can not do it generally. I recommend reading our article on dependency lifetimes to learn more.

One way to fix this is to not using unstructured tools such as schedule and Combine, and instead start using Swift's native concurrency tools. In such contexts dependencies propagate just fine.

If it's not possible to do that, then there is a tool you can use to propagate dependencies through to an escaping closure, and it's called withEscapedDependencies. It can be used like this:

return .future { completion in
  withEscapedDependencies { continuation in
    schedulerClient.operation.schedule {
      continuation.yield {
        let values = testGetValuesClient.getValues()
        completion(.success(.scheduledAction(values)))
      }
    }
  }
}

With that change your test will pass as you expect.

Because this is not an issue with the library I am going to convert it to a discussion.

[dvosscricut]

Is this issue noted in the Migrating to ReducerProtocol section?

https://pointfreeco.github.io/swift-composable-architecture/main/documentation/composablearchitecture/migratingtothereducerprotocol

I did not notice it when reading it. It would be helpful to add that any escaping closures in reducers will not work with overriding dependencies in tests.

In particular "the newest version of the library is still 100% backwards compatible with all previous versions." statement in the documentation implies that we shouldn't have to change our reducer's logic to make ReducerProtocol work for migrating, which isn't the case for unit tests and escaping closures in reducers.

[mbrandonw]

Hi @dvosscricut, we do not claim any kind of backwards compatibility with the new dependency management system because, after all, and it's an entirely new system. It is expected that if you are going to adopt an entirely new way of managing dependencies that there may be some differences.

The backwards compatibility we are referring to in the article means that your current application will continue building as-is without making a single change. There were no breaking changes when we introduced the ReducerProtocol.

If you cannot deal with the new dependency management system right now then we suggest you do not adopt @Dependency, and instead hold onto dependencies as lets on your reducer, which is what the migration guide suggests as a first step towards migrating.

[stephencelis]

@dvosscricut I've added some documentation here to help note that migrating dependencies may require additional steps when you use pre-structured concurrency paradigms like Combine publishers:

#1873

[dvosscricut]

Thanks for adding that to the documentation. Hopefully it'll save some future devs some pain.

I found out from my team that this hack in unit tests also works for making dependencies in escaping closures behave how I want:

func testDemoFeatureDependencyHack() async {
        let mainScheduler = DispatchQueue.test
        let operationScheduler = DispatchQueue.test

        let store = TestStore(
            initialState: DemoFeature.State(),
            reducer: DemoFeature()
        )

        store.dependencies.schedulerClient = .init(
            mainScheduler: {
                mainScheduler.eraseToAnyScheduler()
            },
            operationScheduler: {
                operationScheduler.eraseToAnyScheduler()
            }
        )

        let mockValues: [Int] = [1776, 1778, 1896]

        store.dependencies.testGetValuesClient = .init(
            getValues: {
                mockValues
            }
        )

        await DependencyValues.withValue(\.testGetValuesClient, store.dependencies.testGetValuesClient) {
            await store.send(.demoAction)
            
            await mainScheduler.advance(by: 1)
            await operationScheduler.advance(by: 1)
            
            await store.receive(.scheduledAction(mockValues)) {
                $0.values = mockValues
            }
        }
    }