Platform: RFC: MockStore dispatch spy

[jtcrowson]

Following up on @MikeRyanDev's comment in #915:

It should also setup Store#dispatch to be a spy.

It is very common to spyOn dispatch in action-dispatching tests.

Describe any alternatives/workarounds you're currently using

• Manually set up the spy in each test:
  spyOn(store, 'dispatch');

Possible implementations:

1. make dispatch a spy (breaking change, will error if spy already exists within a test).
2. Add option to spy on dispatch to MockStoreConfig, with default value not to spy (non-breaking).

If accepted, I would be willing to submit a PR for this feature

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

[timdeschryver]

This can be tricky because this would depend on the user's test framework.

[brandonroberts]

I agree because of the variation between test frameworks. This is why it wasn't in the original design. With the MockStore, we're only spying to see if an action was dispatched, which we could alternatively do by listening to the ActionsSubject in the test.

[jtcrowson]

Should I use this issue to include listening to the ActionsSubject in the MockStore documentation?

[brandonroberts]

Possibly. We could also look at exposing some test API that keeps a list of the dispatched actions that you can assert against, and clear it if needed.

[jtcrowson]

Agreed. I also don't think we need to reuse the store provided ActionsSubject. Right now we have to skip(1) to get passed the @ngrx/store/init action. If we had a separate, non-ActionsSubject Subject, we wouldn't need to skip everywhere. Implementation would be simple:

dispatch<V extends Action = Action>(action: V) {
    this.<someInternalMockStoreSubject>.next(action);
  }

[brandonroberts]

I think we should still push values into both. The non-ActionsSubject would be returned through a method on the MockStore.

...
  dispatch<V extends Action = Action>(action: V) {
    this.<someInternalMockStoreSubject>.next(action);

    super.dispatch(action);
  }

  actions(): Observable<Action> {
    return this.<someInternalMockStoreSubject>.asObservable();
  }
...

[jtcrowson]

@brandonroberts what would you think about the mockstore maintaining a synchronous array instead? Using asynchronous streams like scannedActions$ is tricky and often leads to falsely passing tests if the test runs without a callback.

Here's one way we could implement synchronous testing with a stack:

public dispatchedActions: Action[] = [];
//...
dispatch<V extends Action = Action>(action: V) {
    this.dispatchedActions.push(action);
    super.dispatch(action);
}

get lastDispatchedAction(): Action | undefined {
    return this.dispatchedActions[this.dispatchedActions.length - 1];
}

resetDispatchedActions() {
    this.dispatchedActions = [];
}

Working tests:

it('should allow tracing dispatched actions', () => {
    const increment = { type: INCREMENT };
    const decrement = { type: DECREMENT };
    mockStore.dispatch(increment);
    mockStore.dispatch(decrement);
    expect(mockStore.lastDispatchedAction).toEqual(decrement);
    expect(mockStore.dispatchedActions).toEqual([increment, decrement]);
});

[jtcrowson]

@brandonroberts any thoughts on the above? A synchronous solution would be useful, IMO.

[brandonroberts]

I still think this should remain as a stream of dispatched actions. What about an accumulated array of actions as a stream? It fits better with the other APIs as a consumer, instead of it being a strictly synchronous API.

[david-shortman]

Would this API be good to pursue in a PR, then?

it('should allow tracing dispatched actions', () => {
    const increment = { type: INCREMENT };
    const decrement = { type: DECREMENT };
    mockStore.dispatch(increment);
    mockStore.dispatch(decrement);
    expect(mockStore.dispatchedActions$.pipe(last())).toBeObservable(cold('a', { a: decrement });
    // for convenience
    expect(mockStore.lastDispatchedAction$).toBeObservable(cold('a', { a: decrement });
    expect(mockStore.dispatchedActions$).toBeObservable(cold('ab', { a: increment, b: decrement });
});