Platform: Stop recommending jasmine-marbles and remove it from docs examples

Created on 12 Apr 2019  路  6Comments  路  Source: ngrx/platform

Minimal reproduction of the bug/regression with instructions:

The NgRx documentation recommends the use of the jasmine-marbles library, while also pointing the user to the official RxJS docs on marble test syntax.

However jasmine-marbles is silently broken w.r.t. to basic time progression syntax, with no sign of a fix.

E.g. this observable...

const expected = cold('500ms a', { a: new Action() });

...will not produce the observable that the user expects, and which the RxJS documentation suggests it will. Nor will it error, it will simply produce a valid (but wrong) observable. This issue has been open for a year, with no sign of a fix. It appears that jasmine-marbles is effectively abandoned.

This is going to be a major source of frustration of users, who try to write tests as per the documentation and spend a long time debugging them and thinking they have done something wrong.

Expected behavior:

jasmine-marbles should implement the official RxJS marble testing syntax. If it does not, / cannot it should not be recommended as part of the official NgRx testing documentation.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

NgRx: 7.4.0

I would be willing to submit a PR to fix this issue

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

Most helpful comment

I'm also confused, but merely by the fact that jasmine-marbles still exists at all nowadays. What's the benefit over pure TestScheduler usage? In my eyes, the NgRx docs shouldn't confuse users and recommend to use a library that does not add anything on top of existing best practices (which is to use TestScheduler). That is, unless I'm missing something? If so, please forgive me, but it's hard to find out because jasmine-marbles doesn't even have a proper README file or any other docs, at least I cannot find anything in its repo https://github.com/synapse-wireless-labs/jasmine-marbles (this is the right repo, isn't it? ... because there's nothing linked from the npm page of jasmine-marbles package).

All 6 comments

We won't be removing jasmine-marbles from the example app just because its missing that feature. Would you like to submit a PR to jasmine-marbles to add that feature instead? If so, I can possibly help you get it landed.

@brandonroberts I can "fix" it as per this PR: synapse-wireless-labs/jasmine-marbles/pull/38, but I don't see any way it will ever be much more than a hack, as the design of jasmine-marbles is not sympathetic to how RxJS want you to use TestScheduler nowadays. You're not supposed to be creating hot and cold observables at any point, you're only supposed to create them within the context a run() using the supplied helpers, and I don't see any way to make that work without essentially rewriting jasmine-marbles so it does almost nothing but wrap TestScheduler.run().

I think the fix is fine for the purpose of jasmine-marbles. jasmine-marbles has been syntax sugar on top of the TestScheduler that wasn't relatively easy to use. If you want to use the vanilla TestScheduler, jasmine-marbles shouldn't prevent you from doing that.

If you want testing lib which uses TestScheduler.run functionality - take a look at rxjs-marbles: https://github.com/cartant/rxjs-marbles

I'm also confused, but merely by the fact that jasmine-marbles still exists at all nowadays. What's the benefit over pure TestScheduler usage? In my eyes, the NgRx docs shouldn't confuse users and recommend to use a library that does not add anything on top of existing best practices (which is to use TestScheduler). That is, unless I'm missing something? If so, please forgive me, but it's hard to find out because jasmine-marbles doesn't even have a proper README file or any other docs, at least I cannot find anything in its repo https://github.com/synapse-wireless-labs/jasmine-marbles (this is the right repo, isn't it? ... because there's nothing linked from the npm page of jasmine-marbles package).

Same feedback here, I've been confused.
For example, by the need to parametrize the RxJS scheduler in an effect of the example project https://github.com/ngrx/platform/blob/6a065fefaa824858eb49ed1ef10019c543405999/projects/example-app/src/app/books/effects/book.effects.ts#L35
or by this comment in data's EntityEffects (https://github.com/ngrx/platform/blob/6a065fefaa824858eb49ed1ef10019c543405999/modules/data/src/effects/entity-effects.ts#L33
I understand that RxJS TestScheduler is more verbose, but it is well documented and pretty clear.

Anyway, I'll be glad to make a PR to remove jasmine-marbles from the example project (and clarify the documentation) if you agree.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mappedinn picture mappedinn  路  3Comments

brandonroberts picture brandonroberts  路  3Comments

NathanWalker picture NathanWalker  路  3Comments

dmytro-gokun picture dmytro-gokun  路  3Comments

oxiumio picture oxiumio  路  3Comments