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

RFC: Mock Store #915

Closed
TeoTN opened this issue Mar 15, 2018 · 73 comments
Closed

RFC: Mock Store #915

TeoTN opened this issue Mar 15, 2018 · 73 comments

Comments

@TeoTN
Copy link
Contributor

TeoTN commented Mar 15, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[x] Feature request
[ ] Documentation issue or request

What is the current behavior?

There's no simple way to mock a Store for testing purposes.

Expected behavior:

When testing a component or an effect, I'd like to be able to inject a fake Store to the tested entity. It became increasingly non-trivial after the pipable operators were introduced.

For instance, one may want to test behavior component with a given state that is a result of many various actions. Similarly, one may want to test an effect with various fixtures for state.
Across the web there are multiple snippets with custom implementation of MockStore that extends either Store or BehaviorSubject but all of them are outdated. The presence of such snippets suggests though that there's a real need for some testing utils.
I propose introduction of @ngrx/store/testing module which would contain provideMockStore for providing MockStore in place of Store and an implementation of MockStore that provides nextMock method that replaces current state just like BehaviorSubject.next does on Subject.

Version of affected browser(s),operating system(s), npm, node and ngrx:

Newest ngrx, reasonably new npm, node, Chrome and Windows.

Other information:

I'll make a contribution with MockStore class and a provider, if you feel it's a valid issue.

@TeoTN TeoTN changed the title Mock Store RFC: Mock Store Mar 16, 2018
@MikeRyanDev
Copy link
Member

Great suggestion! @brandonroberts and I have a mock Store implementation we use internally that works similarly to how you propose. It should also setup Store#dispatch to be a spy.

@vitaliy-bobrov
Copy link
Contributor

Great proposal! IMHO will be nice to have a separate package inside platform with all testing helper tools.

@hoangdovan
Copy link

Yeah, We really need it!

@alex-okrushko
Copy link
Member

This has been raised a number of times already and I'd like to get the consensus whether mock Store is needed or not :)

E.g. in ngrx/store#128 @robwormald says:

If you want to implement a mock version (which, IMHO, is entirely unnecessary, but feel free...)

The same recommendation is at current testing guide for the store https://github.com/ngrx/platform/blob/master/docs/store/testing.md#providing-store-for-testing

Reducing state is synchronous, so mocking out the Store isn't required.

Internally at Google I've been recommending to avoid mocking the Store as well - my suggestion was to use the real Store and if it's needed to be in certain state for the tests to just dispatch the sequence of actions. But we are using EffectsRunner copied from the previous version of ngrx/effects, so that might be helping us.

@TeoTN
Copy link
Contributor Author

TeoTN commented Apr 5, 2018

@alex-okrushko I am perfectly aware of the fact the store is synchronous.
However in my opinion, dispatching a sequence of actions (possibly wrapped in effects) only for testing something completely else, is simply writing code that serves no purpose. I think of it as just working around the fact that we have no possibility to simply pass an object with the state snapshot for tests.
It's probably a waste of time, programmers' keyboards and CPUs power ;)

I believe that unit tests should just verify very narrow scope of reality and we shouldn't really need to build boilerplate code to make them run, right?

@philmerrell
Copy link

@alex-okrushko We can't all be Rob Wormalds. 😉

Whether a mock store is necessary or unnecessary, I think at the very least, the documentation for unit testing around ngrx should define a much broader scope than is currently provided in the testing guide.

The attraction for a mock store for me is ease of use and simplicity. If there is a mock that can be provided in my spec classes that reduces the friction for setting up spies, establishing needed mock state, and testing with effects easier, count me in. Maybe I'm lazy, or I don't understand the inner workings of ngrx sufficiently, but I'd like to make the task of writing tests near brainless. Don't make me think too hard just to scaffold out tests.

Short of providing a mock, better documentation would be great around different scenarios. If that documentation exists... a more obvious link would be great.

@blackholegalaxy
Copy link

I'd appreciate a complete testing guide. Testing every part of the store (including router state) AND inclusion in components throught selectors. Testing documention is almost non existent.

