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

Testing story #108

Open
jayphelps opened this Issue Sep 11, 2016 · 31 comments

Comments

Projects
None yet
@jayphelps
Member

jayphelps commented Sep 11, 2016

We need to improve the documentation around testing. Partially the problem is that testing RxJS v5 isn't very well documented either, but we can't use that as an excuse forever.


Edit: this thread contains plenty of great ideas, we just need someone willing to PR better documentation around them.

@jayphelps jayphelps added this to the v1.0.0 milestone Sep 11, 2016

@jayphelps

This comment has been minimized.

Member

jayphelps commented Sep 22, 2016

Experimenting internally with some changes to RxJS + a expectEpic helper that allows this:

const searchQueryEpic = (action$, store, api) =>
  action$.ofType(SEARCH_QUERY)
    .debounceTime(100)
    .switchMap(action =>
      api.search(action.payload)
        .map(searchQueryFulfilled)
    );

// test

expectEpic(searchQueryEpic, [
  'a----------b-c----------d-----------|',
  '----------a------------c----------d-|'
], {
  a: searchQuery('first'),
  b: searchQuery('second'),
  c: searchQuery('third'),
  d: searchQuery('fourth')
}, {
  a: searchQueryFulfilled('first_GOOD'),
  c: searchQueryFulfilled('third_GOOD'),
  d: searchQueryFulfilled('forth_GOOD')
}, null, {
  search: query => Observable.of(query + '_GOOD')
});

// also possibly a marble builder for controlling time in ways
// that regular marbles can't or don't represent easily
// e.g. how would you otherwise represent 13ms or 10000ms?

cold.builder()
  .next(searchQuery('first'))
  .sleep(100)
  .next(searchQuery('second'))
  .sleep(10)
  .next(searchQuery('third'))
  .sleep(100)
  .next(searchQuery('fourth'));

// Some people may prefer this approach regardless cause
// they are used to this more imperative-style testing.

lmk your thoughts (if you can figure out what this is doing e.g. marbles and stuff)

@rgbkrk

This comment has been minimized.

Member

rgbkrk commented Sep 22, 2016

The marbles make sense here. The way I read this was that expectEpic takes

  • epic
  • marble diagram
  • actions to hand to epic
  • expected actions in order
  • null is currently taking the place of where the store could be passed in
  • api (which can be a mock)
@jayphelps

This comment has been minimized.

Member

jayphelps commented Sep 22, 2016

@rgbkrk bingo--technically speaking, all args after the actions stuff are just transparently passed along to your epic, so if you pass a store, api, or anything else it's just passed along.

The arg order/format of the marble/actions things is up in the air, but this was the best I could come up with that makes sense while still making it "pretty" for the common cases.

e.g. this might be more "correct" (I originally used this)

expectEpic(searchQueryEpic,
  ['a----------b-c----------d-----------|',  {
    a: searchQuery('first'),
    b: searchQuery('second'),
    c: searchQuery('third'),
    d: searchQuery('fourth')
  }],
  ['----------a------------c----------d-|', {
    a: searchQueryFulfilled('first_GOOD'),
    c: searchQueryFulfilled('third_GOOD'),
    d: searchQueryFulfilled('forth_GOOD')
  }], null, {
  search: query => Observable.of(query + '_GOOD')
});

...but I found not lining up the input/output marble lines sequentially makes it harder to understand and reason at a glance. Off by one - bar and such

@jayphelps jayphelps self-assigned this Sep 22, 2016

@steida

This comment has been minimized.

steida commented Oct 16, 2016

From a practical point of view, I would rather have the second arg reserved for any configurable dependencies, not a limited store only. A third argument is too much. I am using this pattern within injectMiddleware and it works well.

// Like redux-thunk, but with just one argument.
const injectMiddleware = deps => ({ dispatch, getState }) => next => action =>
  next(typeof action === 'function'
    ? action({ ...deps, dispatch, getState }) // Note custom and default deps are mixed
    : action
  );

This is my current workaround

import 'rxjs';
import * as appEpics from './app/epics';
import { combineEpics, createEpicMiddleware } from 'redux-observable';

const epics = [
  appEpics,
];

