swiftlang / swift-testing

A modern, expressive testing package for Swift
Apache License 2.0
1.81k stars 77 forks source link

`confirmation` reports confirmed zero times, despite being confirmed, when triggered on the Main Actor #788

Closed clausjoergensen closed 3 weeks ago

clausjoergensen commented 3 weeks ago

Description

confirmation reports confirmed zero times, despite being confirmed, when triggered on the Main Actor

Expected behavior

The test should pass

Actual behavior

Confirmation was confirmed 0 times, but expected to be confirmed 1 time

Steps to reproduce

Minimal reproducible example:

import Testing
import Combine
import Foundation

final class Service {
    let value = CurrentValueSubject<String, Never>("")
}

final class ViewModel {
    private var cancellables = Set<AnyCancellable>()
    private let service: Service

    var didChangeValue: (String) -> Void = { _ in }

    init(service: Service) {
        self.service = service
    }

    func start() {
        service
            .value
            .receive(on: DispatchQueue.main) // This breaks the test
            .sink { [weak self] value in
                self?.didChangeValue(value)
            }
            .store(in: &cancellables)
    }
}

@Suite
struct SwiftTestingBug {
    @Test
    func example() async {
        let service = Service()
        let viewModel = ViewModel(service: service)
        viewModel.start()

        await confirmation { confirmation in // Confirmation was confirmed 0 times, but expected to be confirmed 1 time
            viewModel.didChangeValue = { value in
                #expect(value == "Hello World")
                confirmation()
            }

            service.value.value = "Hello World"
        }
    }
}

The test can also be broken by using DispatchQueue.main.async for the callback, e.g.

.sink { [weak self] value in
    DispatchQueue.main.async {
        self?.didChangeValue(value)
    }
}

swift-testing version/commit hash

I don't know how to provide this, but the above can be produced in Xcode 16.1 (16B40)

Swift & OS version (output of swift --version ; uname -a)

swift-driver version: 1.115 Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2) Target: arm64-apple-macosx14.0 Darwin clajun-mbp 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:14:30 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6000 arm64

clausjoergensen commented 3 weeks ago

It's possible this is expected, based on this from the documentation

Some tests, especially those that test asynchronously-delivered events, cannot be readily converted to use Swift concurrency. The testing library offers functionality called confirmations which can be used to implement these tests. Instances of Confirmation are created and used within the scope of the function confirmation(:expectedCount:isolation:sourceLocation::).

Confirmations function similarly to the expectations API of XCTest, however, they don’t block or suspend the caller while waiting for a condition to be fulfilled. Instead, the requirement is expected to be confirmed (the equivalent of fulfilling an expectation) before confirmation() returns, and records an issue otherwise:

But the behaviour is so strange I wanted to report it as a bug regardless.

grynspan commented 3 weeks ago

This is expected because your callback is called asynchronously without suspending the caller. Confirmations as currently designed do not block or suspend their caller waiting for events to occur. The flow of events here roughly looks like:

  1. Create the confirmation and enter its body closure.
  2. Trigger the state change, which calls through to sink {} in Combine and then to DispatchQueue.main.async {}.
  3. Exit the body closure of the confirmation and fail.
  4. Deliver the callback that calls confirmation().

If you are able, consider using .values from your Combine publisher instead of .sink {} so that you can suspend the caller until the event you're waiting for is delivered. If that isn't possible (which it won't always be!), you can use a continuation within the confirmation to make the test suspend until the event occurs. Something like (untested):

await confirmation { confirmation in // Confirmation was confirmed 0 times, but expected to be confirmed 1 time
  await withCheckedContinuation { continuation in
    viewModel.didChangeValue = { value in
      #expect(value == "Hello World")
      confirmation()
      continuation.resume()
    }

    service.value.value = "Hello World"
  }
}

We are tracking the need for a blocking equivalent (which would require that you specify a timeout.) We will track this report along with that information. Thanks for filing!