cartant / rxjs-marbles

An RxJS marble testing library for any test framework
https://cartant.github.io/rxjs-marbles/
MIT License
301 stars 18 forks source link

[jest] "toEqual" timeouts with observe #48

Closed vire closed 6 years ago

vire commented 6 years ago

rxjs-marbles revision: a7ae5dc node: v9.5.0 rxjs: 6.2.0

copy&paste snippet below into ./examples/jest/observe-spec.ts

    it(
        "should property reduce `localState`",
        observe(() => {
            // would be passed as param
            const userState$ = of({
                userName: "jdoe",
                lastLogin: "1231241241"
            });

            const logout$ = never();
            const login$ = never();
            const actionsWithUser$ = merge(
                merge(logout$, login$),
                userState$.pipe(
                    map(userState => localState => ({
                        ...localState,
                        user: userState
                    }))
                )
            );

            const result$ = actionsWithUser$.pipe(
                startWith({ user: undefined, someOtherProp: "foo" }),
                scan((state, reduce) => reduce(state))
            );
            return result$.pipe(
                skip(1),
                tap(val =>
                    expect(val).toEqual({
                        someOtherProp: "foo",
                        user: { lastLogin: "1231241241", userName: "jdoe" }
                    })
                )
            );
        })
    );

when used with toBe there is an proper failure because Object.is equality check

when I use expect(val).toEqual the test timeouts... Timeout - Async callback was not invoked within the 5000ms timeout specified by jest.setTimeout.

EDIT: it also might be related to the skip() call but with scalar state - only number this test works

cartant commented 6 years ago

The observable under test has to complete or error, so this looks like the expected behaviour, to me.

vire commented 6 years ago

just figured out the expect has to be written in a way to pick a specific value using skip+take :/

skip(1),
take(1),
tap(
// expect toEqual
)

@cartant is there a better way to make an assertion about a specific value in the stream?

a) using marbles ? b) end the stream under test ? c) something else ?

cartant commented 6 years ago

It should be pretty easy to test that with a marble test. Something like this should do it:

it(
    "should property reduce `localState`",
    marbles(m => {

        const initState = { user: undefined, someOtherProp: "foo" };
        const userState = { userName: "jdoe", lastLogin: "1231241241" };
        const expectedState = { someOtherProp: "foo", user: userState };

        const userState$ = m.cold("--u", { u: userState });
        const logout$ =    m.cold("---");
        const login$ =     m.cold("---");
        const expected$ =  m.cold("i-e", { i: initState, e: expectedState });

        const actionsWithUser$ = merge(
            merge(logout$, login$),
            userState$.pipe(
                map(userState => localState => ({
                    ...localState,
                    user: userState
                }))
            )
        );

        const result$ = actionsWithUser$.pipe(
            startWith(initState),
            scan((state, reduce) => reduce(state))
        );
        m.expect(result$).toBeObservable(expected$);
    });
);

I would recommend attempting to test with a marble test and resort to observe only if writing a marble test proves to be too complicated.

cartant commented 6 years ago

Closing this, but feel free to ask further questions.

vire commented 6 years ago

@cartant I'm curious if you could point me towards the right direction - what's the best way to test streams that get updated by calling a subject-based handler - in my gist a simplified version of https://github.com/acdlite/recompose/blob/master/docs/API.md#createeventhandler

here is a test https://gist.github.com/vire/806158c18fe0f14637cc37d0514d3f08 (I've spent couple of hours to make it work, but without success.

Thx for any advice (I can create a PR of the example)

cartant commented 6 years ago

The thing to remember with marble tests is that for the virtual scheduling to work, the root sources have to be observables that were created by the TestScheduler. That is, they have to be created by calling either m.hot or m.cold.

Your test doesn't work because the subject is not such an observable, so the TestScheduler simply doesn't know what to do with it - when it comes to virtual scheduling. That is, the call to next won't fit within the TestScheduler's concept of virtual time because you've called it imperatively - in real time - within the test.

The solution is to use another source to either trigger or map to the subject. See the ReplaySubject tests for an example of the latter. For the former, you could do something like this:

const trigger$ =   m.cold("--t");
const triggerSub =        "^--";
const expected$ =  m.cold("i-u", {
    i: initialState,
    u: updatedState
});
const tapped$ = trigger$.pipe(tap(() => increment(42));
// You'll need an expectation for tapped$ to be subscribed to.
// Either:
m.expect(tapped$).toBeObservable(trigger$);
// Or:
m.expect(tapped$).toHaveSubscriptions(triggerSub);
m.expect(source).toBeObservable(expected$);

It's likely possible to also achieve this with explicit calls to flush, but I think a declarative approach that lets the TestScheduler do its thing with virtual time is the best way to go.


Also, using m.cold to create the expected observable is just a convenience. Something that I pinched from jasmine-marbles. I find it useful because it lets you specify the expected observable and values alongside the sources. Usually, the toBeObservable takes a marble diagram string and an optional map of values, but calling it that way means that the values are specified separately to the marble diagram (as the marble diagrams are best declared one-after-the-other and aligned).

vire commented 6 years ago

with the the proposed tapped$ stream and expecting a trigger$ to be executed it works like chart. I knew it will be something related to the fact the Subject() gets executed as sideffect outside the test, but didn't had a clue how to execute it with test.

I've tried to call tap() with increment and merge it with the main stream but it did not worked out...

If you want I can make out of this snippet an example case

vire commented 6 years ago

your propose solution works for the first case

m.expect(tapped$).toBeObservable(trigger$);

but for the second approach (expecting subs)

m.expect(tapped$).toHaveSubscriptions(triggerSub);

I get an exception

Expected a hot or cold test observable with subscriptions.

Not sure why, where const tapped$ = trigger$.pipe(tap(() => increment(42))); should produce the expected pattern...

cartant commented 6 years ago

Yeah, that error makes sense, so I guess the expectation has to be toBeObservable.

If you want to create a PR to add an example, that would be cool.

deser commented 5 years ago

Is there a way to use toMatchSnapshot within toBeObservable ?

cartant commented 5 years ago

@deser Not that I'm aware of.

Also, a tip: rather than comment on a closed issue, it's always better to open a new issue and reference any related issues from within the new one. All of the maintainers I know feel the same way about this. One of these days I'll set up the lock bot to lock old, closed issues.

deser commented 5 years ago

Ok, thanks