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
@ngrx/signals: mockSignalStore
feature
#4256
Comments
(Bear with me as I unravel some thoughts) I'm currently trying to standardise on using ng-mocks. With that I can currently do the following: // In a spec file
// Note: `MockXX` are special ng-mocks helpers.
beforeEach(() => MockBuilder(MyComponent, null).mock(MyStore));
// `buildRxMethodSpy` is my ugly helper method to generate a Jasmine spy with the correct TypeScript signature
// (see: https://github.com/ngrx/platform/issues/4206#issuecomment-1932448978 if you want to get very annoyed).
it("should call the store method on creation", () => {
const methodSpy = MockInstance(
MyStore,
"myRxMethod",
buildRxMethodSpy("myRxMethod")
);
const fixture = MockRender(MyComponent);
const component = fixture.point.componentInstance;
expect(component).toBeTruthy();
expect(methodSpy).toHaveBeenCalled();
}); With However, I quite like to let ng-mocks take care of mocking things (in this case, the Store), to keep things consistent and predictable, and then provide spy functions (using |
I see. Well, in that case, we might want to discuss a set of testing helpers. |
@rainerhahnekamp — I do think having a |
Yeah, then it's up to the NgRx guys to decide if that is something we should have or not. |
Another suggestion on this topic: #4252 |
Oh, that's a coincidence. That's huge. Congrats @gergelyszerovay. We could take this issue as the "issue" and #4252 as the official PR. What I'd like to discuss is the dependency to sinon.js. I think we have three options here:
|
Aside: @rainerhahnekamp — you may have intended to use a different link for your point (2) above? |
I published an article that compares manual Signal Store mocking and my I chose to use Sinon in my prototype because I believe that many teams are currently using Jasmine and are considering moving to Jest in the long run. For these teams, using test framework-agonistic spies may be the best solution. @rainerhahnekamp We should also consider implementing both options (1) and (3): (1). The basic version of (3). Additionally, we might provide test/mock framework specific tools: these would 'auto-spy' the basic mock store, for example the Sinion version would it replace: The drawback of this approach is that supporting more test/mock frameworks would mean more dependencies. |
Great article, @gergelyszerovay! Please give me some days to collect and structure all my thoughts on your implementation. |
@rainerhahnekamp any thoughts so far ? Looks like the PR is closed and there will be another direction. Looking at the API of the existing |
I see two non-technical reasons that speak against Sinon.
That being said, at the moment I’d like to propose another possibility: The signalStore itself requires an absolute minimum of code to set it up. Maybe we don’t need a mocking feature at all. Let me give you an example: const UserStore = signalStore(
withState({ id: 1, firstname: 'Rudolf', lastname: 'Weber' }),
withMethods((store) => {
return {
async load() {
// ...
},
rename() {
// ...
},
};
}),
withComputed((state) => {
return {
prettyName: computed(() => `${state.firstname()} ${state.lastname()}`),
};
})
); Version without mocking function const MockedStore = signalStore(
withState({ id: 2, firstname: 'Sandra', lastname: 'Smith' }),
withMethods(() => ({ load: jest.fn() })),
withComputed(() => ({ prettyName: signal('Sandra Smith') }))
);
TestBed.configureTestingModule({
providers: [{ provide: UserStore, useClass: MockedStore }],
});
const store = TestBed.inject(UserStore) as unknown as InstanceType<
typeof MockedStore
>;
store.load.mockImplementation(() => {
// mocking behavior
}); What do you say? |
Hi @rainerhahnekamp, I think your proposal works very well for simple cases (which may be the majority?). I guess it has the same implications as managing any manual mocks — e.g. keeping it in sync with the implementation (especially from a TypeScript typing perspective). I'm not a fan of the manual typecasting as we're then breaking out of TypeScript's protections. Still, this goes a very long way in testing and is a good first port of call. A sticking point I (personally) have right now is mocking As you say, we may not need a full mocking capability (e.g. (Apologies for focusing so much on |
I wouldn't have too high expectations for the internal implementation of the mocking libraries. They are very good in terms of typing, but internally, they are a collection of functions created with their specific mocks. We usually use these libraries because writing manual mocks generally requires more code than automatic ones. What if we take 3 or 4 examples and compare them? If the manual turns out to be exactly the same amount of code, I suggest that the documentatios shows how to mock the signalStore properly with the common mocking libraries. About mocking |
To remove Jest/Jasminse/Sinon dependencies we should consider the following API:
So users have to specify they preferred mock functions, similarly to ng-mock's custom auto spies works. In the documentation, we can provide sample Jest/Jasminse/Sinon The type of Alternatively, we might provide a configuration file generator, users can specify one of the following: Jest/Jasminse/Sinon, and the generator will generate a signal-store-test-config.ts with the spy framework specific
I think it depends on the application. I've used I expect a higher property number per store if I use Signal Store, because with the Custom Store Features, it's really easy to create Signal Stores with dozens of signals/computeds/methods. For example if I use the withDataService Custom Store Feature, it adds 6 new properties to the store. For example, when we create a store for a component, that can modify a Book object (it can read and write a Book), the store has at least 13 properties. If we add a few selectors an methods, for example for supporting tags for the books, we can easily have 20-30 properties. So this is the store: export const BookStore = signalStore(
withState({ book: Book }),
withDataService({
actionName: 'loadBook',
// ...
}),
withDataService({
actionName: 'saveBook',
// ...
}),
}); The minimal, auto-mocked version of the store: provideMockSignalStore(ArticleListSignalStoreWithFeature, {
spies,
mockComputedSignals: false,
} If we need a pre-loaded book in the mocked store: provideMockSignalStore(ArticleListSignalStoreWithFeature, {
spies,
mockComputedSignals: false,
initialStatePatch: {
book: {
title: "Algorithms + Data Structures = Programs",
author: "Wirth, Niklaus"
},
loadBookRequestState: HttpRequestState.FETCHED
}
} The manually mocked version of the previous store (with a pre-loaded book): const MockedStore = signalStore(
withState({
book: {
title: "Algorithms + Data Structures = Programs",
author: "Wirth, Niklaus"
},
loadBookRequestState: HttpRequestState.FETCHED,
saveBookRequestState: HttpRequestState.INITIAL,
// if we want to mock the computeds
isLoadBookRequestStateInitial: false,
isLoadBookRequestStateFetching: false,
isLoadBookRequestStateFetched: true,
getLoadBookRequestStateError: undefined,
isSaveBookRequestStateInitial: true,
isSaveBookRequestStateFetching: false,
isSaveBookRequestStateFetched: false,
getSaveBookRequestStateError: undefined,
}),
withMethods(() => ({
loadBook: fakeRxMethod(),
saveBook: fakeRxMethod(),
})),
withComputed((state) => ({
// if we want to keep the original logic of computeds:
isLoadBookRequestStateInitial: computed(() => state.loadBookRequestState() === HttpRequestStates.INITIAL),
isLoadBookRequestStateFetching: computed(() => state.loadBookRequestState() === HttpRequestStates.FETCHING),
isLoadBookRequestStateFetched: computed(() => state.loadBookRequestState() === HttpRequestStates.FETCHED),
getLoadBookRequestStateError: computed(() => state.loadBookRequestState() === HttpRequestStates.FETCHED),
isSaveBookRequestStateInitial: computed(() => state.saveBookRequestState() === HttpRequestStates.INITIAL),
isSaveBookRequestStateFetching: computed(() => state.saveBookRequestState() === HttpRequestStates.FETCHING),
isSaveBookRequestStateFetched: computed(() => state.saveBookRequestState() === HttpRequestStates.FETCHED),
getSaveBookRequestStateError: computed(() => state.saveBookRequestState() === HttpRequestStates.FETCHED),
}))
); So it's possible to manually mock stores like this, but I think it's a time-consuming task. Automatic mocking can also reduce the required amount of coding, when we add new features to the store later.
This also depends on the use case. I like Storybook, and prefer to make Stories:
So If I want to mock a Signal Store of a smart or dumb component for use in Storybook, I need to provide meaningful values for all the state signals and computeds, otherwise the component will not render correctly in Storybook. I like and use ng-mocks, but I found that
To mock these, I've created a fakeRxMethod function, we can also use this inside a |
I'm sorry for not getting back to you sooner. For some reason, I've missed your answer. I was also thinking about providing something like We can provide the spies property for different testing frameworks without requiring new dependencies. The testing library, for example provides jest-specific functions but doesn't list jest as a dependency: https://github.com/testing-library/angular-testing-library/blob/main/projects/testing-library/package.json. @markostanimirovic: The question is how should we proceed? I think we kind of agree that we want to have that mocking feature. |
Which @ngrx/* package(s) are relevant/related to the feature request?
signals
Information
It would be great, if we can provide a similar
provideMockStore
feature whenever we want to mock the SignalStore. It would also containjest.fn
orjasmine.createSpy
for the methods and computeds:And then inside a test
Describe any alternatives/workarounds you're currently using
At the moment, I'd have to do that manually.
I would be willing to submit a PR to fix this issue
The text was updated successfully, but these errors were encountered: