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

Add testing guide for NgRx SignalStore #4206

Closed
1 of 2 tasks
markostanimirovic opened this issue Jan 8, 2024 · 37 comments · Fixed by #4461
Closed
1 of 2 tasks

Add testing guide for NgRx SignalStore #4206

markostanimirovic opened this issue Jan 8, 2024 · 37 comments · Fixed by #4461

Comments

@markostanimirovic
Copy link
Member

Information

The new page should be created for the testing guide: @ngrx/signals > SignalStore > Testing

The guide should explain how SignalStore should be tested with examples.

Documentation page

No response

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@va-stefanek
Copy link
Contributor

Can take care of it.

Any specific best practices you would like to be included?

@rainerhahnekamp
Copy link
Contributor

Can I join in this?

@va-stefanek
Copy link
Contributor

@rainerhahnekamp sure. We need to clarify whether @markostanimirovic has some specific requirements he would love to put on that page

@markostanimirovic
Copy link
Member Author

Hey 👋

Here are the main topics I'd like to be covered:

  • Testing SignalStore without TestBed (SignalStore without dependencies - focus on testing core functionalities)
  • Testing SignalStore with TestBed (incl. mocking dependencies)
  • Testing rxMethod

Optionally:

  • Testing custom SignalStore features
  • Testing custom updaters

For more inspiration check how SignalStore Core Concepts and Custom Store Features pages are structured (attention to details, examples, etc.).

@rainerhahnekamp
Copy link
Contributor

Okay, about testing without TestBed: I think the number of stores without DI, will be a minority.

You will very likely have an HttpClient or a service depending on it almost always. I think that kind a test without a TestBed only applies to testing the store itself (so tests which are part of NgRx), or if for stores managing a UI state.

I would include an example without TestBed at the beginning, but I'd see the main focus on test cases with DI.

When it comes to mocking, we should also have a chapter on mocking the Store itself when it is used inside a component.

rxMethod should show how to test it with marbles but also without it.

What do you think?

@markostanimirovic
Copy link
Member Author

Sounds good to me 👍

@jordanpowell88
Copy link
Contributor

Excited for this documentation to land.

@rainerhahnekamp
Copy link
Contributor

Just one addition: I think we should also include some type testing. I find that important for advanced use cases.

@ajlif
Copy link

ajlif commented Feb 2, 2024

excited as well for this documentation.
Is there any workaround to spy on patchState and injected services in withMethods ?
i have already tried to mock followig @markostanimirovic comment here in angular but it doesn't work:

import * as fromUsersStore from './users.store'; // test jasmine.spyOn(fromUsersStore, 'injectUsersStore').mockReturnValue({ /* fake users store */ });

@rainerhahnekamp
Copy link
Contributor

Hi, you want to create a spy on patchState itself? I am afraid that will not work. patchState as standalone is very similar to inject, which we also cannot spy unless the framework itself provides mocking/spy functionalities.

What I would recommend instead, that you don't spy on the packState but check against the value of the state itself.

So something like:

Store:

const CounterStore = signalStore(
  {providedIn: 'root'},
  withState({counter: 1})
);

Service which you want to test:

@Injectable({providedIn: 'root'})
export class Incrementer {
  store = inject(CounterStore);

  increment() {
    patchState(this.store, value => ({counter: value.counter + 1}));
  }
}

The actual test:

it('should verify that increment increments', () => {
  const counterStore = TestBed.inject(CounterStore);
  const incrementer = TestBed.inject(Incrementer);

  expect(counterStore.counter()).toBe(1);

  incrementer.increment();
  TestBed.flushEffects();

  expect(counterStore.counter()).toBe(2);
})

I didn't try out that code. Might include even some syntax errors, but that's the general approach I would recommend.

Is it what you had in mind?

@ajlif
Copy link

ajlif commented Feb 2, 2024

ok that's clear, what about spy on injected services in withMethods . I want to check that a service is called .
in Component Stores we used to use component injection and spy using spyOn:

   const spy = spyOn((store as any).injectedService, 'methodInsideInjectedService');

@markostanimirovic
Copy link
Member Author

ok that's clear, what about spy on injected services in withMethods . I want to check that a service is called . in Component Stores we used to use component injection and spy using spyOn:

   const spy = spyOn((store as any).injectedService, 'methodInsideInjectedService');

In the same way as you do for any other service using TestBed:

TestBed.configureTestingModule({
  providers: [MyStore, { provide: MyService, useValue: { doSomething: jest.fn() } }],
});

const myStore = TestBed.inject(MyStore);
const myService = TestBed.inject(MyService);

myStore.doSomething();

expect(myService.doSomething).toHaveBeenCalled();

