testValue can leak state between tests when used asynchronously

[dafurman]

Description

This might be related to #75, but I'm creating this as a separate issue as it could be a different issue, or it's the underlying issue causing #75.

The problem:

If withDependencies isn't being used, and default testValues are being used instead, if one test kicks off a write to a dependency asynchronously, this write can be triggered during the lifetime of a test run later.
If this happens, that second test's dependency state will be unexpectedly affected by the earlier test, leading to incorrect state.

This can be shown with the contrived example here:

private final class TestObject {
    @Dependency(\.analytics) private var analytics

    func trackAction(named name: String, after delay: Duration = .zero) {
        Task {
            if delay != .zero {
                try await Task.sleep(for: delay)
            }
            analytics.trackEvent(named: name)
        }
    }
}

final class LeakageTests: XCTestCase {
    func testAsyncDependencyMutationOnDefaultDependencyLeaksPart1() async throws {
        TestObject().trackAction(named: "One", after: .milliseconds(10))
    }

    /// This demonstrates that we must avoid using default `testValue` values if they're going to be mutated asynchronously. *This test must be run as part of the suite to pass.*
    func testAsyncDependencyMutationOnDefaultDependencyLeaksPart2() async throws {
        let analytics = DependencyValues._current.analytics as! AnalyticsMock

        // Sleep, to let the async write happen at this time.
        try await Task.sleep(for: .milliseconds(15))

        XCTAssertEqual(analytics.events, ["One"])
        // 💥😱 This assertion passes if the test is run on its own, but it fails if the test is run with testAsyncDependencyMutationOnDefaultDependencyLeaksPart1().
        XCTExpectFailure {
            // What we wanted the value to be:
            XCTAssertEqual(analytics.events, [])
        }
    }
}

A potential workaround

Rather than avoiding using testValue and applying withDependencies absolutely everywhere in tests instead, I found some potential success taking inspiration from https://pointfreeco.github.io/swift-dependencies/main/documentation/dependencies/overridingdependencies#Testing

We can wrap the entire test run in a withDependencies, and just leverage default values automatically:

func withDefaultDependencies(_ operation: () -> Void) {
    withDependencies {
        $0 = .current
    } operation: {
        operation()
    }
}

private extension DependencyValues {
    static var current: Self {
        var values = Self()
        values.context = DependencyValues._current.context
        return values
    }
}

final class LeakageTests: XCTestCase {
    override func invokeTest() {
        withDefaultDependencies {
            super.invokeTest()
        }
    }

    func test_WithDefaultDependencies_OnDefaultDependencyPreventsLeaksPart1() async throws {
        TestObject().trackAction(named: "One", after: .milliseconds(10))
    }

    func test_WithDefaultDependencies_OnDefaultDependencyPreventsLeaksPart2() async throws {
        let analytics = DependencyValues._current.analytics as! AnalyticsMock
        try await Task.sleep(for: .milliseconds(15))

        XCTAssertEqual(analytics.events, [])
    }
}

This passes! 😄

Issues with the workaround

This workaround creates some new issues though, which probably make it not worth using:

1. static initializations of objects start leaking their dependencies, which defeats the whole point of this.
   I'm not really sure why, but once I start introducing this, any statically initialized objects don't seem to get updates to their stored @dependency values. See https://github.com/dafurman/Swift-Dependencies-Exploration/blob/f1be21b6a2725a7656fc7cb799d9bfdd407af60e/Tests/SwiftDepsTests/WithDefaultDependenciesTests.swift#L52

2. Dependencies used in .task() must be propagated with withDependencies(from: self)
   Within the scope of a .task() or any code it kicks off, we have to use stored versions of @Dependency, and propagate dependencies with withDependencies(from: self), otherwise we'll get an incorrect instance of the dependency.
   See https://github.com/dafurman/Swift-Dependencies-Exploration/blob/f1be21b6a2725a7656fc7cb799d9bfdd407af60e/Tests/SwiftDepsTests/WithDefaultDependenciesTests.swift#L38

Maybe someone else can take ideas from this and get something working that keeps testValue safe to use from asynchronous code.

For now, it seems like I have two options, and I'm probably going to go with the second:

1. Apply withDependencies() to every dependency-using action in a test and never rely on testValue.
2. Accept that testValue will sometimes leak and keep an eye out for that. These situations can be hard to notice, but if I do notice them, I at least have the workaround of applying withDependencies() on the leaking test or the test being leaked into.

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

I'd expect use of testValue to be safe between different test runs, and for one test to not read from or mutate another test's dependencies.

Actual behavior

When I write to a default testValue dependency asynchronously, it can affect the dependency's state when accessed from another test, causing tests to succeed if run on their own, but to fail when run as part of a test suite.

Steps to reproduce

Check out https://github.com/dafurman/Swift-Dependencies-Exploration/blob/main/Tests/SwiftDepsTests/LeakageTests.swift and run LeakageTests. Pay attention to testAsyncDependencyMutationOnDefaultDependencyLeaksPart1() and testAsyncDependencyMutationOnDefaultDependencyLeaksPart2(), then run testAsyncDependencyMutationOnDefaultDependencyLeaksPart2() on its own and see that its behavior is different when run in isolation vs having part 1 run beforehand.

Dependencies version information

1.0.0 & main

Destination operating system

iOS 16 & 17

Xcode version information

Xcode 15.0

Swift Compiler version information

swift-driver version: 1.87.2 Apple Swift version 5.9 (swiftlang-5.9.2.1.6 clang-1500.1.0.1.1)

[mbrandonw]

Hi @dafurman, this problem exists in any Xcode test as the following demonstrates:

final class SomeTest: XCTestCase {
  func test1() {
    Task {
      try await Task.sleep(for: .seconds(0.1))
      XCTFail()
    }
  }
  func test2() async throws {
    try await Task.sleep(for: .seconds(0.15))
  }
}

Each individual test passes but the whole suite does not.

This is just a general problem with escaping closures and tests. It is possible to spin up an alternate execution context and execute code long after the test has finished running, allowing one test to leak in another. And it's yet another reason why it's best to stay in the structured programming world as much as possible.

And because of this we don't think there's really anything the library should be doing to try to remedy the situation. There is a chance we could improve the test failure message if we can detect the situation, but that can be very difficult. We tried doing that when we detect a test failure from the test host, but we're not sure how often people read it or how often it helps people.

We are open to suggestions, but since this isn't any issue with the library I am going to convert it to a discussion.