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

Examples or reference materials. #25

Closed
hally9k opened this issue Feb 25, 2018 · 19 comments
Closed

Examples or reference materials. #25

hally9k opened this issue Feb 25, 2018 · 19 comments
Labels

Comments

@hally9k
Copy link

hally9k commented Feb 25, 2018

Firstly, many thanks for some awesome work here.

I have just spent the morning getting everything working with our Jest setup. Struggled a bit TBH. I spent a lot of time hopping between blogs and docs trying to piece together a decent real world test example.

My output is here: https://github.com/hally9k/rxjs-marbles-jest-example

This issue is really 2 questions:

  1. Should we add some examples like this to the source or links to the readme to help future noobs like myself?

  2. Does my output referenced serve as good example or have I missed a better way to do this? (It doesn't feel super clean the way that I am mocking delay...) 🤔

@cartant
Copy link
Owner

cartant commented Feb 25, 2018

Regarding the first question: yes, in light of this issue, from yesterday, I've added a task to my TODO list to improve the documentation. It's grown over time and could do with being reorganised. And, yes, the examples could be improved.

As for the second, I will check out the example you've referenced a little later.

Thanks for raising the issue.

@hally9k
Copy link
Author

hally9k commented Apr 10, 2018

@cartant Did you ever get round to looking at this for me?

@cartant
Copy link
Owner

cartant commented Apr 10, 2018

I've had a look at your example. Thanks for the effort that you've put into it.

However, I think tests that involve schedulers should use the TestScheduler exposed by the context - via m.scheduler - rather than mocking the operators themselves. Your example uses an implicit scheduler, as delay defaults to using the AsyncScheduler.

When testing with schedulers you have two options:

I would do something like this:

describe('Delayer', () => {
    it(
        'should complete when it receives a payload with a delay value of 4 seconds or greater.',
        marbles(m => {

            const values = {
                a: { value: 'alpha', delay: 10 },
                b: { value: 'beta', delay: 20 },
                c: { value: 'charlie', delay: 30 },
                d: { value: 'delta', delay: 40 }
            }

            const producer = m.hot('^a-b--c---d|', values)
            const expected = m.hot('^-a--b---c|', values)

            m.bind();
            const observable = delayer(producer)
            m.expect(observable).toBeObservable(expected)
        })
    )
})

With delayer composed like this (note that to simplify the expected values, x is not mapped to x.value):

module.exports = subject =>
    Observable.from(subject)
        .takeWhile(x => x.delay < 40)
        .concatMap(x => Observable.of(x).delay(x.delay))
        .do(console.log)

The clear benefit is that the expected marble diagram includes the delays. (Note that you will need to reduce your delay values, as each character in the diagram counts as 10 frames.)

I've created a v6 branch that includes the changes necessary to support RxJS version 6. At some stage around the release date for version 6, I will rearrange the README.md and will create some working examples within an examples directory in the repo. I'll add some basic examples and if others want to submit PRs with further examples, they would be welcome to do so.

@hally9k
Copy link
Author

hally9k commented Apr 10, 2018

@cartant Thanks so much for taking the time, this was SUPER helpful.
One question: If I want to test this delayer with the original larger delay values can I change the amount of frames that each character in the diagram represents to 100 or 1000? A frame resolution setting I suppose 🤔

@cartant
Copy link
Owner

cartant commented Apr 10, 2018

I don't think you will be able to change it.

The maximum frame of 750 is a constant and it's passed to the VirtualTimeScheduler in the TestScheduler constructor. And the frameTimeFactor is a protected (in TypeScript) static property in VirtualTimeScheduler. So changing frames per character is not really something that's supported/anticipated.

Around the time I published rxjs-marbles, OJ Kwon - he's on the RxJS core team - published rx-sandbox. It has its own implementation of TestScheduler which supports specifying - IIRC - the number of frames per character. You might want to check it out, if that's a must-have feature for you.

@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

Oh, so we can't test anything that is more than 3/4 of a second delayed? What is the motive for having this threshold?

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

The TestScheduler's primary purpose is the facilitate the testing of observables in RxJS itself. And it works well for that.

Basically, each char is 10 frames and the maximum of 750 is essentially based on that. Who wants to look at and align marble diagrams containing more than 75 characters?

There has been some discussion regarding using OJ's TestScheduler, but the TestScheduler in v6 is, AFAIK, unchanged. There is an experimental branch in which Ben Lesh and Jay Phelps are experimenting with functional observables and some re-implemented schedulers, but I've not looked at it too closely. Regardless, any changes will be some way off.

Do you have a large number of tests that require specific, long durations that cannot be parameterised in the tests? That is, you cannot specify a specific, shorter delay for testing purposes? If you have only a few, you might want to test using just the VirtualTimeScheduler.

@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

Thanks for the references, I have a bit reading to do no doubt. I'm a little out of touch with the movement in the RxJS space, clearly

In our project we have some timings that are part of the characteristics under test. If the 75 char limit is an arbitrary limit based on assumed use cases maybe we could make it configurable, or is there some technical reason we would want to limit it to a specific length?

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

There's no reason why it shouldn't be more configurable. It's just that it was built to test the RxJS library and it does that. The authors didn't need it to do anything else, so it doesn't - YAGNI and all that.

I'll make an note to have a closer look at the internals, later, to see if there's any way I can just clobber it. I doubt it would be a bigger hack than m.bind() 😄

@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

Nice! Let me know how you get on ;) Thanks again!

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