@jits
Copy link
Contributor

jits commented Feb 7, 2024

In the same way as you do for any other service using TestBed:

TestBed.configureTestingModule({
  providers: [MyStore, { provide: MyService, useValue: { doSomething: jest.fn() } }],
});

const myStore = TestBed.inject(MyStore);
const myService = TestBed.inject(MyService);

myStore.doSomething();

expect(myService.doSomething).toHaveBeenCalled();

Has anyone got this to work? Using jasmine.createSpy() I get typing errors due to the Unsubscribable type required both on result type and on the spy itself. UPDATE: I misread this and the solution above still stands for mocking a method on a service used within the Store. My question/issue below is about mocking / spying on an rxMethod method.

I'm trying to implement a helper method to generate an rxMethod spy but stuck on making it work without manual typecasting etc.:

import type { Signal } from '@angular/core';
import type { Observable, Unsubscribable } from 'rxjs';

// This reproduces the same types from @ngrx/signals (which aren't exported)
type RxMethodInput<Input> = Input | Observable<Input> | Signal<Input>;
type RxMethod<Input> = ((input: RxMethodInput<Input>) => Unsubscribable) & Unsubscribable;

export const buildRxMethodSpy = <Input>(name: string) => {
  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  const rxMethodFn = (input: RxMethodInput<Input>) => {
    return {
      unsubscribe: () => {
        return;
      },
    };
  };
  rxMethodFn.unsubscribe = () => {
    return;
  };
  const spy = jasmine.createSpy<RxMethod<Input>>(name, rxMethodFn);
  // Somehow add `.unsubscribe` to the spy in a way that TypeScript understands
  return spy;
};

Note: I already tried using jasmine.createSpy() directly but encountered the typing issues; the above code is likely overkill, assuming there's a solution to typing jasmine.createSpy() properly.


Update: for now I'm resorting to explicit typecasting:

import type { Signal } from '@angular/core';
import { noop, type Observable, type Unsubscribable } from 'rxjs';

// This reproduces the same types from @ngrx/signals (which aren't exported)
type RxMethodInput<Input> = Input | Observable<Input> | Signal<Input>;
type RxMethod<Input> = ((input: RxMethodInput<Input>) => Unsubscribable) & Unsubscribable;

export const buildRxMethodSpy = <Input>(name: string) => {
  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  const rxMethodFn = (input: RxMethodInput<Input>) => {
    return {
      unsubscribe: noop,
    };
  };
  rxMethodFn.unsubscribe = noop;
  const spy = jasmine.createSpy<RxMethod<Input>>(name, rxMethodFn) as unknown as jasmine.Spy<
    RxMethod<Input>
  > &
    Unsubscribable;
  spy.unsubscribe = noop;
  return spy;
};

But would very much like to hear if there's a better way to do this.

@rainerhahnekamp
Copy link
Contributor

@jits it looks like you want to spy on the Store's method itself, right?

The example you are referring to, is about a service which is used in the Store, and you want to spy on a method on that service.

@jits
Copy link
Contributor

jits commented Feb 21, 2024

Hi @rainerhahnekamp — ahh yes, good spot, thanks. I missed that aspect. (I'll update my comment to reflect this).

I don't suppose you have a good way to build a mock or spy for an rxMethod method? (Without resorting to manual type casting).

@rainerhahnekamp
Copy link
Contributor

@jits What do you think about that? #4256

@jits
Copy link
Contributor

jits commented Feb 21, 2024

@rainerhahnekamp — that sounds great! (I'll continue with additional thoughts there)

@AshMcConnell
Copy link

AshMcConnell commented Jun 21, 2024

Most things just work as normal (when you use TestBed), but is there a trick to get computed variables to fire?

withComputed(({request, patient}) => ({
        // Computed state from other parts of the state
        patientRequestDoctorModel: computed(() =>
            request() ? PatientRequestDoctorModel.fromRequest(request()) : PatientRequestDoctorModel.fromPatient(patient())
        )
    }))

I expected it to work when either request or patient signals changed, but it doesn't fire in the test.

EDIT Nevermind, I'm an idiot, I forgot that computed were lazy. I had just run the other tests that changed request and patient, but I hadn't asserted on patientRequestDoctorModel(), so it didn't run the computed. At least it is a cautionary tale for googlers 😂

@markostanimirovic
Copy link
Member Author

Is anyone still interested in working on this issue? 👀

@rainerhahnekamp
Copy link
Contributor

Yes, this is more than overdue. Sorry and expect it by the end of this week

@jits
Copy link
Contributor

jits commented Jul 15, 2024

Hi @rainerhahnekamp. I'd be happy to lend a hand — let me know if I can help in any way.

(And, sorry, I realise I haven't responded to yours and Gergely's comments on #4256 — I'll try to get to that this week).

@Nikzz3
Copy link

Nikzz3 commented Jul 26, 2024

Looking forward to this. When can I except this?

@rainerhahnekamp
Copy link
Contributor

@Nikzz3 I've published the PR for the draft. As you can see, it doesn't contain much, so we can work on it together. I will be able to commit a big chunk over the weekend.

@rainerhahnekamp
Copy link
Contributor

The testing guide is now ready for review. Since we don't have any particular testing libraries, the guide focuses on presenting and explaining common testing scenarios, so it drifted a little bit from the initial plan.

@jits and @gergelyszerovay: We had some discussions #4256 around issues mocking rxMethod and the need for a mocked Signal Store. The guide has examples that use ng-mocks and native jest.fn(). Can you check them out and let us know if there are still issues left a "MockSignalStore" could solve?

Can we continue the discussion directly in the PR #4461

@rainerhahnekamp
Copy link
Contributor

@va-stefanek 🫵 your turn as well 😅

@gergelyszerovay
Copy link

Can you check them out and let us know if there are still issues left a "MockSignalStore" could solve?

I think the guide covered almost all aspects of manual testing and mocking a Signal store. One thing I would add to the guide: methods for tracking the times when the internal pipe in the rxMethod has been triggered.
I described a way to do this with my newFakeRxMethod() function in this comment:

#4427 (reply in thread)

Using an auto-mocked Signal store can speed up the development in case we have many stores in a project with a lot of updaters and selectors. (I have experienced this use case many times in the past with the ComponentStore, this is why I thought this mock store might be useful for others.)

@jits
Copy link
Contributor

jits commented Aug 2, 2024

@rainerhahnekamp

(I'm commenting here and not directly on #4461 as you requested, as I think my comment isn't directly related to your PR and shouldn't impact it).

The testing guide is looking great! In terms of mocking rxMethods, it's good to know you can directly use jest.fn<void, [Signal<string>]>() or jest.spyOn(moviesStore, 'load');. When I tried something similar with jasmine.createSpy I kept getting typing errors, so had to resort to a hacky custom spy builder. Maybe it's because I'm trying to mock and spy at the same time, not sure. In any case, I don't currently use Jest — will switch to it if/when Angular switches to it officially. So I can't add much more at the moment. Hopefully these discussions and your testing guide will be enough for anyone looking for solutions to these testing needs :)

@rainerhahnekamp
Copy link
Contributor

@jits, no problem I will pick up all your comments (as long as they are somewhere on this repo ;)) and will try to improve the testing guide based on that.

@jits
Copy link
Contributor

jits commented Aug 2, 2024

Thanks @rainerhahnekamp. Especially for your invaluable work here.

I should add: I haven't really delved too deeply into testing stores (and store dependencies) just yet, but will likely do so in the next couple of months. I'll come back to your work here when I do and contribute more.

@bvasen
Copy link

bvasen commented Sep 26, 2024

Is there any way on how to setup a specific state of a signalstore in a test for that store when the store has proctectedState: true?

In v17 you could just patch the signalstore to have a specific state for a test (that is different from the initial state) and test how the store behaves in that case.

In v18 I would like to set the store to protectedState: true, but the only two ways I got it working is to set the protectedState to false or to call the methods of the store to setup the state I need for the test, which leads to using a bunch of other store methods when writing a test for one specific method.
Would be nice if there would be a way to set the store to a specific state when testing it, e.g. to be able to set proctectedState to false just for the test.
If there is already a way to do that would be nice if that is documented in the testing guide as well.

@mzkmnk
Copy link

mzkmnk commented Oct 3, 2024

@bvasen
You can read more about the test at the following link: #4427

@rainerhahnekamp
Copy link
Contributor

@bvasen and @mzkmnk, in the meantime there is this: #4540

@mzkmnk
Copy link

mzkmnk commented Oct 3, 2024

@rainerhahnekamp
thanks 👍
Personally, I think injectUnprotectedStore is better because it makes fewer mistakes.

@rainerhahnekamp
Copy link
Contributor

@mzkmnk you need to add your comment in the issue 😀

@mzkmnk
Copy link

mzkmnk commented Oct 3, 2024

@rainerhahnekamp OK 👍

@Francesco-Borzi
Copy link

what about signalState ? what is the recommended way to mock it?

@rainerhahnekamp
Copy link
Contributor

@Francesco-Borzi, the signalState is essentially a DeepSignal. For mocking, you could create another signalState and populate it with the data your “mock” requires.

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

Successfully merging a pull request may close this issue.