@honsq90
Copy link

honsq90 commented Jun 6, 2018

It's not a full-fledged MockStore, but it's helped quite a bit when I was testing container components that were subscribed via different store selectors. We had issues where we had to initialise nested state in the TestBed, and we already had separate selector tests to cover the selector logic.

The MockStore implementation I wrote allows manual publishing of events per selector, which was useful in testing different permutations in the container component.

// in container component
ngOnInit() {
  this.list$ = this.store.select(getList);
  this.pending$ = this.store.select(getPending);
  this.error$ = this.store.select(getError);
}
// in beforeEach
const mockStore =  new MockStore({ getList, getPending, getError });
spyOn(mockStore, 'select').and.callThrough();
spyOn(mockStore, 'dispatch').and.callThrough();

TestBed.configureTestingModule({
  providers: [
     { provide: Store, useValue: mockStore }
  ]
}

// in test
it('should not show list when pending', () => {
  mockStore.selectStream['getList'].next([{ foo: 'bar' }]);
  mockStore.selectStream['getPending'].next(true);
  mockStore.selectStream['getError'].next(false);
  fixture.detectChanges();

  // do assertions
});

Here are the links to the implementation (my own sandbox repo outside of work).

Hope this helps while an official MockStore is being worked on!

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Jul 15, 2018

@alex-okrushko

Internally at Google I've been recommending to avoid mocking the Store as well - my suggestion was to use the real Store and if it's needed to be in certain state for the tests to just dispatch the sequence of actions

I am doing it the way you are describing by triggering actions... i don't feel this is the right way (it's eurkk ).

Example: (why i am saying this way is eurkk ^^!!)
If you have a common component you need to use in different pages/features. Which action you pick in the tests to set the right state in store (the one from pageA, pageB, featureC, ...)?
What happen if you pick pageA action and the product team decide to remove pageA later during the year...? (time lost at least)

I think from what i see in @TeoTN or @honsq90 propositions, there are benefits of mocking directly the store/selectors vs using actions:

  • take less time to reason about and create the test.
  • relying on a type returned by the selector is more effective than actions that probably change more during development.
    (check example on top)
  • more clear for a dev that works on the test after months => he don't need to visualize what actions are doing to the store in the tests

@TeoTN
i didn't check the PR and test it yet. You are proposing nextMock to update the whole store?
seems @honsq90 already implemented a more flexible mocking strategy at first view. Didn't try yet. What do you think about it?

@Juansasa
Copy link

@honsq90 Solution however will not work on the new pipeable select

@TeoTN
Copy link
Contributor Author

TeoTN commented Aug 14, 2018

@nasreddineskandrani
Well, yes, the idea is to use nextMock to update the whole state. Generally, as with all PRs there's an underlying problem I was trying to solve: in a bit more complex applications, building up a desired state for testing would require quite a lot of effort, e.g. dispatching multiple actions etc. Hence, my idea was to be able to simply bootstrap the whole state for testing. Such mocked state could be also extracted and stored in a separate file.

I rely on the fact that State is actually a Subject.

I want to be extra clear on that: it's a dead simple, hammer-like solution. But I'm open for extensions ;)

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Aug 16, 2018

so @TeoTN can we build a solution to mock selector directly. It's more powerfull, No?
I am ready to close this... we need a focused effort to decice and update the code to have it next release PLEASE ^^

@TeoTN
Copy link
Contributor Author

TeoTN commented Aug 16, 2018

@nasreddineskandrani that's an interesting idea but what if someone's not using the selector? I mean, it's a layer above the single source of truth and technically one may decide not to use it

@Juansasa
Copy link

@nasreddineskandrani I've trying to mock the selectors for a whole day without any success.
The new pipeable select is just pain in the ass to mock :/

@nasreddineskandrani
Copy link
Contributor

@TeoTN if he don't use selector from ngrx you mean => I guess he will need to find his own way of testing also :D
@Juansasa Sorry i am starting a new job. A lot of efforts... I ll try when i stabilize if no one finish it yet. In around 2 weeks maximum.