const configureEpics = (deps) => {
  // just extract functions from imported objects
  const flattenEpics = epics.reduce((prev, curr) =>
    prev.concat(Object.keys(curr).map(k => curr[k]))
  , []);
  // HACK Epic
  const epicsWithDeps = flattenEpics.map(epic =>
    (action$, store) => epic(action$, { ...store, ...deps })
  );
  const rootEpic = combineEpics(...epicsWithDeps);
  const epicMiddleware = createEpicMiddleware(rootEpic);
  return {
    epicMiddleware
  };
};

export default configureEpics;

So now I can use proper dependency injection.

export const appStartEpic = (action$, { storageEngine }) => {
  console.log(storageEngine);
  return action$.ofType(APP_START)
    .delay(1000) // Asynchronously wait 1000ms then continue
    .mapTo({ type: 'PONG' });
}

Proposal for a new API

  const epicMiddleware = createEpicMiddleware(rootEpic, deps); // That's all we need.
@jayphelps

This comment has been minimized.

Member

jayphelps commented Oct 16, 2016

Overloading the store runs the risk of colliding with any methods redux adds to it later, and also prevents you from using any of the handy store mocking helpers (without awkward extra work)

@jayphelps

This comment has been minimized.

Member

jayphelps commented Oct 16, 2016

@steida if you do prefer to merge the two, this is perfectly acceptable:

const rootEpic = (action$, store) =>
  combineEpics(epic1, epic2)(action$, { ...deps, ...store });
@steida

This comment has been minimized.

steida commented Oct 16, 2016

@jayphelps Aren't epic1, epic2 combined on every change? rootEpic is called only once?

@jayphelps

This comment has been minimized.

Member

jayphelps commented Oct 16, 2016

@steida epics are basically factories, (typically) invoked only once, then actions pump through their Observables. I say typically, because there are ways of "spawning" short lived epics, but that isn't documented and not often used. So rootEpic is called only once in a typical app.

@steida

This comment has been minimized.

steida commented Oct 16, 2016

@jayphelps Perfect answer. Thank you. I'm in the middle of the process of adding redux-observable in https://github.com/este/este.

@jayphelps

This comment has been minimized.

Member

jayphelps commented Oct 16, 2016

@steida

This is how Rx works. You declaratively set up transforms using operators, then things are next'd into them.

// this pseudo code to illustrate kinda how it works,
// I'm not saying you should use it this way!
const rootEpic = (action$) => {
  console.log('only logs once');
  return new Observable(observer => {
    console.log('only logs once');
    return action$.subscribe(action => {
      console.log('logs for every action received');
      const newAction = // do transforms
      observer.next(newAction); // output of your epic
    });
  });
};
@steida

This comment has been minimized.

steida commented Oct 16, 2016

Still learning things. But I suppose redux-observable is the final answer for async redux.

@rgbkrk

This comment has been minimized.

Member

rgbkrk commented Oct 16, 2016

I suppose redux-observable is the final answer for async redux

I welcome this.

@StarkTS

This comment has been minimized.

StarkTS commented Nov 17, 2016

what if i have a conditional, one to return an action, and other to return nothing? how can avoid the "undefined "type" property" error?

@jayphelps

This comment has been minimized.

Member

jayphelps commented Nov 17, 2016

@Casy you'll need to provide some code for me to fully understand where you're referring to, but you may be talking about something like this:

const somethingEpic = action$
  action$.ofType(SOMETHING)
    .mergeMap(action => {
      if (action.payload === 'JUST....DOIT!') {
        return Observable.ajaxOrWhatever()
          .map(response => somethingFulfilled(response));
      } else {
        return Observable.empty();
      }
    });

Here, if the payload isn't what we want, we mergeMap an Observable.empty() which is an observable that emits nothing, it just complete()s. So when that condition happens, our epic won't emit anything either.

@StarkTS

This comment has been minimized.

StarkTS commented Nov 17, 2016

@jayphelps Observable.empty() is exactly what i'm looking for, once more thanks jey 👍

@cmelion

This comment has been minimized.

Contributor

cmelion commented Nov 18, 2016

I've been following the testing suggestions from the documentation (scroll to the bottom) and have been able to test most of my epics so far with the minor modifications below:

 /**
 * expectEpic - Test helper for redux-observable epics
 *
 * @param epic     - the redux-observable epic to test
 * {
 * @param action   - the action the epic is listening for
 * @param call     - (optional) a local sinon.spy used to get/set/reset api calls
 * @param callArgs - (optional) An array containing the api method, followed by any params
 * @param done     - (optional) mocha callback for async tests
 * @param expected - marble notation for the expected result
 * @param replace  - (optional) allows replacement of fields using timestamps or other generated values
 * @param response - the expected payload
 * @param store    - (optional) a reference to the redux store
 * }
 */
 const expectEpic = (epic, {action, call, callArgs, done, expected, replace, response, store}) => {};
        

There are still a few kinks to work out before I can work this into a pull request, for now I've published a standalone npm module:
https://github.com/cmelion/redux-observable-test-helpers

Sample usage:
http://jsbin.com/dagiguw/embed?js,output
http://jsbin.com/vegonak/embed?js,output
http://jsbin.com/lefuva/edit?js,output

Alternative approach using MockXMLHttpRequest instead of expectEpic/indirect-call:
http://jsbin.com/pudaqe/embed?js,output

@jdetle

This comment has been minimized.

Contributor

jdetle commented Nov 21, 2016

Hi @jayphelps and others, I have think the way nteract tests rxjs and redux observable to be fairly low overhead, although maybe not for everyone.

import { ActionsObservable } from 'redux-observable';
const chai = require('chai');
const expect = chai.expect;
const sinon = require('sinon');

import { executeCell } from '../../../src/notebook/actions';
import { executeCellEpic } from '../../../src/notebook/epics/execute';

describe('epic', () => {
  it('Epics with the appropriate input and output of actions', (done) => {
    const action$ = ActionsObservable.of(executeCell('id', 'source')));
    const actionBuffer = [];
    const responseActions = executeCellEpic(action$, store);
    const subscription = responseActions.subscribe(
      (x) => actionBuffer.push(x.payload.toString()), // Every action that goes through should get stuck on an array
      (err) => expect.fail(err, null), // It should not error in the stream
      () => {
        expect(actionBuffer).to.deep.equal(['Error: kernel not connected']); // ;
        done();
      },
    );
  });
});

is one test for the following epic:

/**
 * the execute cell epic processes execute requests for all cells, creating
 * inner observable streams of the running execution responses
 */
export function executeCellEpic(action$, store) {
  return action$.ofType('EXECUTE_CELL')
    .do(action => {
      if (!action.id) {
        throw new Error('execute cell needs an id');
      }
      if (typeof action.source !== 'string') {
        throw new Error('execute cell needs source string');
      }
    })
    // Split stream by cell IDs
    .groupBy(action => action.id)
    // Work on each cell's stream
    .map(cellActionStream =>
      cellActionStream
        // When a new EXECUTE_CELL comes in with the current ID, we create a
        // a new stream and unsubscribe from the old one.
        .switchMap(({ source, id }) => createExecuteCellStream(action$, store, source, id))
    )
    // Bring back all the inner Observables into one stream
    .mergeAll()
    .catch(createErrorActionObservable(ERROR_EXECUTING));
}

Additionally, debugging these tests with the chrome dev tools is extremely useful:
mocha --debug-brk + [whatever options you had for testing previously] allows for easy inspection after inserting debugger statements in your tests.

@jayphelps

This comment has been minimized.

Member

jayphelps commented Nov 21, 2016

@jdetle it shouldn't be necessary to provide an error handler mapping to expect.fail. If you don't provide a handler, it should bubble and be caught as normal exceptions would and cause the test to fail with the correct formatting. If not, please let me know.

You might also find .toArray() a cleaner solution, but maybe not 😄

it('Epics with the appropriate input and output of actions', (done) => {
  const action$ = ActionsObservable.of(executeCell('id', 'source')));

  executeCellEpic(action$, store)
    .toArray()
    .subscribe(actions => {
      expect(actions).to.deep.equal([
        kernelNotConnected() // whatever action creators
      ]);

      done();
    });
});
@jdetle

This comment has been minimized.

Contributor

jdetle commented Nov 21, 2016

Ooh thanks, I'll have to check .toArray out. As an aside, I really am a fan of your responses. So clear an concise 👍

@jayphelps

This comment has been minimized.

Member

jayphelps commented Nov 21, 2016

@jdetle thanks! toArray() will basically drain the source, buffering each next'd item into an array until the source completes, once it completes, it then emits that single array of next'd values to the subscriber. So it handles that boilerplate for us. If the source never completes, the test will timeout, likely signalling that your epic is misbehaving (unless it's supposed to never complete naturally, which is very very rare, and something you'd need to test explicitly)

