ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.78k stars 3k forks source link

Productizing our test suite. #1775

Closed benlesh closed 6 years ago

benlesh commented 8 years ago

It's of paramount importance that we create some sort of test suite out of our tools that is easy for people to consume. Right now we're doing a lot of "magic" in our tests (like patching it to flush the test scheduler and setting a global test scheduler).

We really need to productize what we have so others can benefit from it more easily. That might mean we need to change the API a little. Perhaps something like:


import { rxSyncTest } from 'rxjs-test';

describe('some feature with observables', () => {
  it('should do things', () => {
    rxSyncTest((expect, { scheduler, hot, cold, time, etc }) => {
        // set up any tests in here, it will execute _immediately_ and when this block is done,
        // it will flush the scheduler

        let s = hot('---a---b---c---|');
        let e =     '---a---b---c---|');
        expect(s).toBe(e);
    });
  });
});

basically, the API I'm thinking of is:

interface Context {
  scheduler: TestScheduler;
  hot: (marbles: string) => HotObservable;
  cold: (marbles: string) => ColdObservable;
  time: (marbles: string) => number;
}

const rxSyncTest = ((expect: expectObservableFunc, context: Context) => void) => void;

This testing suite needs to:

  1. Be easy to use
  2. Be extensible
  3. Work with most major testing libraries (Jasmine, Mocha, QUnit, etc)

The names, of course, can be bikeshedded somewhat, but I'd like to get this out. I went with rxSyncTest because I want people to realize that this test is to be run in a totally synchronous way, and that they shouldn't be adding async code inside of it.

kwonoj commented 8 years ago

What about sandbox style api? Not exactly same interface though...


import { rxSandbox } from '...'

describe('suite',  () => {
    let sandBox: Context;
    let scheduler, hot, cold;

    beforeEach(() => {
        sandBox = rxSandbox.setup();
        { scheduler, hot, cold } = sandBox;
    });

    afterEach(() => {
        rxSandbox.tearDown(); // if global teardown required in implementation
    });

    it('test', () => {
        let s = hot('---a---b---c---|');
        let e =     '---a---(b|)';

        Observable.of(1).getMarble(); //throws, you can't get marble from real observable        

        //getMarble() implicitly flushes testscheduler for first time
        //it's called in single test scheduler context,
        //returns text marble from hot / cold observable
        const marble = s.take(2).getMarble();
        expect(marble).toBe(e);
    })
});

Cons :

staltz commented 8 years ago

I sort of prefer @kwonoj's proposal.

kwonoj commented 8 years ago

I can take this up once initial api design reaches agreement.

erykpiast commented 8 years ago

Is there a plan to provide API similar to one from Rx 4? I mean onNext, onCompleted and onError functions accepting value and time (ones from ReactiveTest)? Are marbles considered as the only way to test Rx 5 code? I'm thinking about eventual migration from v4 to v5. Changes in code seems trivial, but I can't find out how to adapt my tests without much effort.

kwonoj commented 8 years ago

Are marbles considered as the only way to test Rx 5 code?

Since marble test was introduced with v5, it hasn't explicitly excluded but implicitly didn't support any kind of backward compatibility to different version. I think testScheduler would be piece would require most of interop to support v4 as well, not sure how much it'll require at this moment.

Current plan is make migration to v5 perfectly first before consider expanding support.

erykpiast commented 8 years ago

I'm not sure I was understood correctly. I would to ask if I can test Rx 5 code similarly to Rx 4, using onNext, onCompleted and onError functions of ReactiveTest. I migrated my Rx 4 code to Rx 5 without any problems, but I can't really migrate my tests easily. I don't see any other solution than creating marbles from scratch. Seems to be easy, but also very boring and repetitive work, so I would like to avoid it.

kwonoj commented 8 years ago

Ah sorry, I understood in opposite way. No, I don't think there's immediate plan for those, migrating rx4 test to support rx5. It's just matter of priority, there are things to be wrapped first for public release of rx5. First goal is make rx5 test assertion suite first.

erykpiast commented 8 years ago

Well, I understand there are priorities. Thank you for your answer :)

paulpdaniels commented 8 years ago

Any movement on this? I started writing the chapter on testing in RxJS in Action and it would be great to have this ironed out.

kwonoj commented 8 years ago

I did some experiment based on my proposal, and seems it takes some time with those way. Haven't decided to go further down with current approach yet, may need some more time.

kwonoj commented 8 years ago

@erykpiast I also hit this issue recently and came to need v4 interface as well, wrote small package for those purpose. It is not 100% compliant and may possibly have debatable changes, so isn't submitted upstream PR.

benlesh commented 8 years ago

FWIW @jayphelps has been doing some work in this area. Perhaps he can comment

Zalastax commented 7 years ago

I've put some work into cleaning up the rendering of marble tests. Mainly reworking img2png/painter.js so that calculating the shapes and drawing them is done separately. This allowed me to add a new renderer that outputs svg files and has no dependencies, enabling rendering images directly in the browser if so wanted. I've made the changes in a separate repository but there shouldn't be any problem migrating it back. https://github.com/Zalastax/rxjs-marble-testing

I'd love to help with cleaning up the rest but would need direction from someone more involved before I get started. Ping @Zalastax on gitter so we can have a chat!

kwonoj commented 7 years ago

I'm bit inactive in this issue at this moment, unassigning myself and /cc @jayphelps driving primary effort for revisiting api surface. @Zalastax afaik there possibly can be breaking changes in api surface, so might need align with those as well.

Zalastax commented 7 years ago

We also need to accommodate libraries built on top of Rx. E.g. redux-observable want a TestScheduler but starts things of with their own ActionsObservable and does some preparation before calling testScheduler.expectObservable. There's a pretty good demo at http://jsbin.com/pufima/edit?js,output. I'd guess that @jayphelps already knows about this but I just hit into this problem in my own project.

avocadowastaken commented 7 years ago

Here how I made it work with jest.

Personally I hate globals, so I used TestScheduler explicitly. That also allowed me override it's expectObservable method for my personal needs.

kwonoj commented 7 years ago

I have published separate module https://www.npmjs.com/package/rx-sandbox for this effort. It indeed have breaking api changes, so I've published it as user-level module to gather opinions around. Major difference is it doesn't depends on any framework specific setups or using its own assertion but exposes metadata directly, so any preferred assertion can be done.

staltz commented 7 years ago

Just as a reminder of the challenges for testing that Ben raised during contributor days, and where we stand right now:

Question mark means it's an open problem, green check means there's a relatively straight-forward solution.

  1. APIs that default schedulers ❓
  2. Really long running schedulers ✅
  3. Fine-tuned tests ✅
  4. Comprehensive strategy docs ❓

Default schedulers are going to be hard to invade and change, because currently they are (e.g. for delay) passed as default function arguments. Does anyone have an idea for this? @kwonoj does rx-sanbox do that? (as far I can see, it doesn't)

Challenge (2) is basically solved with the idea of ...n... already in use in rx-sandbox.

Challenge (3) seems solved with frameTimeFactor in rxSandbox.create(autoFlush?: boolean, frameTimeFactor?: number): RxSandboxInstance.

Challenge (4) is about many things, the one I'm thinking most about is testing cases when multiple other schedulers are used, like changing between requestAnimationFrame scheduler, breadth-first scheduler, and depth-first scheduler. These choices actually affect how code behaves, and expected outputs, not just the idea of "threads" like e.g. RxJava has. Should we have multiple TestSchedulers?

kwonoj commented 7 years ago

does rx-sanbox do that? (as far I can see, it doesn't)

You're correct, currently it doesn't touch that at all, aimed to create feature-parity to testscheduler in Rx first. need to think about this as well as (4).

benlesh commented 6 years ago

This is done with TestScheduler.prototype.run