help-me-mom / ng-mocks

Angular testing library for mocking components, directives, pipes, services and facilitating TestBed setup
https://www.npmjs.com/package/ng-mocks
MIT License
1.05k stars 76 forks source link

MockRender is not working when we try to spy on a service method which is called in component constructor #488

Closed vinayk406 closed 3 years ago

vinayk406 commented 3 years ago

Stackblitz link to reproduce this issue https://stackblitz.com/edit/ng-mocks-examples-gwee2q?file=app/app.component.spec.ts

satanTime commented 3 years ago

Hi again,

Thanks for the report.

This is expected. Because MockRender adds a middleware component.

To spy in such cases MockInstance should be used.

Please don't close the ticket. I'll update docs based on the issue.

vinayk406 commented 3 years ago

@satanTime Can we add a configurable option to bypass the middleware component?

add it only when needed?

something like MockRender(XComponent, null, { renderWithoutWrapperComponent: true })

satanTime commented 3 years ago

Something like that.

describe("mock extended component", () => {
  beforeEach(() => {
    MockInstance(AppService, 'method', jasmine.createSpy('method'));
    //
    // Or simply ngMocks.autoSpy('jasmine');
  });

  beforeEach(() => MockBuilder(AppComponent, AppModule));

  it("expect service.method to have been called", () => {
    MockRender(AppComponent);

    const service = TestBed.get(AppService);
    expect(service.method).toHaveBeenCalled();
  });
});

If you want to bypass the middleware component it means you don't need MockRender and could use TestBed.createComponent instead.

The idea of ng-mocks and its tools is to allow to configure and customize mock environment before TestBed initialization. And then in tests to render required stuff and to do assertions.

vinayk406 commented 3 years ago

Thanks for the explanation 👍

satanTime commented 3 years ago

You are welcome, feel free to ask any questions and propose any changes.

I'll dive deeper and check if it is possible somehow to inject a component after the initialization and post an update here.

The main diff between createComponent and MockRender is that createComponent allow to test inputs, outputs, queries and some lifecycle hooks. And, ideally, it should be a replacement for TestBed.createComponent, but such cases, like you have: when there is a real service and only one of its methods should be mocked, unfortunately, doesn't work.

Might I ask to explain why you want to keep AppService as it is and only to mock one method?

vinayk406 commented 3 years ago

Here is the snippet from our codebase.

configureModuleBuilder(() => {
    return MockBuilder(XComponent, XModule)
        .mock(DomSanitizer, { bypassSecurityTrustUrl: v => v }, { precise: true })
        .provide([
            { provide: I18NService, useClass: I18NServiceMock },
            { provide: WindowService, useClass: WindowMock },
            { provide: Store, useClass: StoreMock },
            XActions,
        ]);
});

beforeEach(() => {
    mockWindowService = TestBed.inject(WindowService);
    store = TestBed.inject(Store);
    spyOn(store, 'dispatch');

    fixture = TestBed.createComponent(XComponent);
    component = fixture.componentInstance;
    fixture.detectChanges();
});

it('dispatches ', () => {
    expect(store.dispatch).toHaveBeenCalledWith();
});

When I tried to use MockRender instead of TestBed.createComponent it failed.

It will be good if MockRender has the default behaviour of createComponent. use the wrapper component only when needed. or an option to bypass when not required.

we are in the process of migrating all our testcases to use MockRender configureModuleBuilder used here to configure a module and reuse across the spec file between tests. next step is to explore on the memoization of MockRender to improve the performance further.

satanTime commented 3 years ago

Oh, it looks a bit confusing. WindowService and Store are mock classes already, and to mock them further could be avoided I think.

Might you share how I18NServiceMock, WindowMock and StoreMock look like? I think you could use ngMocks.defaultMock and MockInstance for such a test, then its code should be shortened to a few lines.

vinayk406 commented 3 years ago

I18nServiceMock, WindowMock and StoreMock are different. different spec files may require different versions of these.

Sorry. Can't share the code for these.

satanTime commented 3 years ago

Sure, I totally understand this.

I'll try to find a way how to make your test working and being something like:

configureModuleBuilder(() => {
  MockInstance(DomSanitizer, 'bypassSecurityTrustUrl', v => v);
  MockInstance(I18NService, I18NServiceMock);
  MockInstance(WindowService, WindowMock);
  MockInstance(Store, StoreMock);
  MockInstance(Store, store => spyOn(store, 'dispatch'));

  return MockBuilder(XComponent, XModule).provide(XActions);
});

beforeEach(() => {
  fixture = MockRender(XComponent);
  component = fixture.point.componentInstance;
  mockWindowService = TestBed.inject(WindowService);
  store = TestBed.inject(Store);
});

it('dispatches ', () => {
  expect(store.dispatch).toHaveBeenCalledWith();
});
vinayk406 commented 3 years ago

Thank for your time on this. And please take a look into memoization of MockRender when you find some time.

satanTime commented 3 years ago

And please take a look into memoization of MockRender when you find some time.

Could you create a separate issue and provide an example how you want it to memoize the things?

vinayk406 commented 3 years ago
  beforeEach(() => {
    MockInstance(Store, 'dispatch', jasmine.createSpy());
    MockInstance(WindowService, 'open', jasmine.createSpy());

    mockWindowService = TestBed.inject(WindowService);
    store = TestBed.inject(Store);

    fixture = MockRender(XComponent);
    component = fixture.componentInstance;
  });

This snippet is not working.

vinayk406 commented 3 years ago

And please take a look into memoization of MockRender when you find some time.

Could you create a separate issue and provide an example how you want it to memoize the things?

Sure. will do.

satanTime commented 3 years ago

Hi,

the snippet you provided won't work, because calls of TestBed.inject are done before MockRender.

I think, I understood the point, which is missing in all examples above, this is incorrect usage of tools:

To make the example above work properly, its test should look like that:

// simplifies spy configuration
beforeEach(() => ngMocks.autoMock('jasmine'));
// beforeEach(() => MockInstance(Store, 'dispatch', jasmine.createSpy()));
// beforeEach(() => MockInstance(WindowService, 'open', jasmine.createSpy()));

// describing environment
beforeEach(() => MockBuilder(XComponent).mock(Store).mock(WindowService));

// initializing testing environment
beforeEach(() => {
  fixture = MockRender(XComponent);
  mockWindowService = TestBed.inject(WindowService);
  store = TestBed.inject(Store);
});

it('test', () => {
  expect(store.dispatch).toHaveBeenCalled();
});

For the mock classes, could you try like that and let me know if it works?

configureModuleBuilder(() => {
  return MockBuilder(XComponent, XModule)
    .provide([
      MockProvider(DomSanitizer, { bypassSecurityTrustUrl: v => v }),
      MockProvider(I18NService, new I18NServiceMock()),
      MockProvider(WindowService, new WindowMock()),
      MockProvider(Store, ngMocks.stub(new StoreMock(), {
        dispatch: jasmine.createSpy(), // a way to provide a spy before call of MockRender
      })),
      XActions,
    ]);
});
satanTime commented 3 years ago

v11.11.0 has been released and contains a fix for the issue. Feel free to reopen the issue or to submit a new one if you meet any problems.

satanTime commented 3 years ago

Hi @vinayk406,

I've added an option to memories MockRender if it is used in beforeAll: https://ng-mocks.sudo.eu/api/ngMocks/faster#mockrender

satanTime commented 3 years ago

And this is info about default values in params: https://ng-mocks.sudo.eu/api/MockRender#params-inputs-and-outputs