@jayphelps

This comment has been minimized.

Member

jayphelps commented Nov 21, 2016

@cmelion ultimately we're waiting for changes in RxJS itself, most notably TestScheduler needs to be autoinjected into any async operator e.g. debounceTime so that it can properly control virtual time. We also are working on a way to to represent arbitrary time durations; right now the marble diagrams ticks represent increments of 10 virtual ms per -, which isn't ideal in real world situations where you have large numbers like 1000ms and possibly non-divisible numbers like 555ms.

e.g. right now 1000ms represented in marble would have to be 100 dashes. ---------------------------------------------------------------------------------------------------- which is unmaintainable. It also won't work out of box because there is a max of 750ms which we're still investigating

@cmelion

This comment has been minimized.

Contributor

cmelion commented Nov 21, 2016

@jayphelps yeah, the 100 dashes thing was a bit disconcerting for me. I opted to pass the interval as part of the api to support mocking a different interval but ultimately punted. My bigger concern was that the interval path was not picked by the test framework without calling done. Once that hurdle was overcome I was able to get 100% code/branch coverage.

http://jsbin.com/lefuva/embed?js,output

export const api = {
    fetchIssues: () =>
        ajax.getJSON(BASE_URL),
    fetchInterval: () => 10000
};


const animateYoutrackIssuesEpic = (action$, store, call = indirect.call) =>
    action$.ofType(ActionTypes.ANIMATE_YOUTRACK_ISSUES, ActionTypes.PAUSE_ANIMATE_YOUTRACK_ISSUES)

        .switchMap(action => action.type === ActionTypes.ANIMATE_YOUTRACK_ISSUES ?

            /// If Animating then start the interval that cycles through issues and sets CSS animation state.
            Observable.interval(call(api.fetchInterval))
                .map(() => Actions.goToYoutrackIssue({
                    direction: 1,
                    count: action.count
                })) :

            /// If Paused then cancel the observable, stopping animation
            Observable.never()
        );

it('calls GOTO_YOUTRACK_ISSUE upon ANIMATE_YOUTRACK_ISSUES', (done) => {
    const payload = {issue: [{}, {}]};

    expectEpic(YouTrackIssuesEpic, {
        expected: ['-a', {
            a: {type: ActionTypes.GOTO_YOUTRACK_ISSUE, payload: {direction: 1, count: payload.issue.length}}
        }],
        action: ['a|', {
            a: {type: ActionTypes.ANIMATE_YOUTRACK_ISSUES, count: payload.issue.length}
        }],
        response: ['-'],
        call: call,
        callArgs: [api.fetchInterval],
        done: done
    });
});

      
@jyane

This comment has been minimized.

jyane commented Dec 5, 2016

I have found another test story.
IMO, The problem is native observable does not have ofType method.
ofType method is extended by ActionObservable.

So, I have extended testScheduler.createColdObservable (add ofType method to cold).
Here is an example of test story.

const createCold = (testScheduler) => (marbles, values) => {
  const input$ = testScheduler.createColdObservable.bind(testScheduler)(marbles, values);

  /**
   * @see 'https://github.com/redux-observable/redux-observable/blob/fd393a1adf359381c98ca494bc5dc2cac80f6d07/src/ActionsObservable.js'
   */
  input$.ofType = (...keys) => {
    return input$.filter(({ type }) => {
      const len = keys.length;
      if (len === 1) {
        return type === keys[0];
      } else {
        for (let i = 0; i < len; i++) {
          if (keys[i] === type) {
            return true;
          }
        }
      }
      return false;
    });
  };

  return input$;
};

An epic will test

const fetchUser = (action$) =>
  action$.ofType('FETCH_USER').mergeMap((action) =>
    apiClient.getUser()
      .map((res) => res.data)
      .map(fetchUserSuccess)
      .catch((e) => Observable.of(fetchUserFailure(e)))
  );

Usage in Jest:

