Add XCTAssertEqual for [Recorded<Event<Void>>]

[kouki-dan]

The observer of Void is not testable on RxSwift.
This is a test case for this.

        let scheduler = TestScheduler(initialClock: 0)
        let observer = scheduler.createObserver(Void.self)

        scheduler.scheduleAt(100, action: {
            observer.onNext(())
        })

        scheduler.start()

        XCTAssertEqual(observer.events, [
            .next(100, ())
        ])

This code fails to compile on latest RxSwift code because XCTAssertEqual is not defined for Recorded<Event<Void>>>.

This PR adds XCTAssertEqual method and equatable for related them.

This PR also may fix the issue's problem: #1552

[RxPullRequestBot]

	1 Warning
⚠️	No CHANGELOG changes made

Generated by 🚫 Danger

[freak4pc]

If we’re doing a specific overload for void, might as well make it time only, since the value is irrelevant and constant

[freak4pc]

I added an overload to accept time only for Void events, I think this would prove very useful so you vould just do

XCTAssertEqual(voidStream.events, [
    .next(5),
    .next(20),
    .next(55),
    .next(70)
])

What do you think ?

[kouki-dan]

I also think this factory method for Void events is very useful!

[freak4pc]

LGTM

[kouki-dan]

Thank you for your review and fantastic idea!

[freak4pc]

@kzaher Have a quick minute for this? pretty quick change, but remedies an issue some users have. What do you think?

[kzaher]

Hi guys,

I can understand why this is annoying. I'm also annoyed by it. But as far as I can tell this is Swift compiler bug/inadequacy.
In case this is fixed in the compiler layer and it is possible to make () conform to Equatable or they make () conform to Equatable directly we will have ambiguity errors.

I would prefer if we stayed away from trying to introduce workarounds for language/compiler inadequacies.

 1> () == ()

$R0: Bool = true
  2>  
  3> func a<T: Equatable>(l: T, r: T) -> Bool { return l == r }
  4> a(l: (), r: ())
error: repl.swift:4:13: error: argument type '()' does not conform to expected type 'Equatable'
a(l: (), r: ())

If this was some serious issue, then sure, we would pull it in because hell, world isn't perfect. But it seems to me that this is just an annoyance.

If somebody needs to assert a list of voids, just transform it to one of your custom Empty: Equatable types in the unit tests and temporarily test these. It should be equivalend. This is what the compiler should do anyway.

[freak4pc]

@kzaher I disagree with this assumption.

We can only program for the language we have, not for the language we might have. There is no intention from the Swift core team to Fix this, it seems to me at least. And even if that happens, we can always have it as part of a new release, like we always do.

The problem is substantial, as right now the only way to assert equality of Recorded<Event<Void>> is to map it to something else (like voidOutput.map { "x" }.bind(to: stringTestableObserver)), since otherwise there would be no possible way to assert the validity of these events.

I urge you to reconsider this as its a huge pain in the butt for asserting anything void (which is substantial, as mentioned).

[kzaher]

Hi @freak4pc ,

what evidence do you have that needing to add one additional map is a substantial problem? It seems to me that this library exists for 4 years, so if this was a substantial issue, why is this PR made now and not sooner?

I would say that we need to be consistent and only provide Equality extensions. We can't provide tuple specific overloads for every single API where we have Equatable. This is not a realistic expectation.

If we accept this PR, how are you going to decide should we also add overload for (E) where E: Equatable, or (E1, E2) where E1: Equatable, E2: Equatable? At what number should we stop?

Should we also add these overloads for distinctUntilChanged operator?

And even if we did all that, that still wouldn't solve any of my problems since there is myriad of other APIs I have that accept Equatable, but that don't work with tuples.

I urge you to reconsider this as its a huge pain in the butt for asserting anything void (which is substantial, as mentioned).

It is. I'm looking at this every day. My code is littered with structs that are only there because tuples aren't Equatable.

[freak4pc]

I don't think the fact users were able to hack around this for 4 years makes this a non-considerable change. The fact is - Void events are not going anywhere, and they are very very common. Testing against those is also very very common.

Tuples are out of the question for this specific case IMO - since there are so many variations on tuples, but Voids are just a singular thing.

distinctUntilChanged will cause a stream to be stuck since Void cannot change - again this isn't the best example as Void isn't a value that has "transitions", it is a singular static value that is basically just a "Signal" something has happened, so only the time piece is relevant to a stream of Void. Which is why, IMO it deserves this special consideration (even if it was Equatable, the overload asserting only times is helpful since values have no meaning in this case).

[freak4pc]

Any advanced thoughts of this or should we close?
I still bump into this a lot, we've moved these extensions to many projects but think it is a really nice and senseful piece of code to have in the core project.

[kzaher]

@freak4pc I have some different ideas how to tackle this. There is a chance that we'll write a swift evolution proposal that would indirectly solve this issue on the compiler level, where it should be solved. Last I've heard was that there was some support in the compiler community, but I think that we are still waiting for approval.

I would really want to solve this in a more correct way.

[freak4pc]

@kzaher I actually thank that "API-wise" this is the much cleaner path.

Even if in the compiler you'd make Void conform to Observable, you would still need to manually repeat a value that never changes.

e.g.

.next(10, ())
.next(20, ())
.next(25, ())
.completed

This API, even though is more manual, provides us with a much cleaner usage, in this specific case where value has no meaning:

.next(10)
.next(20)
.next(25)
.completed

I still think that the road to fix this in the compiler is very very far, so we should have a normal solution in the meantime that doesn't rely on mapping to a string just to perform a simple test. That's just my personal opinion :)

[freak4pc]

Hey @kzaher - would appreciate if you would give this another thought.
Especially my last comment, even if Void would be Equatable, it would still not provide an "easy" syntax to only provide team, which provides a lot of boilerplate (e.g. repeating () even though it's a singular entity)

[YiYiZheng]

@freak4pc
As the pull request has not merge, how can I fix this problem now?

[freak4pc]

Manually mapping void streams to some string and testing against that

[kouki-dan]

Thank you for your discussion and sorry for absent. I understood you are saying.

When I'm facing this problem, I thought why Tuple does not conform Equatable. If this is resolved by compiler, I think that is better.

@YiYiZheng I always map to true of Bool in my testing file when I want to test Void stream. Like this.

let observer = scheduler.createObserver(Bool.self)
anyVoidOutputs
    .map { true }
    .bind(to: observer)
XCTAssertEqual(observer.events, [
    .next(time, true),
])

Any other ideas?
I thought using own enum instead of Void which has only one case and the enum conforms Equatable may fix this issues. But I'm not sure that is the right way to use Rx, so I always use map to true in my testing.
I mean define own enum like this and use it instead of Void in my code(not test code).

enum Event: Equatable {
    case publish
}

[lordzsolt]

Dear diary,

It is March, 2021. This PR was opened 2.5 years ago, but it's still as relevant today.
Nothing has changed.
I am sad.

[freak4pc]

Dear diary,
Apple apparently still doesn't consider Void equal to Void or equatable at all for that matter :)