1.x: TestSubscriber extra info on assertion failures #3934
Merged
+165
−25
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| * @param message the message to use for the error | ||
| */ | ||
| final void assertionError(String message) { | ||
| StringBuilder b = new StringBuilder(); |
There was a problem hiding this comment.
nit: you can set some initial capacity to save allocations
There was a problem hiding this comment.
On an unit test failure report?
There was a problem hiding this comment.
oh damn lol k u good
|
This is great! |
| * TestSubscriber, giving more information when some assertXXX check fails. | ||
| * @param message the message to use for the error | ||
| */ | ||
| final void assertionError(String message) { |
There was a problem hiding this comment.
TestObserver needs tests too
There was a problem hiding this comment.
assertValues go to testObserver.assertReceivedOnNext()
There was a problem hiding this comment.
Sure… but it's implementation detail…
| } catch (AssertionError expected) { | ||
| assertEquals("Number of items does not match. Provided: 2 Actual: 3.\n" + | ||
| "Provided values: [1, 2]\n" + | ||
| "Actual values: [a, b, c] (0 completions) (+1 errors)", |
There was a problem hiding this comment.
In case of long Actual values: [] number of completions and errors may not be visible enough for the developer, let's move them on new line?
|
Added newline and plural/singular form. |
|
👍 this is really awesome. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds extra information to assertion failure messages on
TestSubscriberandTestObserver, indicating:onCompletedcalls, which is an indication of hung or skipping operation,Previously, if there was something wrong with the sequence, the order and type of assertions were mostly unhelpful: if
assertValueswas first, the lack of values failure could hide a revealing onError call. If theassertNoErrors()was first, the error is visible but no way of knowing how far the sequence got.Now, it is generally okay to use
assertValuesfirst, which along the difference, will print the lack of completion and the number of exceptions received, plus, theAssertionErrorwill have its cause initialized to the actual or composited exception. The message format thus changes:This extra information saved me a lot of time in 2.x and Rsc development.
Note that this change doesn't make the
assertXXXs also assert for completion or error at all. If the values match, but there is an additional error instead of completion, one has to assert that separately, just like now.