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 selectors in MockStoreConfig #1827

Closed
jtcrowson opened this issue May 6, 2019 · 7 comments · Fixed by #1836
Closed

RFC: mock selectors in MockStoreConfig #1827

jtcrowson opened this issue May 6, 2019 · 7 comments · Fixed by #1836
Assignees
Labels
Accepting PRs community watch Someone from the community is working this issue/PR enhancement Project: Store

Comments

@jtcrowson
Copy link
Contributor

jtcrowson commented May 6, 2019

I'd like to open a pull request to add a mock selectors option to the MockStoreConfig. This will enable developers to mock selectors up front in the testing module configuration, similar to how you can provide an initial state.

This enables developers who do not need to update the state or selector to omit creating a MockStore instance.

Current example

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

  beforeEach(() => {
    TestBed.configureTestingModule({
      providers: [
        AuthGuard,
        provideMockStore({
          initialState: ...
          // able to provide initial state state without local instance
        }),
      ],
    });
    store = TestBed.get(Store);
    guard = TestBed.get(AuthGuard);

    store.overrideSelector(fromAuth.getLoggedIn, false);
    // need local instance to override selector
  });

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

Future example

describe('Auth Guard', () => {
  let guard: AuthGuard;
  // let store: MockStore<fromAuth.State>;
  // don't need local instance

  beforeEach(() => {
    TestBed.configureTestingModule({
      providers: [
        AuthGuard,
        provideMockStore({
          initialState: ...,
          // able to provide initial state state without local instance
          selectors: [
            { selector: fromAuth.getLoggedIn, value: false }
          ]
          // able to provide selectors without local instance
        }),
      ],
    });
    guard = TestBed.get(AuthGuard);
  });

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

Proposed solution

@Injectable()
export class MockStore<T> extends Store<T> {
  // ...
  constructor(
    private state$: MockState<T>,
    // ...
    @Inject(INITIAL_STATE) private initialState: T,
    @Inject(INITIAL_SELECTOR_VALUES) initialSelectorValues?: MockSelector[]
  ) {
    super(state$, actionsObserver, reducerManager);
    // ...
    if (initialSelectorValues) {
      initialSelectorValues.forEach(mockSelector =>
        this.overrideSelector(mockSelector.selector, mockSelector.value)
      );
    }
  }

Since it would add an extra optional configuration property, this would not introduce a breaking change.

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

@brandonroberts
Copy link
Member

Good idea, I'm assigning this issue to you for tracking.

@jtcrowson
Copy link
Contributor Author

Sounds good, I'll pick it up.

@timdeschryver timdeschryver added Accepting PRs community watch Someone from the community is working this issue/PR labels May 7, 2019
@jtcrowson
Copy link
Contributor Author

@brandonroberts do you know why the example-app test files are being compiled and tested with jest using a typescript version < 3.4? I'm trying out using a typescript 3.4+ feature for enforcing the mock value type matches the selector type.

@brandonroberts
Copy link
Member

brandonroberts commented May 7, 2019

Not sure. Do you have TypeScript globally installed?

From running npm list typescript and npm list @microsoft/api-extractor, it looks like @angular/[email protected] is pulling in @microsoft/[email protected] that has a dependency on TS ~3.1. What we normally do is use the resolutions object in the package.json to force version resolutions. You can try that for TypeScript to resolve to 3.4+.

@jtcrowson
Copy link
Contributor Author

@brandonroberts I decided against type checking the selector return type against the mock value type due to the added complexity and lack of clarity in expected type it provided the developer. If you're curious, I can explain more.

@mjh5153
Copy link

mjh5153 commented Oct 14, 2019

Is there a working example of this? The ngrx overrideSelector stackblitz isn't working for multiple overrides in a single component. Any help is great thanks.

@jtcrowson
Copy link
Contributor Author

Yes, checkout the example app included in this repository. The unit tests mostly use this setup.
🔗:
Example-App
File using selectors + MockStoreConfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepting PRs community watch Someone from the community is working this issue/PR enhancement Project: Store
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants