Engine unit test is too verbose

[pmdartus]

Description

Since the merge of the snabdom refactor (#47), the engine test became extremely verbose. It produces more than 10k lines of lines of stack traces. All those logs is a real block for running the engine tests in watch mode

One of the big offenders is that the engine checks if the same tag name is used by multiple components: registerComponent

[caridy]

Yeah, we need to spend time fixing all tests to either a) slice tests more so is easy to manage different tagNames in the same test, or b) use different tagName for each tests in every test file.

Part of that work is also to do the following for each tagName:

interface HTMLElementTagNameMap {
   'x-foo1': MyComponent1
}

which indicates to the type system that creation of such tags will be bound to that class definition, and this will remove many type errors in tests.

[pmdartus]

What about disabling the check in test mode ?

[caridy]

@pmdartus do we have such mode?

[pmdartus]

Jest sets by default process.env.NODE_ENV to `test: jest-runtime

We can rewrite registerComponent to:

export function registerComponent(tagName: string, Ctor: ComponentConstructor) {
    if (!isUndefined(TagNameToCtor[tagName])) {
        if (TagNameToCtor[tagName] === Ctor) {
            return;
        } else if (process.env.NODE_ENV !== 'production' && process.env.NODE_ENV !== 'test') {
            // TODO: eventually we should throw, this is only needed for the tests today
            assert.logWarning(`Different component class cannot be registered to the same tagName="${tagName}".`);
        }
    }
    TagNameToCtor[tagName] = Ctor;
}

[davidturissini]

@pmdartus I thought that we talked about having multiple modes and it was a no-go because it would get complicated really quickly. I prefer the other approach of not recycling tag names because that matches more closely how LWC will be used, especially when customElements.register is used under the hood.

[pmdartus]

Then going down this route we need to be more restrictive and throw if 2 tests are using the same name. A warning is not enough the prevent new test to reuse existing names.

My main concern is that we will see an explosion of components with convoluted names, and I don't think it will be maintainable in the long run.

That being said we still have a couple of options to avoid adding another mode:

• call jest.resetModules between each test to make sure we start again with a clean slate. It's something that I use in the performance timing tests to ensure to reset the uid between each test. One of the unpleasant side-effect of this approach is that it will slow down the test suite because it forces node re-evaluating the modules.
• expose an internal flushRegistry method on def.ts and call it between each test. This method will get tree-shacked when bundling the engine.

After listing the different options we have, I still believe that using process.env.NODE_ENV is the simplest and most reliable approach. But it's my 2 cents comment.

[davidturissini]

@pmdartus Could we run the tests in production mode as opposed to dev mode?

[pmdartus]

Today the tests run in test mode. We don't do any special handling for this mode since we only check if production.

I think it's useful to run the tests in development to validate the assertions and warnings we put in place for development mode. But maybe it worth investigating running the tests also in production mode.

---

Edit:

React runs 4 suites of tests:

• Test from source in development mode
• Test from source in prod mode
• Test from build artifact (react.js) in development mode
• Test from build artifact (react.js) in prod mode

[pmdartus]

Here is branch with all the verbose unit test skipped: https://github.com/salesforce/lwc/compare/pmdartus/localize-all-warnings

That would be useful for the willing to tackle this issue.