Closed rainerhahnekamp closed 2 months ago
(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 mockSignalStore
I could just provide the mock instance to ng-mocks directly and check spys on it. Neat!
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 MockInstance
). So is it worth also having some form of buildRxMethodSpy(…)
utility function provided by @ngrx/signals
? This would be a lower-level utility that folks could use with whatever testing framework / helpers they want.
I see. Well, in that case, we might want to discuss a set of testing helpers.
@rainerhahnekamp — I do think having a mockSignalStore
high level helper is a very good first step (and probably more impactful to the community at large). My particular need could come out of that work, or later.
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: https://github.com/ngrx/platform/pull/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:
undefined
. It is the up to the user to use their tools to mock/spy in whatever way they want.provideSignalStore
function would then allow to define the behavior directly (very similar to ng-mocks): https://ng-mocks.sudo.eu/api/MockProvider/#useclassAside: @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 provideBasicMockSignalStore()
prototype (automatic mocking).
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 provideBasicMockSignalStore()
would return a mocked version of the original store, its properties would be replaced with these symbols: MOCK_COMPUTED
, MOCK_METHOD
and MOCK_RXMETHOD
s. And it's up to the user to replace these property values with spies.
(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:
MOCK_COMPUTED
s by WritableSignals
MOCK_METHOD
s by Sinon fakes
MOCK_RXMETHOD
s by FakeSinonRxMethods
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 provideMockStore
can we have something similar for mocking signal stores. MockStore enables us to overrideSelector
in our tests. Can we have a similar approach here for computed, method and rxmethod ?
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 rxMethod
s because of their particular TypeScript signature. (Hence the ugly utility method I've had to use). I think if something could be done about this then using tools like ng-mocks
as an extra layer becomes quite powerful. Though note that I haven't gone too deep into testing SignalStores so there may be other issues / nuances that need considering. And I'm yet to go through @gergelyszerovay's impressive-looking article.
As you say, we may not need a full mocking capability (e.g. mockSignalStore
), especially as libraries like ng-mocks
already provides the ability to "auto mock" things. I think what's key is the public interface of a store — in a test you probably only need to mock a few things in the whole store. ng-mocks
essentially allows me to mock out the whole object / class instance (it just nullifies stuff) and plug in in spys where I need, whilst maintaining the whole shape of the thing being mocked out. I need to look into how ng-mocks
works under the hood — it's quite possible that it too resorts to manual typecasting to achieve it's mocking capabilities.
(Apologies for focusing so much on ng-mocks
— that just happens to be the thing I'm focused on using at the moment, but hopefully the principles are useful generally).
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 rxMethod
: To the component/consumer it is a simple method. Maybe ng-mocks make it more complicated than it actually is? I hope the examples will bring some answers.
@rainerhahnekamp
We want to have as few libraries as possible ... As @jits pointed out, provideMockStore should integrate into the existing testing tools that developers use already.
To remove Jest/Jasminse/Sinon dependencies we should consider the following API:
export const spies = {
'function': () => sinon.fake(), // for mocking function
asyncFunction: () => sinon.fake.yieldsAsync(), // for mocking async functions
}
provideMockSignalStore(PersonStore, {
spies,
// ... other config options
})
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 spies
configurations.
The type of provideMockSignalStore(PersonStore)
and PersonStore
would be the same, but in the documentation we can also provide sample implementations for helper functions such as asSinonSpy or asMockSignalStore() .
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 spies
config, and the asSinonSpy
, asMockSignalStore()
and asMockRxMethod()
helper functions, that can typecast the store and its properties to their mocked version.
Maybe we don’t need a mocking feature at all.
I think it depends on the application. I've used ComonentStore
s extensively in the past, and in an large enterprise application we can easily have dozens of stores with 20-30+ properties (selectors, updaters, effects) per store.
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.
@jits
in a test you probably only need to mock a few things in the whole store.
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 MockProvider
can't properly mock ComponentStore
's updater
s and effect
s, and the new RxMethod
s. These three methods work similarly, all the three return a function, that can be called:
To mock these, I've created a fakeRxMethod function, we can also use this inside a MockProvider
.
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 createMock
from the testing library, but your version is much better suited for the signal store's needs, like keeping computeds and the initial state patch.
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.
Hey 👋
I'm going to convert this issue into a discussion for now. Feel free to create a standalone library/prototype with @ngrx/signals
testing utilities and we will take a look.
To consider introducing a testing plugin as part of the core package, it has to be independent of testing frameworks.
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