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

Allow mocking an internal vs external module #44

Closed
ike18t opened this issue Feb 2, 2019 · 15 comments · Fixed by #91, #136 or #233
Closed

Allow mocking an internal vs external module #44

ike18t opened this issue Feb 2, 2019 · 15 comments · Fixed by #91, #136 or #233
Assignees
Labels
Milestone

Comments

@ike18t
Copy link
Collaborator

ike18t commented Feb 2, 2019

My thought around this was that there would be two types of modules:

Internal module:
The module where your declaration lives. In this case we don't really care about exports, we primarily care about imports. The goal is to be able to provide a list of declarations to not mock to give it a quick mocked ecosystem. Internal module imports would all be made up of external module mocks.

External module:
This module would be more about mocking exports.

Giving this separation should resolve the issue listed here:
https://github.com/ike18t/ng-mocks/blob/master/lib/mock-module/mock-module.ts#L159

This could be accomplished by either making two separate methods or by allowing an optional options object param.

@satanTime Thoughts?

@satanTime
Copy link
Member

Hi @ike18t, let me think about.

@satanTime
Copy link
Member

satanTime commented Feb 14, 2019

Hi @ike18t,

I hope you're doing well.
I was thinking about the solution and came to the next idea.

TestBed creates TestModule based on definition we provide, so in our case everything what we have in declarations is part of TestModule, and if the same declaration is used in another module - we receive conflict about double usage.

So if we have Module1 which has Component1 and Component2 and we want to test Component1, then we can't do MockModule(Module1) because Component1 will be part of TestModule and MockOfModule1.

What I would suggest is to add wrapper around what we pass to configuration of TestBed. So instead of

    TestBed.configureTestingModule({
      declarations: [
        TestedComponent,
      ],
      imports: [
        MockModule(DependencyModule),
      ],
    });

to have

    TestBed.configureTestingModule(MockSetupForTestingModule({ /*maybe to call it just Mock */
      declarations: [
        TestedComponent,
      ],
      imports: [
        DependencyModule,
      ],
    }, [TestedComponent, SomeImportedDependecyInModule /* things we don't want to mock */]));

in this case we delegate mocking logic to MockSetupForTestingModule and it checks what and how should be mocked.
Now we can handle everything. So we know we're going to mock everything expect things from 2nd argument, here we can specify any Module, Component, Pipe, Directive, but not providers. They should be defined / mocked as usual.

Then while mocking module we check if one of its Component is in list of declaration - then we do 2 things:

  1. We exclude the Component from declarations of the module, so we don't get error about double usage, and component will be used only in TestModule.
  2. Because now we know that Component belongs to this module - we export all its declarations and imports. This change solves the issue of the ticket.
  3. Profit.

To reduce amount of code we can move call of TestBed inside of MockSetupForTestingModule and return its result so final code could be:

    Mock({
      declarations: [
        TestedComponent,
      ],
      imports: [
        DependencyModule,
      ],
    }, [
        TestedComponent,
        SomeImportedDependecyInModule,
    ]).compileComponents().then(done);

Or we can call it MockTestBed.

Looking forward for your feedback!

@getsaf
Copy link
Collaborator

getsaf commented Feb 14, 2019

Hey guys, we have a pretty flexible solution for this in shallow-render. Basically, you configure your module mocks before creating the module like so:

const rendered = await new Shallow(ComponentToTest, ModuleThatDeclaresTestComponent)
  .dontMock(FooService, BarService, BazModule)
  .mock(BatService, {getBat: () => 'mocked bat'})
  .render();

I don't mind one bit if you want to pull my module mocking out and use it in ng-mocks. If it's fully compatible, I'll remove it from shallow-render and use the one provided by ng-mocks instead.

Could be something like:

const testModule = mockModule(ModuleThatDeclaresTestComponent)
    .dontMock(ComponentToTest, FooService, BarService, BazModule)
    .mock(BatService, {getBat: () => 'mocked bat'})
    .createModule();
await TestBed.configureTestingModule(mockModule).compileComponents();

You get the added benefit of having "stackable" configuration of the mocked module so you could provide a "generic" module in your setup and "extend" that module in each individual test as needed:

  let mockedModule!: MockModule;

  beforeEach(() => {
    mockedModule = mockModule(MyModule)
      .dontMock(MyComponent)
      .mock(FooService, {getFoo: () => 'mocked foo'});
  });

  const render = async (module = mockedModule) => {
    await TestBed.configureTestingModule(module).compileComponents();
    return TestBed.createComponent(MyComponent);
  };

  it('displays the results of getFoo', () => {
    const fixture = await render();
    fixture.query(By.css('.submit')).triggerEventHandler('click', {});
    fixture.detectChanges();
    const fooMessage = fixture.query(By.css('.foo-message'));
    
    expect(fooMessage.nativeElement.textContent).toBe('mocked foo');
  });

  it('displays a message when the FooService throws an error', () => {
    // Here, we can override the FooService mock to test the negative use-case
    const fixture = await render(mockedModule.mock(FooService, { getFoo: () => throw new Error('Boom!')});
    fixture.query(By.css('.submit')).triggerEventHandler('click', {});
    fixture.detectChanges();
    const errorMessage = fixture.query(By.css('.error-message'));

    expect(errorMessage.nativeElement.textContent).toBe('Foo service is unavailable');
  })

If you go this route, eventually, you're going to want to do things like replaceModule so you can use RouterTestingModule and NoopAnimationsModule while testing with modules that use their counterparts. That might lead you to want to do it globally with an alwaysReplaceModule... By then, you'll be pretty close to the whole mocking system in shallow-render so you may as well take it all (if you take any).

Using shallow-render for your tests is always an option too :). All kidding aside, there's no reason you couldn't use a combination of ng-mocks and shallow-render across a test suite if one doesn't suit all your needs.

@satanTime
Copy link
Member

satanTime commented Feb 15, 2019

Hi @getsaf, thanks for the suggestion I'll check your implementation.

In case of ng-mocks, there's no reason to replace neither RouterModule or AnimationsModule, they're mocked correctly, but if you want to use Test and Noop instead - no problem to do so too.

Last time when I checked shallow-render, I found that it uses different approach how to mock declarations and, unfortunately, it doesn't cover a lot of cases which ng-mock does, that's why I continued to contribute to ng-mocks.

IMHO - I would keep close to TestBed interface as much as possible so people who use TestBed could easily switch to ng-mocks, without requirement to learn another approach, just to replace / wrap some names.

I'll check on weekend how shallow-render can help and leave my feedback here.

@ike18t
Copy link
Collaborator Author

ike18t commented Feb 24, 2019

I've been toying with the concept of a MockModuleBuilder similar to the approach @getsaf mentioned. I think that we may be able to leverage the pre-existing cache logic that is used by the mock functions to thread in some overrides. If and when I get a functional POC I'll open a PR for you guys to check out.

@satanTime
Copy link
Member

as I understand goal of builder is to override some modules / components / declarations instead of mock. It can be done with TestBed itself without any builder: https://angular.io/api/core/testing/TestBed, there're override* methods which allow you to override any module / component / declaration in TestBed as you want.

If there're any other cases, which TestBed doesn't cover - let's discuss them.

@ike18t
Copy link
Collaborator Author

ike18t commented Feb 24, 2019 via email

@satanTime
Copy link
Member

satanTime commented Feb 25, 2019

Nice test case, checking.

it's possible to do next way:

describe('override-pipe:mock', () => {
  beforeEach(() => {

    @Pipe({
      name: 'mockedPipe',
    })
    class MockedPipe implements PipeTransform {
      transform = (text: string): string => `mock:${text}`;
    }

    TestBed.configureTestingModule({
      declarations: [
        MockPipe(TranslatePipe),
        MockedPipe,
      ]
    });

    TestBed.overridePipe(MockedPipe, {
      add: {
        name: 'translate',
      },
    });
  });

  it('translates fake', () => {
    const fixture = MockRender(`{{ value | translate }}`, {
      value: 'label',
    });
    expect(fixture.nativeElement.innerText).toContain('mock:label');
  });
});

Huge and ugly. Definitely what @getsaf suggested makes more sense. Checking how much effort it requires for integration.

Checked. Unfortunately, it depends on own mocking system and there's no way to add abstraction so we could inject own mocking system. But we can use the same interface with own implementation.

Result can be like:

const testingModule = Mock({
  providers: [RealProvider],
  declarations: [RealComponent],
  imports: [RealModule],
}).dontMock(RealComponent, RealProvider).mock(TranslatePipe, {
  transform: (value: string) => `translated.${value}`,
}).merge(ModuleWithCustomizedMocks, SomeMockedComponent).createTestingModule();

TestBed.configureTestingModule(testingModule).compileComponents();

everything what we got in Mock we'll mock recursively. dontMock accept anything we should except from mocking. mock accept base class and Partial implementation of its methods we want to change, also 2nd argument can be any class for replacement, not only partial. merge is responsible to override the same elements in testingModule, so if we have some Component / Pipe / Directive and we have the same in merge, then first one will be ignored and value from merge is used. It allows us to create TestModule, define there all custom logic like TranslationPipe, so we don't need to copy-paste the same dontMock and mock chains in every test, we can simply copypaste only .dontMock(SomethingToTest).merge(TestModule).

Looking forward for your thoughts.

@ike18t
Copy link
Collaborator Author

ike18t commented Feb 26, 2019

I actually pulled in shallow-render into my team's application today. It really did simplify all of the boiler plate involved with getting TestBed up and running by just specifying the parent module and the component under test. The finders are also really nice since they return typed components so that I don't have to cast the resulting query component instance. It also allowed me to create a global mock of the pipe I had in my example so that it only had to be configured once which was really nice. Considering I'm the only one who really asked for this feature and since shallow-render solved the problem then I guess I'm good.

