Testing tasks

[Miiha]

Hi everyone,

I stumbled over some swift concurrency quirks while testing a reducer that does some calls to an async dependency which returns an AsyncStream. I know that testing swift concurrency is a delicate problem and maybe I also missing something here.
The issue that appears, is that the calls to the dependency are not 100% reproducible and expecting on the received parameters failes the test.
The following unit test should illustrate the problem, when run e.g. a 100 times.

import XCTest
import ComposableArchitecture

struct Client {
  var observe: @Sendable (Int) async -> AsyncStream<Void>
}

struct Feature: ReducerProtocol {
  let client: Client

  struct State: Equatable {
  }

  enum Action: Equatable {
    case task
  }

  func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
    switch action {
    case .task:
      return Effect.run { send in
        await withTaskGroup(of: Void.self) { group in
          [1, 2, 3].forEach { number in
            group.addTask {
              for await _ in await client.observe(number) {
                continue
              }
            }
          }
        }
      }
    }
  }
}

@MainActor
final class TaskIssueTests: XCTestCase {
  func testReceivedParameters() async {
    let observedNumbers = ActorIsolated<Set<Int>>([])
    let store = TestStore(
      initialState: Feature.State(),
      reducer: Feature(
        client: Client(
          observe: { value in
            await observedNumbers.withValue { _ = $0.insert(value) }
            return .never
          }
        )
      )
    )

    let task = await store.send(.task)
    // Repeating this test multiple times shows that this test is in fact flaky.
    // It looks like the tasks of the task group will not always be run until we get here.
    // Putting a couple of Task.yield() or Task.megaYield() in here also does not seem to solve it.
    await observedNumbers.withValue { XCTAssertEqual($0, [1, 2, 3]) } // note: this compares Sets, so order should not matter.
    await task.cancel()
  }
}

Same behaviour appears when I model the effect differently, e.g. like this:

    case .task:
      return .merge(
        Effect.run { send in
          for await _ in await client.observe(1) {
            continue
          }
        },
        Effect.run { send in
          for await _ in await client.observe(2) {
            continue
          }
        },
        Effect.run { send in
          for await _ in await client.observe(3) {
            continue
          }
        }
      )

[tgrapperon]

Hey @Miiha! Thanks for the report. I don't know what is the correct alternative here, but putting your storage on MainActor seems to serialize things enough for the test to pass 1000 times repeatedly on my machine:

@MainActor
final class TaskIssueTests: XCTestCase {
  func testReceivedParameters() async {
    var observedNumbers = Set<Int>()
    let store = TestStore(
      initialState: Feature.State(),
      reducer: Feature(
        client: Client(
          observe: { @MainActor value in
            observedNumbers.insert(value)
            return .never
          }
        )
      )
    )

    let task = await store.send(.task)
    XCTAssertEqual(observedNumbers, [1, 2, 3])
    await task.cancel()
  }
}

[Miiha]

Hi @tgrapperon, thanks for looking into this.
I haven't thought about doing it like you suggest. And it seems to work really well.
For the testing purpose it's probably fine to constrain the assignment to the main actor.

[mbrandonw]

Hi @Miiha, this is an unfortunate consequence of adopting Swift concurrency. The tools for testing are not really there yet. See this long thread for more information.

As @tgrapperon said, adding @MainActor to things can help, and worst case you can always perform a small Task.sleep to give tasks enough time to start up.

[fonkadelic]

@mbrandonw To me it looks like in this case @MainActor is actually more reliable than the ActorIsolated helper. Do you have any recommendations about when to use one or the other? The examples here (and also in isowords) seem to prefer ActorIsolated in tests but to me annotating things with @MainActor looks actually simpler.

[mbrandonw]

Yeah that's true @fonkadelic, and we have a few places where we skip the ActorIsolated and just use @MainActor too, and maybe we should codify that advice in docs somewhere.

We were hopeful we could use unconstrained asynchrony in tests to most closely mimic what happens in the real world, and in such a situation ActorIsoalated is necessary, but it just seems that Swift's testing tools aren't there yet. Hopefully some day.