Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

RxTest: Adding XCTAssertEqual methods for Void streams #2485

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rkreutz
Copy link
Contributor

@rkreutz rkreutz commented Feb 9, 2023

This addresses #1552.

It basically treats next events on streams with Void as element as if they were the same.

I am aware there was another PR addressing this which got closed, however I do think it is worth raising the discussion again and updating the PR to include Maybe and Single streams as well.

The reasons I believe it is worth revisiting the original stance on this is twofold:

  • There's been somewhat a trend in the Swift community (or even the Developer community for that matter) to use Tests as documentation, having that in mind using the current workarounds might not be as readable as if we treated Void as if they were Equatable, e.g:
// less readable, might raise confusion to other developers/new joiners of why mapping values

let observer = scheduler.createObserver(Bool.self)

anyVoidOutputs
    .map { true }
    .bind(to: observer)

let expected: [Recorded<Event<Bool>>] = [
    .next(TestScheduler.Defaults.subscribed, true)
]

XCTAssertEqual(observer.events, expected)

// more readable (IMHO)
let observer = scheduler.start { anyVoidOutputs }
let expected: [Recorded<Event<Void>>] = [
    .next(TestScheduler.Defaults.subscribed)
]

XCTAssertEqual(observer.events, expected)
  • As mentioned, there might be a day where the Core Swift team might decide to implement Equatable conformance to Void, however that will probably be limited to whatever Swift version that is introduced and won't be backwards compatible so we would lock-out library consumers of this nicer API until they are able to update to the latest Swift version. If we go ahead with merging this PR, we can then add #if swift() macros around these Void stream implementations so they do not clash with the Equatable implementations.

With that said, it might not be much of an issue to most people that use this library, but that might be due to the fact that tests are usually neglected (don't usually follow the same strict guidelines as "production" code, or worst, not implemented at all), at least that has been my previous experience, so any opportunity to make testing a bit easier might contribute to a future with more reliable software (one can hope 馃槀).

Edit: I also noticed there was a concern around tuples (like what to do in the case of (Void, Void) elements or even (Int, Void) or any other combination), should we keep adding methods to support those cases? Though I believe this is unrealistic, I'd say 99% of the time people would use Void streams rather than (Void, Void) and tuples with different types (like (Int, Void)) is even less likely since what most would do would be to just map to the actual value needed (Int in this case). So I don't think it would be an issue not providing this simpler interface to tuples with Void elements.

@rkreutz rkreutz changed the title Adding XCTAssertEqual methods for Void streams RxTest: Adding XCTAssertEqual methods for Void streams Feb 9, 2023
@freak4pc freak4pc force-pushed the main branch 2 times, most recently from 1198683 to 5949cbd Compare April 21, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant