angular: Unit test should fail if an element is not known

🚀 Unit test should fail if an element is not known

Relevant Package

This feature request is for @angular/core. And I think this is a kinda a regression since this behavior changed with Ivy.

Description

When executing unit tests with Karma and my test module has not been setup correctly, to contain all the relevant components, I get a warning like this:

WARN: ''app-my-element' is not a known element:
1. If 'app-my-element' is an Angular component, then verify that it is part of this module.
2. If 'app-my-element' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.'

The test is still successful in that case. In my opinion it should fail, since the test module clearly is missing this component.

I saw that there have been efforts to make it appear as an error: 00f3c58. This has been reverted in this commit: 00f3c58.

But that wouldn’t change the fact, that the test still passes.

Describe the solution you’d like

I would like one of these options:

  • tests with unknown elements fail (as they did pre Ivy)
  • an option to enable this behavior to make the tests fail
    • i.e. a CLI argument or a property in angular.json

If the default is “fail”, then a user could add CUSTOM_ELEMENTS_SCHEMA or NO_ERRORS_SCHEMA to their test module if they want to suppress the errors.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 57
  • Comments: 27 (8 by maintainers)

Commits related to this issue

Most upvoted comments

We landed two PRs that introduce new options for the TestBed in Angular v14 and that should fix this issue.

You can now enable errorOnUnknownElements and/or errorOnUnknownProperties when setting up the TestBed globally or in a specific test to throw an error on unknown elements or unknown properties encountered in templates (instead of just logging the NG303/NG304 error).

These new options are opt-in for now.

If you’re using the CLI, you can enable them globally in test.ts

getTestBed().initTestEnvironment(
  BrowserDynamicTestingModule,
  platformBrowserDynamicTesting(),
  { 
    errorOnUnknownElements: true,
    errorOnUnknownProperties: true
  }
);

Or you can enable/disable them in a specific test:

TestBed.configureTestingModule({
  declarations: [AppComponent],
  errorOnUnknownElements: true, 
  errorOnUnknownProperties: false
})

See https://next.angular.io/api/core/testing/TestModuleMetadata

For a workaround, I have added the code below to the end of our src/test.ts file.

// https://github.com/angular/angular/issues/36430
// eslint-disable-next-line @typescript-eslint/no-explicit-any
console.error = (data: any) => fail(data);

Here is mine, somewhere in a file in testing utils:

const orgConsoleError = window.console.error;

export function patchConsoleToFailOnError() {
    window.console.error = function (...args: any[]) {
        orgConsoleError.apply(this, args);
        try { throw new Error('console.error'); } catch (err) { console.info('console.error', args, err); }
        fail('console.error was called, this is not allowed in a unit test run');
    };
}

And in test.ts:

beforeEach(() => patchConsoleToFailOnError());

So far seems to work nicely

In case anybody is interested, I have the following workaround in place until this is fixed:

Executing the following function before creating the component with TestBed.createComponent() will ensure tests fail if there are unknown elements.

Since we use a util function in every component test to setup our test bed, it was easy to add there.

/**
 * Monkey patch console warn and error to fail if a test makes calls to console.warn or console.error.
 */
function patchConsole(): void {
    console.warn = function(message?: any, ...optionalParams: any[]): void {
        const params = optionalParams ? `\nParams: ${optionalParams}` : '';
        fail(`Test contained console warning:\n${message}${params}`);
    };
    console.error = function(message?: any, ...optionalParams: any[]): void {
        const params = optionalParams ? `\nParams: ${optionalParams}` : '';
        fail(`Test contained console error:\n${message}${params}`);
    };
}

We originally moved away from throwing an error because Ivy’s direct property introspection is stricter than View Engine’s old property validation strategy using element schemas. Throwing causes issues if you bind to a property that is supported in one browser, but not another. In VE, the browser that didn’t have support would just not have the binding so it would still work, but in Ivy the browser that didn’t have support would throw. Hence, logging an error as a compromise to avoid a breaking change.

All that said, I agree that logging is not sufficient when it comes to tests. It’s super confusing that your tests would pass with a logged error. We should probably re-evaluate whether we should throw in that case. I think it makes more sense to have a breaking change for apps that bind to properties that only work in some browsers than to have everyone’s tests potentially affected.

We think that this could be changed to throw an error now, but it would be a breaking change that could land in 14.0.0 at the earliest. For now we will put it in the backlog, and run a test in Google to see how breaking it would be. If it turns out this is a big effort to merge into Google, when we will need to prioritize it as a project.

It would be nice if this feature could be a flag option to fail the test run upon any console.error. This way compatibility wouldn’t be hurt.

Is there any update on this? I’m experiencing the same issue, running jest tests in an NX monorepo. On our CI, I’ve got a bunch of errors logged out about missing components, but locally, I don’t get anything. Even if I run it on a completely clean state, like @inorganik mentioned, I still don’t get the errors locally. Is there a workaround for this maybe?