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

"NullInjectorError: No provider for Location!" when using createRoutingFactory #179

Closed
andy-rash opened this issue Sep 4, 2019 · 16 comments · Fixed by #188
Closed

"NullInjectorError: No provider for Location!" when using createRoutingFactory #179

andy-rash opened this issue Sep 4, 2019 · 16 comments · Fixed by #188
Assignees

Comments

@andy-rash
Copy link

andy-rash commented Sep 4, 2019

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[x] Support request
[ ] Other... Please describe:

Current behavior

When trying to use createRoutingFactory, I get the issue seen here.

Expected behavior

createRoutingFactory should automatically handle mocking the Router, but it appears that it isn't. I feel like I'm probably doing something wrong here.

Minimal reproduction of the problem with instructions

For bug reports please provide the STEPS TO REPRODUCE and if possible a MINIMAL DEMO of the problem via
https://stackblitz.com or similar (you can use this template as a starting point: https://bit.ly/2zme3bj).

Constructor for component under test (TimeEntryPage):

    constructor(
        private controlNumberService: ControlNumbersService,
        private fb: FormBuilder,
        private modalController: ModalController,
        private route: ActivatedRoute,
        private router: Router,
        private timeout: TimeoutService
    )

where ControlNumberService and TimeoutService are custom services, ModalController is an Ionic service, and FormBuilder, ActivatedRoute, and Router are Angular services.

Component test:

describe(`TimeEntryPage`, () => {
    const controlNumbersService: StubControlNumbersService = new StubControlNumbersService();
    let router: SpyObject<Router>;
    let spectator: Spectator<TimeEntryPage>;

    const createComponent = createRoutingFactory({
        component: TimeEntryPage,
        params: { controlNumber: '23836110' },
        mocks: [ModalController, TimeoutService],
        providers: [
            {
                provide: ControlNumbersService,
                useValue: controlNumbersService
            },
            FormBuilder
        ],
        imports: [IonicModule, ReactiveFormsModule, SharedModule],
        schemas: [CUSTOM_ELEMENTS_SCHEMA]
    });

    afterEach(() => {
        spectator.debugElement.nativeElement.remove();
    });

    beforeEach(() => {
        route.testParams = { controlNumber: '23836110' };

        spectator = createComponent({ detectChanges: false });

        router = spectator.get(Router);
    });

    it(`should instantiate`, () => {
        expect(spectator.component).toBeDefined();
    });
});

What is the motivation / use case for changing the behavior?

N/A

Environment


Angular version: 8.3.2 (latest)


Browser:
- [x] Chrome (desktop) version 76.0.3809.132
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: v11.15.0  
- Platform: macOS 10.14.6 

Others:


- Jasmine + Karma (built-in with Angular CLI project)

@dirkluijk
Copy link
Collaborator

For what I can tell you, createRoutingFactory does not provide a Location as the RoutingTestModule would do. Maybe we can provide a mock for this, I will investigate this for you.

Do you use a Location in your component?

In the mean time, I think that adding mocks: [Location] to your factory would fix the issue. Please let me know if that helps.

@andy-rash
Copy link
Author

To answer your question, I don't use Location in the component. Before createRoutingFactory was added in v4, I used createTestComponentFactory, and I had run into the same problem by adding Router to mocks instead of adding RouterTestingModule to imports. Using RouterTestingModule usually fixed this problem. However, I should note that I found that this was only a problem in components that used both Router and ActivatedRoute. I have some components that just use Router, and adding Router to mocks in those tests is all that is needed--no need for RouterTestingModule.

After adding Location to the mocks array, I get another error: NullInjectorError: No provider for UrlSerializer!. I tried importing UrlSerializer from @angular/router and adding it to mocks, but this causes the build to fail since UrlSerializer is an abstract class. After adding it under providers, the app builds, but then the test fails like so. This also happens if I simply add RouterTestingModule to imports instead of messing with Location and UrlSerializer.

How this fails is localized to NavController from Ionic, which appears to rely on having a fully fleshed out Router instance with an events Observable. See the failing line here.

As a workaround, I've found that I can use createComponentFactory like so:

    const controlNumbersService: StubControlNumbersService = new StubControlNumbersService();
    const route: StubActivatedRoute = new StubActivatedRoute();
    let router: SpyObject<Router>;
    let spectator: Spectator<TimeEntryPage>;

    const createComponent = createComponentFactory({
        component: TimeEntryPage,
        mocks: [ModalController, TimeoutService],
        providers: [
            {
                provide: ActivatedRoute,
                useValue: route
            },
            {
                provide: ControlNumbersService,
                useValue: controlNumbersService
            },
            FormBuilder
        ],
        imports: [
            IonicModule,
            ReactiveFormsModule,
            RouterTestingModule,
            SharedModule
        ],
        schemas: [CUSTOM_ELEMENTS_SCHEMA]
    });

    beforeEach(() => {
        route.testParams = { controlNumber: '23836110' };

        spectator = createComponent({ detectChanges: false });

        router = spectator.get(Router);
    });

where StubActivatedRoute is a slightly modified version of what you posted here.

