Rxswift: Extend RxBlocking to retrieve error from observable in unit tests

[sgleadow]

Short description of the issue:

Feature request: Add a mechanism to retrieve an error from an observable, synchronously, via RxBlocking.

Expected outcome:

If this is a desirable feature, can we agree on a good interface?

I wrote an interface in #1354 (removed pending discussion here) that optionally returned the error if the observable terminated with error, or nil otherwise:

extension BlockingObservable {
    public func toError() -> Swift.Error? {  ... }
}

This is a different style interface to the existing toArray(), which will return the elements from the observable if they exist and throws an error otherwise.

Given this is for use in tests, I'd prefer all these to return optional values that we can assert on, and not to throw an error... the throwing seems to make the tests more verbose to eg. check that an error occurred. For example without throwing the style is more like:

        let error = Observable<Int>.error(testError).toBlocking().toError()
        XCTAssertNotNil(error)

As opposed to needing to catch functions if they throw:

        do {
            _ = try Observable<Int>.error(testError).toBlocking().single()
            XCTFail()
        }
        catch let e {
            XCTAssertErrorEqual(e, testError)
        }

What actually happens:

_Not implemented yet_

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

3.6.1

Platform/Environment

• [x] iOS
• [x] macOS
• [x] tvOS
• [x] watchOS
• [x] playgrounds

Xcode version:

  Xcode version goes here

:warning: Fields below are optional for general issues or in case those questions aren't related to your issue, but filling them out will increase the chances of getting your issue resolved. :warning:

Installation method:

• [x] CocoaPods
• [ ] Carthage
• [ ] Git submodules

I have multiple versions of Xcode installed:
(so we can know if this is a potential cause of your issue)

• [x] yes (which ones)
• [ ] no

Level of RxSwift knowledge:
(this is so we can understand your level of knowledge
and formulate the response in an appropriate manner)

• [ ] just starting
• [x] I have a small code base
• [ ] I have a significant code base

[kzaher]

Hi @sgleadow ,

I think this is a more general question "how to concisely test throwing method?".

I think you can do

XCTAssertThrowsError(try xs.toBlocking().toArray())

or

XCTAssertThrowsError(try xs.toBlocking().toArray()) { error in
    XCTAssertEqual(error as? MyError, MyError.first)
}

[sgleadow]

Thanks for the feedback. That's definitely a more concise way of testing a throwing method, I'll try that out and see how it feels.

Having RxBlocking throw when there's an error seems a bit at odds with the Rx approach. I have very little code that throws in an RxSwift codebase, because it's just another way of an Observable terminating.

It would be a breaking change, but what do you think about RxBlocking just returning the values rather than throwing? eg.

toArray:

• [], if it completes with no values
• [a, b, c], for any values that occur before completion
• nil, if it completes with error

toError:

• nil, if it completes successfully
• error, if it completes with error

It seems closer to what I'd expect if essentially _let the observable synchronously run until completed_ and kept the values.

What do you think?

[kzaher]

Hi @sgleadow ,

I think that probably most accurate description would be something like:

enum SequenceMaterializeResult<T> {
     case completed(elements: [T])
     case failed(elements: [T], error: Error)
}

func materialize() -> SequenceMaterializeResult<T>

Have no idea what would be the best name for this, but it seems people like materialize a lot lately :trollface: .

[sgleadow]

I like that actually, it makes sense, and could also live alongside the existing implementations without being confusing.

Do you see that sitting next to (or near) the existing toArray functionality in RxBlocking? If so, I'd be happy to have a go and see what you think, and maintain the existing API so there are no migration issues.

[kzaher]

Yeah, I think this should be the basic implementation and to array should just be a convenience based on that method.

I think that internal implementation could have additional limit so we can also reuse it for single, first, etc.