If it is OK with you I can go ahead and close this issue unless you'd like to look into it further?

@satanTime
Copy link
Member

satanTime commented Feb 26, 2019

Glad to hear! I also like its idea, the problem for me was only missed mocking feature for ng-template. In rest it has all required things.

Back to ng-mocks. In my todo list it was last task: to minimize usage of TestBed to provide only some declaration to test and mock whole AppModule. Very similar to this task.
Currently declaration from TestBed and declaration from AppModule has conflict about usage of the same selector in different components.
The idea was to wrap TestBed setup to get way to mock AppModule in smart way to avoid such conflicts.

I would suggest to discuss here interface and once it's fine I can implement it.

Currently I can suggest one from comment above:

const testingModule = MockBuilder({
  providers: [RealProvider],
  declarations: [RealComponent],
  imports: [RealModule],
}).dontMock(RealComponent, RealProvider).mock(TranslatePipe, {
  transform: (value: string) => `translated.${value}`,
}).merge(ModuleWithCustomizedMocks, SomeMockedComponent).createTestingModule();

TestBed.configureTestingModule(testingModule).compileComponents();

and mirror of shallow-render

const testingModule = MockBuilder(AppModule).dontMock(RealComponent, RealProvider).mock(TranslatePipe, {
  transform: (value: string) => `translated.${value}`,
}).merge(ModuleWithCustomizedMocks, SomeMockedComponent).createTestingModule();

TestBed.configureTestingModule(testingModule).compileComponents();

What do you prefer?

@ike18t
Copy link
Collaborator Author

ike18t commented Mar 8, 2019

I think I'd choose the 2nd approach, primarily because it requires less boiler plate and is more about mocking a module.

@satanTime
Copy link
Member

Great, thanks for the feedback, next weeks I'll prepare PR.

@satanTime satanTime self-assigned this Feb 22, 2020
@satanTime satanTime added the enhancement New feature or request label Feb 22, 2020
@satanTime satanTime linked a pull request Feb 29, 2020 that will close this issue
17 tasks
@satanTime
Copy link
Member

Tomorrow I'll updated docs. The PR is ready for a review.

@satanTime
Copy link
Member

The doc is ready and you can check it here: https://github.com/satanTime/ng-mocks/tree/issues/44

@satanTime satanTime mentioned this issue Mar 11, 2020
17 tasks
@satanTime satanTime added this to the v10.0.0 milestone May 18, 2020
@satanTime satanTime mentioned this issue Jun 6, 2020
satanTime referenced this issue in satanTime/ng-mocks Jun 6, 2020
satanTime referenced this issue in satanTime/ng-mocks Jun 6, 2020
satanTime referenced this issue in satanTime/ng-mocks Jun 6, 2020
satanTime referenced this issue in satanTime/ng-mocks Oct 29, 2020
satanTime referenced this issue in satanTime/ng-mocks Oct 29, 2020
satanTime referenced this issue in satanTime/ng-mocks Oct 29, 2020
satanTime referenced this issue in satanTime/ng-mocks Nov 13, 2020
BREAKING CHANGE: respects internals vs externals, to export things use MockBuilder

closes #44
satanTime referenced this issue in satanTime/ng-mocks Nov 13, 2020
BREAKING CHANGE: respects internals vs externals, to access them use guts or MockBuilder

closes #44
satanTime referenced this issue in satanTime/ng-mocks Nov 13, 2020
BREAKING CHANGE: respects internals vs externals, to access them use guts or MockBuilder

closes #44
@satanTime satanTime linked a pull request Nov 13, 2020 that will close this issue
satanTime referenced this issue in satanTime/ng-mocks Nov 13, 2020
BREAKING CHANGE: respects internals vs externals, to access them use guts or MockBuilder

closes #44
satanTime referenced this issue in satanTime/ng-mocks Nov 14, 2020
BREAKING CHANGE: respects internals vs externals, to access them use guts or MockBuilder

closes #44
satanTime referenced this issue in satanTime/ng-mocks Nov 14, 2020
BREAKING CHANGE: respects internals vs externals, to access them use guts or MockBuilder

closes #44
satanTime added a commit that referenced this issue Nov 15, 2020
# [11.0.0](v10.5.4...v11.0.0) (2020-11-15)

### Bug Fixes

* removing deprecations ([2625352](2625352))
* respecting internals vs externals ([d4abf41](d4abf41)), closes [#44](#44)

### Features

* angular 11 support ([eef7b94](eef7b94))

### BREAKING CHANGES

* respects internals vs externals, to access them use guts or MockBuilder
* removed NG_GUARDS, use NG_MOCKS_GUARDS
* removed NG_INTERCEPTORS, use NG_MOCKS_INTERCEPTORS
* removed custom meta in MockComponent
* removed MockHelper, use ngMocks
* A11
@satanTime
Copy link
Member

v11.0.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.

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