Can't spy SignalStore feature method called inside another feature method #4627
Replies: 12 comments
-
this case is very interesting one, cause the final store looks like a simple class instance with methods: But hmm the methods do not come from the prototype, seems like they are attached to the class instance object that is already created. |
Beta Was this translation helpful? Give feedback.
-
The same problem can be reproduced without SignalStore: it('fails', () => {
function foo() {}
function bar() {
foo();
}
const myStore = { foo, bar };
const spy = jest.spyOn(myStore, 'foo');
myStore.bar();
expect(spy).toHaveBeenCalled(); // error: `spy` is never called
});
it('fails (with SignalStore)', () => {
const MyStore = signalStore(
withMethods(() => {
function foo() {}
function bar() {
foo();
}
return { foo, bar };
})
);
const myStore = new MyStore();
const spy = jest.spyOn(myStore, 'foo');
myStore.bar();
expect(spy).toHaveBeenCalled(); // error: `spy` is never called
}); I'd recommend against mocking methods within the same store to test other methods in that store. Here's why:
Instead, I'd suggest testing the actual behavior and mocking external dependencies (services, other stores, etc.) rather than internal methods. If you find yourself wanting to mock internal methods, it might indicate the store has too many responsibilities and could benefit from being split into smaller, more focused stores. |
Beta Was this translation helpful? Give feedback.
-
Hi @markostanimirovic :) Thank you for the explanation. One small doubt regarding the test examples: In these cases it will fail because the function Nevertheless, I am digging around the source code of the store and what I can see from there is that Let me present how I would modify the store ecosystem factory functions. I will just raise a draft PR (nothing binding) just to showcase why the test does not work (regardless of what is being tested and any conceptual circumstances of the testing). Myabe it is defind the way it is for some conceptual reason I don't know. So if am wrong, correct me please when you see my draft PR. |
Beta Was this translation helpful? Give feedback.
-
Hi @markostanimirovic and @IhorMaistrenko sorry for for the dalay but we have an extended holiday weekend now in Poland :) @IhorMaistrenko and I we are working together btw 👍 I have some thought on the source code I would love to share with you @markostanimirovic in regard to:
Anyway, will raise my showcase dratf PR tomorrow ;) Today and tomorrow will spend some time on it. Have a great weekend guys! |
Beta Was this translation helpful? Give feedback.
-
@markostanimirovic thank you for explanation, now it makes more sense. In real world project I have custom feature that has method responsible for displaying notifications. And this feature will be added to other stores for the same purpose. My idea was not mocking it, but make sure that method was called. Custom store feature is a building block of store, so I thought it is ok to expect that I can spy for methods in my store if I have added this feature. |
Beta Was this translation helpful? Give feedback.
-
@IhorMaistrenko if you say your feature displays notifications, then it is probably using some services. For example in Angular Material you have |
Beta Was this translation helpful? Give feedback.
-
Hi @rainerhahnekamp and @markostanimirovic yes, we can mock external services. This is feasible. But it would be way more intuitive if we could just spy on an exact method on the store, what we normally do with regular services. But it is not applicable to the store service. Is this for some reason that each of the snapshots and the final store are all independent that way? I think the final store could get all the methods/signals/computed hoisted/moved/attached to it and then the store features would have called with a specific final store snapshot injected at that point and the snapshot would have specific getters return the methods/signals/computed from the final store. This is what I wanna showcase by my draft PR. Does it add up? |
Beta Was this translation helpful? Give feedback.
-
@rainerhahnekamp yes, I already did this and it works. Thank you. |
Beta Was this translation helpful? Give feedback.
-
Ok, I see that my proposal is not met with interest, I give up. Thaks for your explanation guys regarding the issue 👍 |
Beta Was this translation helpful? Give feedback.
-
Hi @pawel-twardziak, to be honest I didn't even find the time to think your proposal, through. But my first impression was to change some parts of the internal, which - honestly speaking - will have a low chance. Especially if we consider its outcome. You can always try to come up with a PR and we can take a look at it. But as I said, I'm not sure that you have a guarantee. |
Beta Was this translation helpful? Give feedback.
-
Hi @rainerhahnekamp thank you for dropping some lines in response to my comments :) Ok, makes sense. Will raise a PR but it will probably happen in around two weeks. Maybe it will add up. 👋 |
Beta Was this translation helpful? Give feedback.
-
On the contrary, we are interested in seeing your proposal! But we cannot commit to anything before seeing it. I'm going to convert this issue to a discussion. If you're still interested in creating a prototype, feel free to share it here and we'll take a look. |
Beta Was this translation helpful? Give feedback.
-
Which @ngrx/* package(s) are the source of the bug?
signals
Minimal reproduction of the bug/regression with instructions
method2
callsmethod1
from another feature)method1
method2
expect(method1).toHaveBeenCalled();
)Project to check reproduction:
https://stackblitz.com/github/IhorMaistrenko/signal-store-feature-methods-testing
There are 2 tests: one is calling methods located at the same feature, second one is calling method inside method from another feature.
Expected behavior
Expect test to pass, because store contains
method1
andmethod1
has been called insidemethod2
.Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)
Test project was create on this environment:
NgRx/signals: 18.1.1
Angular: 18.2.0
Node: v20.12.0
jasmin-core: 5.2.0
Karma: 6.4.0
OS: MacOS
Browser: Chrome
Issue initially was found with this setup:
NgRx/signals: 18.1.0
Angular: 18.2.4
Node: v22.3.0
Jest: 29.7.0
OS: Windows
Browser: Chrome
Other information
Bug was reproduced in different projects with both Jasmine and Jest.
Your Stackblitz project doesn't contain tests, I've tried to add them but got some errors. My project successfully runs in Stackblitz or you can clone if from Github. It is a blank project, only @ngrx/signals added.
P.S. I report bug for the first time. Please let me know if you have any questions
FYI @pawel-twardziak
I would be willing to submit a PR to fix this issue
Beta Was this translation helpful? Give feedback.
All reactions