@TeoTN
Copy link
Contributor Author

TeoTN commented Aug 21, 2018

@nasreddineskandrani lol, that's so inclusive approach

@maxime1992
Copy link
Contributor

maxime1992 commented Aug 21, 2018

Joining the discussion here after I upgraded a huge project to use the pipeable operator.

I've made my own implementation of MockStore:

export class MockStore<T extends RootState = RootState> extends BehaviorSubject<T> {
  dispatch = jasmine.createSpy();
}

But realised that it is far from ideal when dealing with unit tests (whereas it's just fine for integration tests where you provide the initial state).

For unit tests, I want to be able to return directly a value for a given selector.

So with the help of @zakhenry I came up with the following:

export class MockStore<StateType extends RootState = RootState> extends BehaviorSubject<StateType> {
  private selectorsToValues: Map<(...args: any[]) => any, any> = new Map();

  public dispatch = jasmine.createSpy();

  constructor(initialState: StateType = null, private returnNullForUnhandledSelectors = true) {
    super(initialState);

    spyOnProperty(ngrx, 'select').and.callFake(_ => {
      return selector => {
        let obs$: Observable<any>;

        if (this.selectorsToValues.has(selector)) {
          const value = this.selectorsToValues.get(selector);

          obs$ = value instanceof Observable ? value : this.pipe(map(() => value));
        }

        obs$ = this.pipe(map(() => (this.returnNullForUnhandledSelectors ? null : selector(this.getValue()))));

        return () => obs$.pipe(distinctUntilChanged());
      };
    });
  }

  addSelectorStub<T>(cb: (...args: any[]) => T, mockedValue: T | Observable<T>): this {
    this.selectorsToValues.set(cb, mockedValue);
    return this;
  }

  setState(state: StateType): this {
    this.next(state);
    return this;
  }

  setReturnNullForUnandledSelectors(value: boolean): this {
    this.returnNullForUnhandledSelectors = value;
    return this;
  }
}

It's very fresh and might need some more work.

Also, notice the spyOnProperty(ngrx, 'select'), I've imported ngrx like that:

import * as ngrx from '@ngrx/store';

@Juansasa
Copy link

@maxime1992 Nice spyOnProperty is precisely what i missed, didnt know there are such method in jasmine 👍

@maxime1992
Copy link
Contributor

maxime1992 commented Aug 21, 2018

@Juansasa took me a while to figure that out too. Had no idea it existed.

Good thing is, I've learned quite a few things in the research process!

With spyOnProperty you can also spy/stub getters and setters ⚡

It seems that the default is setting the get so I just took it out but you can also do spyOnProperty(ngrx, 'select', 'get')

@blackholegalaxy
Copy link

Is there any equivalent on jest?

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Aug 22, 2018

@blackholegalaxy maybe phra/jest@1a3b82a

@maxime1992 can you please provide an example of your mockstore on stackblitz?

@marcelnem
Copy link

marcelnem commented Aug 28, 2018

@maxime1992 nice! Does it work universally with the traditional and also pipeable select?
Also I think the
obs$ = this.pipe(map(() => (this.returnNullForUnhandledSelectors ? null : selector(this.getValue()))));should be wrapped in the else clause.

@marcelnem
Copy link

marcelnem commented Aug 29, 2018

@maxime1992 I really like your implementation, I have added the store() function to your MockStore code, so is should work with the classic as well as pipeable select operator.

and I have wrapped this piece of code in the else clause:
obs$ = this.pipe(map(() => (this.returnNullForUnhandledSelectors ? null : selector(this.getValue()))));

Now it looks like (code based on code of @maxime1992 from few posts earlier):

import { Injectable } from '@angular/core';
import { BehaviorSubject, Observable } from 'rxjs';
import { AppState } from '@app/reducers.index';
import { map, distinctUntilChanged } from 'rxjs/operators';
import * as ngrx from '@ngrx/store';

@Injectable()
export class MockStore<StateType extends AppState = AppState> extends BehaviorSubject<StateType> {
  private selectorsToValues: Map<(...args: any[]) => any, any> = new Map();
  public dispatch = jasmine.createSpy();

  public select = jasmine.createSpy().and.callFake(
    (selector: any): Observable<any> => {
      return this.getObservableWithMockResult(selector).pipe(distinctUntilChanged());
    }
  );

  constructor(initialState: StateType = null, private returnNullForUnhandledSelectors = true) {
    super(null);
    spyOnProperty(ngrx, 'select').and.callFake(_ => {
      return selector => { 
        return () => this.getObservableWithMockResult(selector).pipe(distinctUntilChanged());
      };
    });
  }

  private getObservableWithMockResult(selector:any):Observable<any>{
    let obs$: Observable<any>;

    if (this.selectorsToValues.has(selector)) {
      const value = this.selectorsToValues.get(selector);

      obs$ = value instanceof Observable ? value : this.pipe(map(() => value));
    } else {
      obs$ = this.pipe(map(() => (this.returnNullForUnhandledSelectors ? null : selector(this.getValue()))));
    }
    return obs$;
  }

  addSelectorStub<T>(cb: (...args: any[]) => T, mockedValue: T | Observable<T>): this {
    this.selectorsToValues.set(cb, mockedValue);
    return this;
  }

  setState(state: StateType): this {
    this.next(state);
    return this;
  }

  setReturnNullForUnandledSelectors(value: boolean): this {
    this.returnNullForUnhandledSelectors = value;
    return this;
  }
}

I am using it in tests like this at the moment:

import { async, ComponentFixture, TestBed, inject } from '@angular/core/testing';
import { StoreModule, Store } from '@ngrx/store';
import { MockStore } from '@app/test-utils/mockup-store';


describe('SomeComponent', () => {
  let component: SomeComponent;
  let fixture: ComponentFixture<SomeComponent>;
  let mockStore: MockStore<ExtendedStateWithLazyLoadedFeatures>;

  beforeEach(async(() => {
    TestBed.configureTestingModule({
      declarations: [SomeComponent],
      imports: [StoreModule.forRoot({})],
      providers: [{ provide: Store, useValue: new MockStore<ExtendedStateWithLazyLoadedFeatures>(null, true) }]
    }).compileComponents();
  }));

  beforeEach(inject([Store], (testStore: MockStore<ExtendedStateWithLazyLoadedFeatures>) => {
    // save the automatically injected value so we can reference it in later tests
    mockStore = testStore;
  }));

  describe('with values for selectors defined', () => {
    beforeEach(() => {
      mockStore.addSelectorStub(selector1, {});
      mockStore.addSelectorStub(selector2, ["mockValue"]);
    });

    beforeEach(() => {
      fixture = TestBed.createComponent(SomeComponent);
      component = fixture.componentInstance;
      fixture.detectChanges();
    });

    it('should create', () => {
      expect(component).toBeTruthy();
    });

    // this is checking if the non-pipeable variant of select was called
    it('should call selector selector1', () => {
      expect(mockStore.select).toHaveBeenCalledWith(selector1);
    });

    // this is checking if the non-pipeable variant of select was called
    it('should call selector selector2', () => {
      expect(mockStore.select).toHaveBeenCalledWith(selector2);
    });

  });
});

I am a beginner with Angular tests. Feedback is appreciated.

@TeoTN TeoTN closed this as completed Aug 29, 2018
@TeoTN TeoTN reopened this Aug 29, 2018
@nasreddineskandrani
Copy link
Contributor

@SerkanSipahi you need to build the selector with pipe(select... and add of( ) in the returnValue. Please check the online example i posted.

@SerkanSipahi
Copy link
Contributor

I checked the online example. I took an example which is not built with pipe (it works in online example):

luffy.selector.ts

export const selectPowerA = createSelector(selectLuffy, (luffyState) => {
  return luffyState.powerA;
});

luffy.component.spec.js

spyOn(luffySelectors, 'selectPowerA').and.returnValue(of(true));

Error:

not error on your example

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 8, 2018

@SerkanSipahi GG! i assumed it's not working without pipe but it's even more simple 💃 so why are we trying to mock select :D.

With my friends, we didn't try this at first place following the doc with action triggers. But it was none productive that's why i was hunting a better solution.

So i guess an update is needed with this way of mocking selectors into the ngrx testing documentation. For the doc update:

  • components and effects to consider
  • A doc separation between unit testing and integration testing is to consider and will be helpfull knowing that this is for "Unit Testing" and @TeoTN proposal for integration testing.

Question: this kill the need of mocking select in mockStore or someone see a case?

@SerkanSipahi
Copy link
Contributor

SerkanSipahi commented Sep 8, 2018

I was really exited about that we getting mock store, selector, etc but i think its not ready for usage its makes more complicated. I dont know.

Well, mocking selector and store are not worked well for me or just lets say: really hard to do that simple (maybe im stupid) or maybe it is not fully developed yet, i dont know! For me a test setup should be very easy without or with less think about it.

For me it workes very well and easy to test my effect with the real store as @alex-okrushko suggested!

effect:

  @Effect({ dispatch: false }) toggleAppElement$ = this.actions$.pipe(
    ofType<Fullscreen>(FullscreenActionTypes.Event),
    switchMap(_ => combineLatest(
      this.store.pipe(select(fromRoot.getFullscreen)),
      this.store.pipe(select(fromRoot.getAppElement), skip(1)),
      this.video.getElement$<HTMLVideoElement>('video')),
    ),
    filter(([, appEl]) => appEl instanceof Element),
    tap(([fullscreen, appEl, videoEl]) =>
      this.toggleAppElement(fullscreen, appEl, videoEl)
    ),
  );

effect.spec:

describe('FullscreenEffects', () => {

  let store$: Store<fromRoot.State>;
  let fullscreenEffects: FullscreenEffects;
  let videoService: VideoService;
  let fakeEvent = (): Event => new Event('fakeName');
  let fakeElement = (): Element => document.createElement('div');

  beforeEach(() => {
    TestBed.configureTestingModule({
      imports: [
        StoreModule.forRoot(reducers, { metaReducers }),
      ],
      providers: [
        Store,
        FullscreenEffects,
        Actions,
        VideoService,
      ],
    });

    fullscreenEffects = TestBed.get(FullscreenEffects);
    videoService = TestBed.get(VideoService);
    store$ = TestBed.get(Store);
  });

  it('should call fullscreenEffects.toggleAppElement', done => {
    spyOn(videoService, 'getElement$').and.returnValue(of(fakeElement()));
    spyOn(fullscreenEffects, 'toggleAppElement');

    fullscreenEffects.toggleAppElement$.subscribe(_ => {
      expect(fullscreenEffects.toggleAppElement).toHaveBeenCalledTimes(1);
      done();
    });

    store$.dispatch(new AppElement(fakeElement()));
    store$.dispatch(new Fullscreen(fakeEvent()));
  });

});

Now im not agree with @TeoTN of: It's probably a waste of time, programmers' keyboards and CPUs power!

what a waste of time? it was really easy! cpu? i have not 1000 effects thats run in tests!

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 8, 2018

with @TeoTN proposal you can set a state in the store without triggering actions. In your case, you are not triggering actions to set a state. But if you had to trigger actions @TeoTN proposal is better.
And then his comment would have applied t's probably a waste of time, programmers' keyboards and CPUs power about triggering actions.

@SerkanSipahi
Copy link
Contributor

SerkanSipahi commented Sep 8, 2018

with @TeoTN proposal you can set a state in the store without triggering actions

But i have to triggering a action. How should i catch ofType<Fullscreen>(FullscreenActionTypes.Event)

In your case, you are not triggering actions to set a state

See im triggering a action(s) and setting a new state:

store$.dispatch(new Fullscreen(fakeEvent()));
store$.dispatch(new AppElement(fakeElement()));

But if you had to trigger actions @TeoTN proposal is better

It did not worked for me. I can just set a state with nextMock and thats it. How to go through ofType<Fullscreen>(FullscreenActionTypes.Event)

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 8, 2018

Sorry i didn't see it :)

you need to trigger the effect. This is the action you can avoid store$.dispatch(new AppElement(fakeElement())); not store$.dispatch(new Fullscreen(fakeEvent())); which fire the stream.
You can avoid to trigger the actions you need to make select operator grab the data you want in the combineLatest by mocking selectors.
--> You can check marble testing for effects also, it helps.

I already explained one case with common component where triggering actions isn't welcome
here -> #915 (comment)

i ll give you another case related to your example,
lets say select(fromRoot.getAppElement) rely on a state that need 1 other action ALHPA to happen before to select proprely what you need. This action ALHPA trigger 4 backend calls. You need to go mock all these 4 api calls to get the store in the right state before triggering the effect.
Mocking the selector allow you to avoid the mock of the 4 apis calls.... by copy/pasting the content of your selector grabbed at runtime (debug mode). And dont worry about updating the mocking of api calls in this test if they change overtime when the end data type in the selector remain the same.
=> You test only the current unit.

*this is not common for me to see 'dom' stuff in effect ( ), you can poke me in gitter in private if you wanna talk about this or ask in gitter/ngrx. I built multi app with redux react/angular never had to do that.

@marcelnem
Copy link

marcelnem commented Sep 9, 2018

@SerkanSipahi can you post the full example how you made it work with a selector which is not wrapped? I tried it in the example and I get an error. https://stackblitz.com/edit/ngrx-component-testing-simplified-7fd5nk?file=src/app/luffy/luffy.component.spec.ts

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 10, 2018

@marcelnem i tried it it's not working for me without pipe i can't spend more time on this. Looking forward to see what ngrx team decide after all these interactions.

@SerkanSipahi
Copy link
Contributor

SerkanSipahi commented Sep 10, 2018

@marcelnem @nasreddineskandrani I think here is a misunderstanding! What i meant was, its not possible to compile that thing when its not wrapped by pipe!

I add spyOn(luffySelectors, 'selectPowerB').and.returnValue(of(true)); into your beforeEach example inside of component fast testing with selectors mocking and i get no compile error like on my env: Error: <spyOn> : getFullscreen is not declared writable or has no setter

When i do that with my selectors to spyOn a selector which is not wrapped by pipe i get an error.

Which version of ngrx you are using?

@SerkanSipahi
Copy link
Contributor

SerkanSipahi commented Sep 10, 2018

1.) ... you need to trigger the effect. This is the action you can avoid store$.dispatch(new AppElement(fakeElement())); not store$.dispatch(new Fullscreen(fakeEvent())); which fire the stream....

2.) ... i ll give you another case related to your example,
lets say select(fromRoot.getAppElement) rely on a state that need 1 other ac

@nasreddineskandrani thank you for your good explanation! Now i got it. It took a while until I understood it (it was to much: mock.selector, mock.next, marble tests, etc. etc)

For just clarification(to improve my understanding): in my case it make no sense to mock the store (nextMock), right? Or is there something what i not see? maybe, imagine select(fromRoot.getAppElement) use internally other selectors to bulid the state for fromRoot.getAppElement. As far as I understand it, i have to mock then each selector, one by one but with store.nextMock i could just set the state easily.

@brandonroberts
Copy link
Member

Update: I left some feedback on the PR for the proposed testing API for Store here #1027 (comment)

Leave feedback if you have any here and not there.

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 14, 2018

@brandonroberts
I totally agree with you testing component this way will work and it's better than trigger actions 👍
so setState solve the original main issue

some argumentations on why, in my opinion, mocking selectors could be good also!

We shouldn't focus on mocking selectors. If you want to test selectors, provide a state that the selectors can use or use StoreModule.forRoot() with reducers for integration.

From your statement, i wanted to say that in my case i don't want to test selectors when i want to unit test components. why? Because integration tests are slower and should be fewer than unit tests to fail faster.

Also there is a way the projector in selector built-in in ngrx to unit tests selectors
->to be consitent i saw a unit test ability in the lib for components as a nice to have.
https://blog.angularindepth.com/how-i-test-my-ngrx-selectors-c50b1dc556bc

conclusion section of article:

A selector can be unit tested, you don’t need a (mocked) store in order to test your selectors.

By your statement you don't provide a way to unit test components (doc update or code update) and encourage integration tests with real selectors to test components.
I am insisting again about the fact it's slower and it may be a problem for some.

i know :) i am taking it far but another little point, when it fails you know it's in the component not the selector => More isolated.

thank you for the answer and i am happy setState is going in to stop the need of triggering actions to be able to test a smart component

@brandonroberts
Copy link
Member

@nasreddineskandrani I see what you are saying. If you want to unit test a component that has selectors, override the class variables with test observables. Here is a simple example of overriding the selector itself, versus setting the state and using the selector as-is.

https://stackblitz.com/edit/ngrx-test-template?file=app%2Fcounter%2Fcounter.component.spec.ts

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 15, 2018

thank you very much so simple 👍
now that you mention it here devs will see it
maybe a little section in the doc about what you just posted.
You know more than me :) if it deserve a place in the doc. I am not that bad in habit but honestly i didn't see this solution. thanks again

@timdeschryver
Copy link
Member

@maxime1992 responding to your comment in the PR - #1027 (comment).

While your solution works, it's bound to jasmine, which is a downside in my opinion.
If you want to mock the selectors, see @brandonroberts 's answer #915 (comment).