describe('Test userEpic', () => {
  let testScheduler;
  let cold;

  beforeEach(() => {
    jest.resetModules();

    testScheduler = new TestScheduler((expected, actual) => {
      expect(expected).toEqual(actual);
    });

    cold = createCold(testScheduler);
  });

  it('should return FETCH_USER_SUCCESS when success', () => {
    jest.mock('src/utils/client', () => ({
      apiClient: {
        getUser: () => Rx.Observable.of({ data: { foo: 'bar' }})
      }
    }));
    const userEpic = require('src/epics/user').default;
    const input$  = cold('---a--', { a: { type: 'FETCH_USER' }});
    const expect$ =      '---b';
    const test$ = userEpic(input$, testScheduler);

    testScheduler.expectObservable(test$).toBe(expect$, { b: { type: 'FETCH_USER_SUCCESS', payload: { foo: 'bar' }}});
    testScheduler.flush();
  });
});
@jayphelps

This comment has been minimized.

Member

jayphelps commented Dec 5, 2016

@jyane ActionsObservable expects a source observable as an argument, where it gets the actions from. So you can do this:

const input$  = cold('---a--', { a: { type: 'FETCH_USER' }});
const action$ = new ActionsObservable(input$);
@jyane

This comment has been minimized.

jyane commented Dec 5, 2016

@jayphelps Thank you for your good suggestion! It is really useful for me.

@dfbaskin

This comment has been minimized.

dfbaskin commented Mar 29, 2017

So given an epic that does something on a regular basis (using interval), I had created a test for it like this:

import {ActionsObservable as Observable} from 'redux-observable'
import {VirtualTimeScheduler} from 'rxjs/scheduler/VirtualTimeScheduler';

import 'rxjs/add/observable/empty';
import 'rxjs/add/observable/interval';
import 'rxjs/add/operator/delay';
import 'rxjs/add/operator/concat';
import 'rxjs/add/operator/reduce';
import 'rxjs/add/operator/takeUntil';
import 'rxjs/add/operator/switchMap';
import 'rxjs/add/operator/toArray';
import 'rxjs/add/operator/do';

const intervalSeconds = 60;
let scheduler;

const intervalEpic = (action$) => {
    return Observable
        .interval(intervalSeconds * 1000, scheduler)
        .takeUntil(action$.reduce((v, action) => 0, 0))
        .switchMap(() => {
            return Observable.of({
                type: "SOME_ACTION"
            })
        });
};

export function runObservableEpicTest(observable, epic, done, verify) {
    let mockStore = {
        getState() {
            return {};
        }
    };
    Observable
        .empty()
        .concat(epic(observable, mockStore))
        .toArray()
        .do((actions) => verify(actions))
        .subscribe({
            error: (err) => done.fail(err),
            complete: () => done()
        });
}

describe('test with interval', () => {

    beforeEach(() => {
        scheduler = new VirtualTimeScheduler();
    });

    it('should allow testing of interval', (done) => {
        let observable = Observable
            .empty()
            .delay((intervalSeconds + 1) * 1000, scheduler);
        runObservableEpicTest(observable, intervalEpic, done, (actions) => {
            expect(actions).toEqual([
                { type: "SOME_ACTION" }
            ]);
        });
        scheduler.flush();
    });
});

But as you can see, I ended up having to pass the scheduler into the epic itself. Not sure there is a way around that at this point (until perhaps the testing updates are released).

@alyahmedaly

This comment has been minimized.

alyahmedaly commented Jul 7, 2017

any update on this

@jayphelps

This comment has been minimized.

Member

jayphelps commented Jul 7, 2017

@redux-observable redux-observable deleted a comment from hedgerh Jul 7, 2017

@alyahmedaly

This comment has been minimized.

alyahmedaly commented Jul 7, 2017

i used Injecting Dependencies with jest and it's working as expected

    const action$ = ActionsObservable.of(fromOrders.fetchOrders(1, 10));
    const dependencies = {
      orderService: {
        getOrders: () => Observable.of(response)
      }
    };

    const expectedAction = {
      type: fromOrders.actionTypes.FETCH_ORDERS_SUCCESS,
      page: response.orders.page,
      pageSize: response.orders.pageSize,
      totalItems: response.orders.totalItems,
      items: response.orders.items
    };

    return OrdersEpic(action$, store, dependencies)
      .toArray()
      .toPromise()
      .then(actions => expect(actions, [expectedAction]));
@clivend

This comment has been minimized.

Contributor

clivend commented Apr 19, 2018

I'm working on this now, maybe it helps
#144 (comment)

@huy-nguyen

This comment has been minimized.

huy-nguyen commented May 27, 2018

With the addition of state$ stream in v1, I think marble testing is now feasible and is a great way to test epics. I wrote a tutorial to show beginners how to do it. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment