Angular-cli: Style guides conflict with stubbing angular components in tests causing unavoidable lint errors

[ajspera]

Bug Report or Feature Request (mark with an x)

- [x] bug report -> please search issues before submitting
- [ ] feature request

Area

- [x] devkit
- [ ] schematics

Versions

Angular CLI: 6.0.8
Node: 8.11.2

Repro steps

• Create an angular project
• add to the app component test a stub for router-outlet as described in the stubbing unneeded components section of the testing docs

@Component( {selector: 'router-outlet', template: ''} )
class RouterOutletStubComponent {}

• runng lint

The log given by the failure

ERROR: /home/alex/Repos/linterr/src/app/app.component.spec.ts[3, 23]: The selector of the component "RouterOutletStubComponent" should have prefix "app" (https://angular.io/styleguide#style-02-07)

Desired functionality

Style guides about selectors should not be applied to mocked or stubbed components.

[CharlesSuttie]

Hi @ajspera

The router outlet selector seems to be the odd one out here. It doesn't have a prefix, but the other components in the testing docs do - 'app'. As you point out, when you write the stub component for the router outlet, the selector needs to be 'router-outlet'. Otherwise it won't match the selector in the template under test:

<app-banner></app-banner>
<app-welcome></app-welcome>
<nav>
  <a routerLink="/dashboard">Dashboard</a>
  <a routerLink="/heroes">Heroes</a>
  <a routerLink="/about">About</a>
</nav>
<router-outlet></router-outlet>

I wonder whether an exception to the linting rule could be made just for the 'router-outlet' selector. I'm not sure whether there are other built in selectors like 'router-outlet' without a prefix.

On the other hand:

• if the stub component template must always match the original component (the one being stubbed) then do we need this linting rule to remind us twice? (once for the original and again for the test)

• should this linting rule apply to any components created in 'spec' files?

cheers,
Charlie

[ajspera]

@CharlesSuttie good points. Since no components that don't exist would ever be defined in a spec file, it seems the rule could just be omitted for all spec files as a simple and sensible fix. Even in the case of a valid linting error, it should be enough to notify about the defining file rather than listing all the stubs in spec files. Those will trigger testing errors anyway once the violating component or directive is fixed. I'm willing to help out with this issue, but the first question is does this belong here?

I temporarily solved this by commenting out the rule in the tslint.json generated by the cli which brought me here, but I'm not entirely sure where the actual source is whether it's here, tslint, codelyzer, or somehwere else.

[alan-agius4]

When running lint there is not distinction if it's a spec file or not.

If you want to disable a rule in a file I suggest you use on of the disable flags. https://palantir.github.io/tslint/usage/rule-flags/

it should be enough to notify about the defining file rather than listing all the stubs in spec files

Listing all errors, is the correct behaviour, For every lint error, one should get the line number and column and a detailed explain of the error, this is the norm in all the linters I'd say. At least I never saw a linter that does something else.

[filipesilva]

I think for this case the disable flag is the right approach. Specs often have dubious usage of APIs and need some disabling tslint on certain lines.

In the given example one is really stubbing the angular router outlet so I think that's a fair tslint error. Disabling it would be:

// tslint:disable-next-line:component-selector
@Component( {selector: 'router-outlet', template: ''} )
class RouterOutletStubComponent {}

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{_This action has been performed automatically by a bot._}