Actually, it'd be pretty simple to add this to rxjs-marbles. I'll likely do it tonight - i.e. in 12 hours or so.

@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

@cartant So I've been jamming a bit with rx-sandbox like you said and the test scheduler has the features I want but there's no good way to inject the test scheduler into the operaters when they're in the production code. I read your bind function on the context to get a feel for how you're doing it (didn't fully grok it TBH) and noticed that you can pass in an array of schedulers to the bind function. Have you tried passing the rx-sandbox test scheduler into the rxjs-marbles bind function?

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

Passing OJ's TestScheduler to bind won't help. bind monkey patches the now and schedule methods of the default schedulers to call the TestScheduler held by the Context.

However, you'd want to bind the default schedulers to the instance of OJ's scheduler.

I guess you could try passing and instance of OJ's scheduler to the Context, but to do that you'd be really digging down into the implementation of rxjs-marbles - which really tries to make the Context the primary/ only API surface.

Regarding bind, I saw this comment from Jay Phelps on Slack:

I think I'm going to hell. 🔥

import { async } from 'rxjs/scheduler/async';
async.schedule = testScheduler.schedule.bind(testScheduler);

And decided that - whilst it might see me, too, going to hell - it would make my life a lot easier when it comes to deeply nested schedulers. bind just monkey patches schedule and now and returns an array that contains the original functions - so that they can be restored when the context is torn down.

The code in Jay's comment might make things a little clearer.

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

What I would really like is to be able to specify the scheduler in the call to subscribe - as suggested by Paul Taylor in this comment,

Being able to do that might save me from eternal damnation. 🔥 😄 🔥

@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

@cartant I may need an invitation to see that slack comment? I'm not in that slack community. Or is that all the code you pasted there?

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

If you want an invite, just ask Tracy Lee. See this tweet or you could email her: tracy@thisdot.​co

And, yep, I pasted the whole comment.

@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

Oh you did paste the whole thing...? Not even slightly clearer though I'm afraid, I clearly require quite a bit of study of the RxJS innards. That snippet looks pretty crazy hacky though 😂
Anyway, I've reverted to passing mock delay values in for my tests for now which is a compromise.
If you manage to clobber this char limit I'll be able to test it properly tomorrow which will be waaay better for this particular application.
The frame resolution thing though, that would be awesome. Thanks for all your help today, I really appreciate it.

@cartant
Copy link
Owner

cartant commented Apr 11, 2018

I've published 2.4.1 which adds a reframe method to the context, It allows the specification of the frames per character and the maximum number of frames.

There's a fixture that demonstrates the usage:

it("should support reframing", marbles((m) => {

    m.reframe(100, 10000);

    const duration = m.time("--|");
    expect(duration).to.equal(200);

    const source =   m.cold("--(a|)");
    const expected = m.cold("----(a|)");
    m.expect(source.delay(duration, m.scheduler)).toBeObservable(expected);
}));

reframe must be called before time, cold, hot or flush are called.

The clobbering is restored after the context is torn down, so the reframing is local to each test.

If you always want to use a particular framing configuration, you could create a module and could export a wrapped marbles function in a manner similar to that used for the framework-specific marbles functions in rxjs-marbles:

import { marbles as _marbles } from "rxjs-marbles/jest";

export function marbles(func: (m: Context, ...rest: any[]) => any): any {
    return _marbles((m: Context, ...rest: any[]) => {
        m.reframe(100, 10000);
        return func(m, ...rest);
    });
}

Closing this. When RxJS v6 is released, I'll release a version of rxjs-marbles that supports it and that version will include examples. At that stage, contributions of further example will be welcome.

@cartant cartant closed this as completed Apr 11, 2018
@hally9k
Copy link
Author

hally9k commented Apr 11, 2018

You sir, are an absolute legend! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants