Tests for styx core observable #198
Conversation
| @@ -92,7 +94,7 @@ public StyxCoreObservable(CompletionStage<T> future) { | |||
| } | |||
|
|
|||
| private static <T> CompletableFuture<T> fromSingleObservable(Observable<T> observable) { | |||
| final CompletableFuture<T> future = new CompletableFuture<>(); | |||
There was a problem hiding this comment.
Why do you need to remove the final modifier?
There was a problem hiding this comment.
Just keeping it in line with our standard coding practices. Excessive finals clutter the codebase, so we avoid them on local variables and method parameters (due to their limited scope).
We only use them on fields because it could be very hard to tract down an unexpected mutation in a large class.
For locals and parameters, we instead aim to keep our methods small and simple enough that any mutations are easily visible (in addition to the other benefits of readable methods).
| } | ||
|
|
||
| @Test(expectedExceptions = NoSuchElementException.class, expectedExceptionsMessageRegExp = "Sequence contains no elements") | ||
| public void throwsExceptionWhenTryingToConvertEmptyToFuture() throws Throwable { |
There was a problem hiding this comment.
It is worth noticing that NoSuchElementException and IllegalArgumentException are now going to be part of our API. Worth documenting this in the StyxObservable interface.
| } | ||
|
|
||
| @Test | ||
| public void flatMapsWorksWithCustomObservable() throws InterruptedException, ExecutionException, TimeoutException { |
There was a problem hiding this comment.
I don't understand this test.
There was a problem hiding this comment.
This test exists because of the problem you observed in your TODO:
TODO: Mikko: Dangerous cast.
Because StyxObservable is an interface, nothing prevents plugins from
implementing it and returning a derived object from thetransformation
function. This would obviously break the cast.
It proves that the problem described has been fixed.
For #196