At the end of the day, the workaround is fine, but it'd be really cool if I could just use the built in features instead of maintaining stubs for third-party things (i.e. things that I might need to share in other projects). Like I said above, I feel like maybe I'm just doing something wrong here, but let me know what you think.

Also, thanks so much for your help! This is really an awesome project, and I'm super grateful for all the effort that you, @NetanelBasal , and the others have put into this.

@dirkluijk
Copy link
Collaborator

dirkluijk commented Sep 6, 2019

We do expose our ActivatedRouteStub in @ngneat/spectator, so you can use that one as well. But the thing is: createRoutingFactory = createComponentFactory + ActivatedRouteStub.

So, why not use createRoutingFactory with Router added to mocks but set the router.events to satisfy NavController, or is that hard to do?

Or what about mocking NavController as well?

@andy-rash
Copy link
Author

It's great that I can use that stub as a standalone, if need be, but I'd really like to just be able to use it (more or less) like I see here.

So, why not use createRoutingFactory with Router added to mocks but set the router.events to satisfy NavController, or is that hard to do?

Or what about mocking NavController as well?

Looking at the source for NavController in Ionic as well as Router in Angular, I think I would have to go down a rabbit hole of mocking , which (I think) is inadvisable when I have the workaround.

Like I said in my last comment, it'd be really cool if I could just use the built in features instead of maintaining stubs for third-party things, especially if I'm never even directly manipulating those things. Since these things are so integral to almost any project (ActivatedRoute for Angular projects in general and NavController for Ionic projects), it would be inevitable that these mocks would need to be shared and maintained between many different projects.


While trying to figure this all out, I was looking at the implementation around createRoutingFactory, and I noticed that Router gets mocked by default whenever you use createRoutingFactory. Is there a reason this was chosen instead of adding RouterTestingModule to the Spectator's imports? I suspect this might solve my problem, but I'm not sure if it would cause problems elsewhere that I'm not aware of.

Please excuse me if I'm coming across presumptuous at all here. I'm certainly no expert when it comes to Angular testing, so I appreciate the help.

@NetanelBasal
Copy link
Member

@andy-rash, @dirkluijk will investigate. Don't worry. You're in good hands :)

@dirkluijk
Copy link
Collaborator

Currently on vacation but yeah, I will try to investigate asap!

@NetanelBasal
Copy link
Member

Have fun! 🌴

@NetanelBasal
Copy link
Member

@andy-rash can you somehow create a small reproduction so we can test it on our side, please?

@andy-rash
Copy link
Author

@NetanelBasal Sure thing. This should do it.

N.B. it is an Ionic project, but that shouldn't matter for running the tests since it still uses Karma + Jasmine

@dirkluijk
Copy link
Collaborator

dirkluijk commented Sep 18, 2019

Excuse me for the late reaction. I have been on vacation, but this gave me a lot of time to think about this in the back of my head! 😁 🚀

Cause of the issue

The cause of the issue is quite simple: instead of having a real Router/Location implementation (for example, from RouterTestingModule), we use mocks. And a mocked Router does not have a router.events property.

Why no RouterTestingModule?

The answer is simple: SpectatorRouting was designed to test Routed Components, not Routing itself.

Importing RouterTestModule enables you to setup fully-functional Angular Routing and make integration tests. Example

However, in order to test the interaction of routed components with ActivatedRoute, we use our own nice stub, ActivatedRouteStub. It seems that RouterTestingModule and our ActivatedRouteStub do not work well together. That makes sense, as our own stub has a quite different implementation. I have not been able to make them compatible yet...

In other words...

For now:

  • You should use Spectator with RouterTestingModule if you want to work with real Router/Location implementations. The only way to change route params is by navigating to a route (Router.navigate()).
  • You should use SpectatorRouting (with the ActivatedRouteStub loaded) for testing routed components. The recommended way to change route params is by using shortcuts like Spectator.setRouteParam()

A solution

There are multiple solutions:

  1. Do nothing and explain this better in the docs
  2. Make a config option "useStubs" (which defaults to true)
    1. If true: it uses ActivatedRouterStub with mocks for Router/RouterLink
    2. If false: it loads RouterTestingModule instead
  3. Improve the mocked Router so that it also has router.events property. We could even add a shortcut like Spectator.triggerRouterEvent().

I would recommend to go for both option 2 and 3.

@NetanelBasal
Copy link
Member

I would go with both 2 and 3. We must add the triggerRouterEvent as the developers will need to test the registered callback.

@dirkluijk
Copy link
Collaborator

I will submit a PR sometime tomorrow.

@dirkluijk
Copy link
Collaborator

I checked the PR in the example repo, and with stubsEnabled: false it seems to work now. Let's merge and release it.

@NetanelBasal
Copy link
Member

@andy-rash are you satisfied with the solution?

@andy-rash
Copy link
Author

@NetanelBasal - very much so! Thank you and @dirkluijk for your help! You guys really are amazing examples when it comes open source project management. Keep up the good work!

@dirkluijk
Copy link
Collaborator

Glad to hear!! 🚀

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

Successfully merging a pull request may close this issue.

3 participants