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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error reporting #55

Merged

Conversation

krzysztof-grzybek
Copy link
Contributor

@krzysztof-grzybek krzysztof-grzybek commented May 16, 2020

Hi,
this PR introduces more readable error reporting. It will display message in marble format, like in the screenshot:

  it('should work with a cold observable', () => {
    const provided = cold('-a----b|', { a: 1, b: 2 });
    const expected = cold('--a---b|', { a: 1, b: 3 });

    expect(provided).toBeObservable(expected);
  });

jasmine-marbles-test-report

  • it prints marble comparison and detailed JSON.stringified test messages below
  • if notification is found in the expected TestObservable, then it will match marble character to this Notification, if not, the character will be equal to ?

Additionally, it fixes not.toBeObservable.

Fixes #51, #11

Please let me know what do you think about these changes :)

@xileftenurb
Copy link

after a closer inspection, I think the library was designed to be both used by the jasmine ToBeObservable assertion and the RxJS test Scheduler, but the logic for the two ways is nearly completely separated.
so your PR fix the ToBeObservable way of testing,
and mine fix the "TestScheduler" way.

I think the best would be to merge part of our two work so it uses a single codebase for the two way of testing.
I will try to check your code more closely to see how to fix the TestScheduler from your code

@xileftenurb
Copy link

Ok, I checked again, and I realize that there is no link between the two style of marble testing.

It is virtually like if there was 2 library in one.

I don't think there is much value to keep these 2 part together. It would be more sensible to split the library.

In fact, after closer inspection, I see that the TestScheduler of rxjs already give everythings needed to make marble testing, so I will use it direcly instead of using a third party library.

@brandonroberts brandonroberts merged commit a1fca8d into CodeSequence:master Jul 18, 2020
@brandonroberts
Copy link
Member

Thanks @krzysztof-grzybek, apologies for the delay. I will cut a release soon

@krzysztof-grzybek
Copy link
Contributor Author

No worries @brandonroberts , thank you!

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.

Better test fail reporting
3 participants