@maxime1992
Copy link
Contributor

@timdeschryver that's a fair point.

That said, I don't really like the idea of replacing directly the component's variable:

  • Using of is probably inappropriate because it'll close the stream after the first emit. So if you are triggering side effects when the stream closes, it might be way too early. Of course, we could create a new Observable, but it's a bit less straightforward.
  • What if the variable is a private one?

@timdeschryver
Copy link
Member

Those are valid points :)

@brandonroberts
Copy link
Member

@maxime1992 I don't disagree with those points, but I don't think we should use a Jasmine/Jest specific way to mock selectors. Using of is not the only way of doing it of course if you need to push multiple values.

I haven't tested this, but selectors could include an API you can access on the selector itself similar to release but it would short circuit the return value.

@brandonroberts
Copy link
Member

brandonroberts commented Sep 16, 2018

Here is a newer version that lets you override selectors in addition to state. It adds a method to the returned selector to let you set the result value that will be returned.

import { Injectable, Inject, OnDestroy } from '@angular/core';
import {
  Store,
  ReducerManager,
  ActionsSubject,
  ActionReducer,
  Action,
  ScannedActionsSubject,
  INITIAL_STATE,
  MemoizedSelector,
  MemoizedSelectorWithProps,
} from '@ngrx/store';
import { BehaviorSubject } from 'rxjs';

