tests: dynamically import all modules when using mocks

[adamraine]

Alternative to #14216. Instead of moving all our mocks to a separate file, we can err on the side of dynamic imports for everything that might depend on our mocked modules.

Basing this off of #14202 to see if this fixes the unit test issues there.

[connorjclark]

Can this just dynamic import the module normally now? (no query param)

[adamraine]

Tests still fail. I tried moving this dynamic before asset saver is mocked but one of the tests still fails. I think we may need to keep it for the time being.

[connorjclark]

In general my hesitation here was rooted in an ideal that one day, we would be able to go back to the idealized jest mock setup where mocks are 1) within the same test file and 2) get hoisted up above the es module imports .... somehow. It's unclear if (2) will ever materialize, but if it did, we would be able to just paste the mock code into the test file, and rely on the magic hoisting to work it out–and get back to test code similar to how jest mocking commonjs modules looked.

But there's no reason to think that'll ever be possible, at least not without some heavy jest-like machinery that pre-processes test files. So this proposal has the benefit of being in a reasonable end-state, unlike the other one (separate mock files aren't perfect, but the hope was one day the transition would be trivial).

I think the approach here can be:

If a test file mocks a module, it should first statically import test modules; then configure the mocks; then dynamically import all non-test modules.

We shouldn't attempt to dynamically import just the set of modules necessary, because when a module is mocked that happens to be a transitive dependency of a module statically imported that you thought wasn't related to the module being mocked, the errors that occur are incredibly hard to debug. So let's just dynamic import all the (non-test) things.

Can we have a singular comment before each block of dynamic modules that refers the reader to a longer explanation in a markdown file?

[iambumblehead]

politely, I wonder if this package I published might be useful here https://www.npmjs.com/package/esmock

[adamraine]

@iambumblehead The fundamental problem this PR is trying to solve is that mocks cannot be applied to anything in the static imports, because static imports will always be resolved before the mock is applied: https://jestjs.io/docs/ecmascript-modules#module-mocking-in-esm

Ideally our mocks would be hoisted before the static imports like jest does with cjs tests, but it doesn't look like that's possible with esmock.

[iambumblehead]

static imports will always be resolved before the mock is applied

@adamraine esmock returns a unique import tree definition, separate from other import trees, regardless of when they are imported. Therefore, the problem resolved here would not exist if esmock were used (I think, maybe I am misunderstanding something).

[connorjclark]

This issue contains an example of the problem we face:

testdouble/testdouble.js#492

[iambumblehead]

esmock would not cause this test to fail. esmock would not affect the import tree at '../lib/lh-error.js'

import assert from 'assert';
import 'quibble';
import {LighthouseError} from '../lib/lh-error.js';

assert( (await import('../lib/lh-error.js')).LighthouseError === LighthouseError );

[adamraine]

@adamraine esmock returns a unique import tree definition, separate from other import trees, regardless of when they are imported. Therefore, the problem resolved here would not exist if esmock were used (I think, maybe I am misunderstanding something).

Unfortunately, our tests were designed with jest, jest mocking, and cjs in mind. The tests expect mocks to be applied globally in isolated test enironments. Modules we mock repeatedly have there own separate mocking functions (example). We could redesign our mocking patterns so we can use esmock, but it's not a drop in replacement for testdouble.

Some tests would be easy to convert to esmock:
https://github.com/GoogleChrome/lighthouse/blob/main/core/test/user-flow-test.js

Others are more complicated because more than 1 imported module depend on mocked modules:
https://github.com/GoogleChrome/lighthouse/blob/main/core/test/runner-test.js
https://github.com/GoogleChrome/lighthouse/blob/main/core/test/legacy/gather/gather-runner-test.js

That being said, our tests may benefit from using the locally contained tree provided by esmock, and the ts chaining support would let use use mocks in our flow report tests. It's worth considering for the future IMO, but would be non-trivial to adopt.