It's actually a bit more complicated to repro. Running the file directly is fine, but with mocha involved I get the error:

// simple-test.js
import assert from 'assert';
import 'quibble'; // no failure if commented out
import {LighthouseError} from '../lib/lh-error.js';

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

// mocha-setup.js
import * as td from 'testdouble';

td.replaceEsm('./empty-module.js', undefined);

npx mocha --require=mocha-setup.js --loader=testdouble simple-test.js

Currently unsure at the moment how important of a role mocha plays, trying to simplify further.

I think this boils down to the same problem. This file repros with just node --loader=testdouble simple-test.js, no mocha:

import assert from 'assert';
import * as td from 'testdouble';
import {LighthouseError} from '../lib/lh-error.js';

td.replaceEsm('./empty-module.js', undefined);

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

I'm aware this is a little convoluted, given there's no reason to dynamically import a module you already have a static import for. The motivation for this pattern is that it's required to defer the imports of some modules if they import a module you td.replaceEsm in a test file - otherwise, they will import with a reference to the old module that can't be undone. Here's an example:

https://github.com/GoogleChrome/lighthouse/blob/a8b864fff1fe2b73800a60c9b369164d8a7eda31/lighthouse-core/test/runner-test.js#L30-L65

But given the above problem, which may be unfixable, perhaps a better solution is for test files that want to 1) be written in ES modules format and 2) replace ES modules – should be first wrapped in their own "es module setup wrapper" (ex" simple-test.td.js) that first calls testdouble on w/e, and then imports the actual test file. This feels hacky, but may be the best bet. Jest has this same issue BTW, it's inherent to mocking ES modules - you can't do anything before static imports, so mocking their dependencies is a burdensome exercise of dynamic import hacks.

Hey @connorjclark I've appreciated your analysis and commentary into this and am somewhere between "I wonder if @giltayar's replaceEsm implementation could be tweaked" and the "this is probably intractable given how static imports work". The idea of a setup wrapper is interesting, too.

I was excited to see Lighthouse uses testdouble, so I'd love for you all to be able to keep using it. How would you like to proceed here?

I sent a PR to fix the main issue discussed here (the module cache being broken unexpectedly).

The other issue I mentioned (re: mocking be hard to manage in es modules) I've been experimenting with and think I've found a good way to "wrap" a test file in mocha with a module that just sets up the mocks–and then only loads the test file when mocha starts to actually run that test suite. It was a bit tricky due to how the Mocha lifecycle is setup... once I vet it a bit more I'd be happy to open a docs PR here to explain it a bit and link to an example of it. Quick peek:

GoogleChrome/lighthouse#14182 (comment)

I was excited to see Lighthouse uses testdouble, so I'd love for you all to be able to keep using it.

It's been invaluable for our transition from Jest -> Mocha (due to limitations in v8 es module support, but also overall we wanted a less "magic" test runner). Thanks for providing the library! Happy to help smooth out the rough edges re: es module support.

Merged your quibble PR as 0.6.10: testdouble/quibble#72

This caused a regression in testdouble.js' test suite: #493

Could you please take a look?

You're right! Once you replaceEsm then all imports are "new" ones and you don't get the old module, even if you didn't mock it!

This is because the code lazy! It upgrades the "generation" of all modules and not just the modules being mocked. This can be fixed by defining the generation to be per module.

Starting to work on it!

Small quibble (pun intended 😉): td.replaceEsm is async, so in your repro, it should be:

await td.replaceEsm('./empty-module.js', undefined);

Small quibble (pun intended 😉): td.replaceEsm is async, so in your repro, it should be:

Yup yup... I found that out the hard way while debugging yesterday! Forgot to update the example here.

This can be fixed by defining the generation to be per module.

This definitely resolves this specific issue, but I'd expect more issues to crop up as described in #439. Wondering what you think of the problem laid out there, and my "strict" proposal: #493 (comment)

@connorjclark before discussing the issues you raised, I tried replicating your problem in a test, and couldn't reproduce it. This is because when resolving a module that is not mocked (e.g. ../lib/lh-error.js), it will not add the quibble query parameter to the resolved url, and so will always use the cached version!

So... are you sure that simple example works? Can you give me a simple repro that absolutely works in a separate repo?

That's because of the PR I added recently fixes the exact issue given as a repro here. But as #493 explains that approach is flawed. And it fails many testdouble tests.

https://github.com/testdouble/quibble/blame/3db427e3ebf6464a0d48fab3f8b1a569243b5904/lib/quibble.mjs#L32

Oh! That part was yours! I was wondering how it happened that in some cases resolve() gets called multiple times in the same function.:-) OK, looking at it again and trying to figure it out.

This issue caused problems for me too. When I found the quote below I was able to solve it.

You're right! Once you replaceEsm then all imports are "new" ones and you don't get the old module, even if you didn't mock it!

I mocked the module that caused comparison problems with itself, and that seems to ensure the same instance in all tests. I.e.

import * as mcm from 'my-central-module'
import { replaceEsm, reset } from 'testdouble'

before(async () => {
  await replaceEsm('my-central-module', mcm)
  await replaceEsm('something-to-mock', ...)  
  
  ...
})

Not superhappy with the workaround but tests pass for now.