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

@ngrx/signals: mockSignalStore feature #4256

Open
1 of 2 tasks
rainerhahnekamp opened this issue Feb 21, 2024 · 15 comments
Open
1 of 2 tasks

@ngrx/signals: mockSignalStore feature #4256

rainerhahnekamp opened this issue Feb 21, 2024 · 15 comments

Comments

@rainerhahnekamp
Copy link
Contributor

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 contain jest.fn or jasmine.createSpy for the methods and computeds:

const PersonStore = signalStore(
  withState({id: 1, firstname: 'Rudolf', lastname: 'Weber'}),
  withMethods(store => {
    return {
      async load() {
        // ...
      },
      rename() {
        // ...
      }
    }
  }),
  withComputed(state => {
    return {
      prettyName: computed(() => `${state.firstname()} ${state.lastname()}`)
    }
  })
)

And then inside a test

const mockedStore = mockSignalStore(PersonStore, {id: 2, firstname: 'Anna', lastname: 'Niedermayer'});

TestBed.configureTestingModule({providers: [
  {provide: PersonStore, useValue: mockedStore}
]});

// jest
mockedStore.load.mockImplementation(() => true)

//jasmine
mockedStore.load.and.returnValue(true);

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

  • Yes
  • No
@jits
Copy link
Contributor

jits commented Feb 21, 2024

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

@rainerhahnekamp
Copy link
Contributor Author

I see. Well, in that case, we might want to discuss a set of testing helpers.

@jits
Copy link
Contributor

jits commented Feb 21, 2024

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

@rainerhahnekamp
Copy link
Contributor Author

Yeah, then it's up to the NgRx guys to decide if that is something we should have or not.

@markostanimirovic
Copy link
Member

Another suggestion on this topic: #4252

@rainerhahnekamp
Copy link
Contributor Author

rainerhahnekamp commented Feb 21, 2024

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:

  1. Don't use any third-party library, but provide methods and computers returning undefined. It is the up to the user to use their tools to mock/spy in whatever way they want.
  2. Use a framework-agnostic mocking mechanism. So the provideSignalStore function would then allow to define the behavior directly (very similar to ng-mocks): https://ng-mocks.sudo.eu/api/MockProvider/#useclass
  3. Provide different implementations for the common frameworks. This might act as an example: https://github.com/cartant/rxjs-marbles/tree/main/source

@jits
Copy link
Contributor

jits commented Feb 22, 2024

Aside: @rainerhahnekamp — you may have intended to use a different link for your point (2) above?

@gergelyszerovay
Copy link

gergelyszerovay commented Feb 23, 2024

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_COMPUTEDs by WritableSignals
MOCK_METHODs by Sinon fakes
MOCK_RXMETHODs by FakeSinonRxMethods

The drawback of this approach is that supporting more test/mock frameworks would mean more dependencies.

@rainerhahnekamp
Copy link
Contributor Author

Great article, @gergelyszerovay!

Please give me some days to collect and structure all my thoughts on your implementation.

@LPCmedia
Copy link

LPCmedia commented Mar 9, 2024

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

@rainerhahnekamp
Copy link
Contributor Author

I see two non-technical reasons that speak against Sinon.

  • We want to have as few libraries as possible. The more you have, the harder it becomes to upgrade Angular. We even see that companies decide against libraries because of too many dependencies.
  • Jasmine & Jest have their own spy/mock implementation. As @jits pointed out, provideMockStore should integrate into the existing testing tools that developers use already.

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?

@jits
Copy link
Contributor

jits commented Mar 12, 2024

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

@rainerhahnekamp
Copy link
Contributor Author

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.

@gergelyszerovay
Copy link

gergelyszerovay commented Mar 17, 2024

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

  • for all the dumb components ('UI components', they render the UI based only on their inputs, and usually they have no external service dependencies, and all the components in their template are dumb components). Some of them also have a component-level store, to manage the internal state
  • for many of the smart components. Most of these have a component-level store and several service dependencies. In addition, application and function/domain level stores are used in complex applications.

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 updaters and effects, and the new RxMethods. These three methods work similarly, all the three return a function, that can be called:

  • with a static value
  • with a signal argument, and the inner function might be executed when signal's value changes
  • with an observable argument, and the inner function is executed each time when the observable emits

To mock these, I've created a fakeRxMethod function, we can also use this inside a MockProvider.

@rainerhahnekamp
Copy link
Contributor Author

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.

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

No branches or pull requests

5 participants