platform icon indicating copy to clipboard operation
platform copied to clipboard

Add testing guide for NgRx SignalStore

Open markostanimirovic opened this issue 2 years ago • 29 comments

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
  • [X] No

markostanimirovic avatar Jan 08 '24 19:01 markostanimirovic

Can take care of it.

Any specific best practices you would like to be included?

va-stefanek avatar Jan 09 '24 10:01 va-stefanek

Can I join in this?

rainerhahnekamp avatar Jan 09 '24 19:01 rainerhahnekamp

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

va-stefanek avatar Jan 11 '24 11:01 va-stefanek

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.).

markostanimirovic avatar Jan 11 '24 12:01 markostanimirovic

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?

rainerhahnekamp avatar Jan 11 '24 13:01 rainerhahnekamp

Sounds good to me 👍

markostanimirovic avatar Jan 11 '24 16:01 markostanimirovic

Excited for this documentation to land.

jordanpowell88 avatar Jan 11 '24 17:01 jordanpowell88

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

rainerhahnekamp avatar Jan 12 '24 21:01 rainerhahnekamp

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 */ });

ajlif avatar Feb 02 '24 08:02 ajlif

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?

rainerhahnekamp avatar Feb 02 '24 10:02 rainerhahnekamp

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');

ajlif avatar Feb 02 '24 11:02 ajlif

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();

markostanimirovic avatar Feb 02 '24 11:02 markostanimirovic

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.

jits avatar Feb 07 '24 16:02 jits

@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.

rainerhahnekamp avatar Feb 21 '24 14:02 rainerhahnekamp

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).

jits avatar Feb 21 '24 14:02 jits

@jits What do you think about that? https://github.com/ngrx/platform/issues/4256

rainerhahnekamp avatar Feb 21 '24 14:02 rainerhahnekamp

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

jits avatar Feb 21 '24 15:02 jits

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 😂

AshMcConnell avatar Jun 21 '24 10:06 AshMcConnell

Is anyone still interested in working on this issue? 👀

markostanimirovic avatar Jul 14 '24 22:07 markostanimirovic

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

rainerhahnekamp avatar Jul 15 '24 06:07 rainerhahnekamp

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).

jits avatar Jul 15 '24 09:07 jits

Looking forward to this. When can I except this?

Nikzz3 avatar Jul 26 '24 09:07 Nikzz3

@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 avatar Jul 26 '24 09:07 rainerhahnekamp

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 avatar Aug 01 '24 12:08 rainerhahnekamp

@va-stefanek 🫵 your turn as well 😅

rainerhahnekamp avatar Aug 01 '24 14:08 rainerhahnekamp

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:

https://github.com/ngrx/platform/discussions/4427#discussioncomment-10216019

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.)

gergelyszerovay avatar Aug 01 '24 19:08 gergelyszerovay

@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 :)

jits avatar Aug 02 '24 15:08 jits

@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.

rainerhahnekamp avatar Aug 02 '24 15:08 rainerhahnekamp

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.

jits avatar Aug 02 '24 15:08 jits