@Injectable()
export class MockState<T> extends BehaviorSubject<T> {
  constructor() {
    super({} as T);
  }
}

@Injectable()
export class MockReducerManager extends BehaviorSubject<
  ActionReducer<any, any>
> {
  constructor() {
    super(() => undefined);
  }
}

@Injectable()
export class MockStore<T> extends Store<T> {
  static selectors = new Set<
    MemoizedSelector<any, any> | MemoizedSelectorWithProps<any, any, any>
  >();

  constructor(
    private state$: MockState<T>,
    actionsObserver: ActionsSubject,
    reducerManager: ReducerManager,
    public scannedActions$: ScannedActionsSubject,
    @Inject(INITIAL_STATE) private initialState: T
  ) {
    super(state$, actionsObserver, reducerManager);
    this.resetSelectors();

    this.state$.next(this.initialState);
  }

  setState(state: T): void {
    this.state$.next(state);
  }

  overrideSelector<T, Result>(
    selector: MemoizedSelector<T, Result>,
    value: Result
  ): MemoizedSelector<T, Result>;
  overrideSelector<T, Result>(
    selector: MemoizedSelectorWithProps<T, any, Result>,
    value: Result
  ): MemoizedSelectorWithProps<T, any, Result>;
  overrideSelector<T, Result>(
    selector:
      | MemoizedSelector<any, any>
      | MemoizedSelectorWithProps<any, any, any>,
    value: any
  ) {
    MockStore.selectors.add(selector);
    selector.setResult(value);
    return selector;
  }

  resetSelectors() {
    MockStore.selectors.forEach(selector => selector.setResult());
    MockStore.selectors.clear();
  }

  dispatch(action: Action) {
    super.dispatch(action);
    this.scannedActions$.next(action);
  }

  addReducer() {
    // noop
  }

  removeReducer() {
    // noop
  }
}

export function provideMockStore<T>(config: { initialState?: T } = {}) {
  return [
    { provide: INITIAL_STATE, useValue: config.initialState },
    ActionsSubject,
    ScannedActionsSubject,
    MockState,
    { provide: ReducerManager, useClass: MockReducerManager },
    {
      provide: Store,
      useClass: MockStore,
    },
  ];
}

If we take the auth-guard.service.spec.ts from the example app, the test now looks like this.

import { TestBed, inject } from '@angular/core/testing';
import { Store, MemoizedSelector } from '@ngrx/store';
import { cold } from 'jasmine-marbles';
import { AuthGuard } from '@example-app/auth/services/auth-guard.service';
import { AuthApiActions } from '@example-app/auth/actions';
import * as fromRoot from '@example-app/reducers';
import * as fromAuth from '@example-app/auth/reducers';
import { provideMockStore, MockStore } from '@ngrx/store/testing';

describe('Auth Guard', () => {
  let guard: AuthGuard;
  let store: MockStore<any>;
  let loggedIn: MemoizedSelector<fromAuth.State, boolean>;

  beforeEach(() => {
    TestBed.configureTestingModule({
      providers: [provideMockStore()],
    });

    store = TestBed.get(Store);
    guard = TestBed.get(AuthGuard);

    loggedIn = store.overrideSelector(fromAuth.getLoggedIn, false);
  });

  it('should return false if the user state is not logged in', () => {
    const expected = cold('(a|)', { a: false });

    expect(guard.canActivate()).toBeObservable(expected);
  });

  it('should return true if the user state is logged in', () => {
    loggedIn.setResult(true);

    const expected = cold('(a|)', { a: true });

    expect(guard.canActivate()).toBeObservable(expected);
  });
});

@Bielik20
Copy link

@brandonroberts getting: selector.setResult is not a function. I am using Jest instead of Jasmine but I don't think that matters in here.

@brandonroberts
Copy link
Member

@Bielik20 this code has not been merged into master yet. @TeoTN how do you want to proceed with this PR?

@TeoTN
Copy link
Contributor Author

TeoTN commented Sep 28, 2018

@brandonroberts I like some of the ideas from here and the PR. I was busy recently and didn't have time to incorporate them in the code, but I'll definitely take the time to improve that. Stay tuned ;)

@nasreddineskandrani
Copy link
Contributor

nasreddineskandrani commented Sep 29, 2018

@TeoTN
I can pr in your fork if you are short in time and you need help (especially for the tests/doc update).
I wrote you in your personal gitter, let me know over there.

@tomson7777
Copy link

@brandonroberts
I checked the 7.0.0-beta.1 version, and i do not see Store.overrideSelector method which you presented few comments above.
I see only such class:
export declare class MockStore<T> extends Store<T> { private state$; private initialState; scannedActions$: Observable<Action>; constructor(state$: MockState<T>, actionsObserver: ActionsSubject, reducerManager: ReducerManager, initialState: T); setState(nextState: T): void; addReducer(): void; removeReducer(): void; }

Shouldn't be overrideSelector method merged?

@nasreddineskandrani
Copy link
Contributor

@tomasznastaly
it's not in. They need time
please read here brandon comment: #1027 (comment)

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

No branches or pull requests