Using `import:` to mock globals messes up coverage (with c8)

[bosschaert]

If I use import: in esmock to mock a global it messes up the coverage report. At least it does when run with c8 mocha. I constructed a minimal test scenario for it below:

Here's an example source file src/myfile.js:

function someFun() {
  console.log('something');
}

export async function testFun() {
  console.log('test');

  return 418;
}

And here's an example test for it. It only tests the testFun method, but it has a mock defined for the global fetch (which isn't actually used):

import assert from 'assert';
import esmock from 'esmock';

describe('ESMock test case', () => {
  it('Simple test', async () => {

    const { testFun } = await esmock(
      '../src/myfile.js', {
        import: { fetch: async (url) => {} }
    });

    assert.strictEqual(418, await testFun());
  })
});

When I run this and look at the coverage report it looks like this:

The above coverage report is clearly wrong.

If I comment out the line with the import, e.g.:

    const { testFun } = await esmock(
      '../src/myfile.js', {
        // import: { fetch: async (url) => {} }
    });

the coverage report is correct again:

Note that using esmock with things other than globals, doesn't have this issue, e.g. the following doesn't cause this issue:

    const { postSource } = await esmock(
      '../../src/source.js', {
        '../../src/included.js': {
          default: mockResp
      }
    });

It would be great if I can use the globals mocking without messing up the coverage report 😄

[iambumblehead]

if there's no objection, I'd like to close this issue to merge with an earlier similar issue https://github.com/iambumblehead/esmock/issues/271 (will wait for reply)

There is a long-outstanding issue at the c8 repo for this https://github.com/bcoe/c8/issues/325

The maintainer requested a reproduction and I submitted one, but there was no follow-up https://github.com/bcoe/c8/pull/501 possibly the solution would need to come in multiple separated PRs

[iambumblehead]

I'm surprised the coverage report is correct when modules and not globals are mocked

[bosschaert]

I have no problem with merging this with #271 but it would be nice to describe this import: related problem there then too. I did read that issue before reporting this one and thought it was about a different problem.

To me it sounds like that these are 2 different problems that might have the same underlying cause?

[iambumblehead]

If you recommend any changes the title or anything else around that ticket they can be made.

To me it sounds like that these are 2 different problems that might have the same underlying cause?

Okay, keep this issue open

For mocking global values, esmock returns a modified version of source file(s) to node's loader; where the global value being mocked is defined at the top of the file. Maybe that change would need to be coordinated with the coverage tool in some way, but to be honest I don't know how to solve the issue.

const {fetch} = global.esmockCacheGet("treeid5"); // <- esmock adds this
// rest of file

Any ideas for a solution?

[bosschaert]

Any ideas for a solution?

One thing I've been doing as an alternative way to mock these globals is to use globalThis.

For example something like this if I want to replace the global fetch:

    const mockFetch = async (url) => { ... }; // My mock fetch function

    const savedFetch = globalThis.fetch; // Save the original global fetch
    try {
      globalThis.fetch = mockFetch; // replace the global fetch with my mockFetch

      // Now call my function that calls fetch under the covers
      // it will use the mockFetch
      assert.strictEqual(418, await testFun());
    } finally {
      globalThis.fetch = savedFetch; // put the original global fetch back in the finally block
    }

You can make these changes to globalThis after the source file being tested is loaded, so you could put something like this in the logic of await esmock(...) call without the need to alter the original source file. The only thing that needs to be done extra is to reset the original values, which I do in a finally block. Maybe an extra call something like esmock.resetGlobals() could do this, but there might be a smarter way to do that somehow.

[iambumblehead]

So it seems supporting globalThis and esmock.resetGlobals() would require esmock to lose its ability to be used concurrently OR esmock would need to provide an extra interface and logic that conditionally use globalThis and implicitly requires sequential usage from the user.

Maybe support for a second "import"-like namespace, such as "globalThis", could be added in order to use globalThis for mocking.

One of the main benefits of esmock imo is that it can be used with concurrent tests and so using globalThis is a downgrade in that regard.

If you are around feel free to chime in @koshic

Also "setting" and "clearing" properties of globalThis could be handled with a seperate file or package and does not actually need to be handled by esmock. However, for users who would want to use globalThis, it would be convenient if esmock could help with that.

[iambumblehead]

I wish there were a way to avoid using globalThis and to get correct results from c8 here but don't know if, or how, it could be achieved.

[iambumblehead]

node.js' native coverage tooling now allows one to specify files for coverage analysis https://github.com/nodejs/node/pull/53553

Have not tried it myself yet, but perhaps native coverage results do not exhibit this same issue

[iambumblehead]

~another we could try doing is; putting the lines added by esmock onto the existing first line of the file with no additional newlines.~ esmock is already doing this

[iambumblehead]

this issue also happens using node's native test coverage tools.

another idea is to add 'global' definitions to the bottom of the file using "var" so to make the definitions available at the top of the file

[koshic]

@iambumblehead can /* c8 ignore start */ / /* c8 ignore end */ around injected code help us for c8 use case? It's a question (may be a bit stupid...), I didn't try it yet )

[iambumblehead]

I experimented with adding using var but the definitions are not "hoisted" the way that I expected.

This results in a runtime error where dateWrapper becomes undefined

const dateWrapper = Date

export {
  dateWrapper
}

var Date = { now: () => 'now' }

changing 'var' for const or let results in ReferenceError [Error]: Cannot access 'Date' before initialization

[iambumblehead]

@koshic thanks for the suggestion that might be something to try. wondering if there is something similar for native node test coverage tools.

[iambumblehead]

ime usage of c8 ignore removes sections of code from being included in the report, but it does not elide those lines from the analysis in a way that would shift the lines being reported

[iambumblehead]

I also tried replacing "const" with " var" to try and get the hoisting behaviour... but var-defined dateWrapper is still 'undefined'

sourcesafe.replace(/const/g, '  var')

I'm all out of ideas

[iambumblehead]

well, node runs all tests now run separated threads with own global environment and maybe the globals could be defined there without side-effects that would have occurred in earlier versions

[koshic]

Technically we can fix source map for the particular module via inline directive. If tested module is a 'pure JS' (== loaded without loader hooks) source map is not exists so coverage tools will use original source, right (we are testing source code, not compiled packages with corresponding .map files)? If tested module was transformed via hooks, previous hook may (should for more or less good tools like tsx) provide source map in separate field -> we can fix it as we want.

[koshic]

node runs all tests now run separated threads

it's not quite true due to https://nodejs.org/docs/latest/api/cli.html#--experimental-test-isolationmode

[iambumblehead]

Technically we can fix source map for the particular module via inline directive. If tested module is a 'pure JS' (== loaded without loader hooks) source map is not exists so coverage tools will use original source, right (we are testing source code, not compiled packages with corresponding .map files)? If tested module was transformed via hooks, previous hook may (should for more or less good tools like tsx) provide source map in separate field -> we can fix it as we want.

This is a really interesting idea. Re-stating this idea, we would include our own source-map directive which references our own modified version of the file, possibly an updated version of a source-map from the previous hook applied to the file.

Thinking about this

[iambumblehead]

const dateWrapper = Date

export {
  dateWrapper
}

await (async () => (await import('./globalsredefine.js')).setItBack())()
export * from './globalsredefine.js'

globalsredefine.js

var originalDate = Date

globalThis.Date = { now: () => 'now' }

const setItBack = () => {
  globalThis.Date = originalDate
}

export {
  setItBack
}

test (passing)

test('should use mocked global', async () => {
  const hostObjects = await import('../local/usesImportObjects.js')

  assert.strictEqual(hostObjects.dateWrapper.now(), 'now')
  assert.strictEqual(typeof Date.now(), 'number')

maybe there is some potential here,

• esmock could define the 'globalsredefine.js' module,
• original parts of the file used for coverage analysis are un-modified --lines are only added to the bottom of the file
• esm loads the imported file first, even though the import is defined at the end of the file

but this approach mutates globalThis and not sure what the best way to "unset" that would be and overall is not entirely different from the earlier suggestion https://github.com/iambumblehead/esmock/issues/296#issuecomment-2043599080

[iambumblehead]

elaborating on that idea, its possible to get the calling files from the stack trace

at TestContext.<anonymous> (file:///Users/bumble/test.global.test.js:18:46)',

and so another thing that could be done is, check the stack trace to see if the originating call should have the default global value behaviour or the mocked behaviour

globalThis.Date = {
  now: function (...args) {
    const mocksAvailable = mocksForThisCaller((new Error).stack)
    return mocksAvailable
      ? mocksAvailable['now'](...args)
      : originalDate.now(...args)
  }
}

[iambumblehead]

we could also possibly use querystring in a clever way and attach some reset behaviour to esmock.purge(loadConfig)

[iambumblehead]

another very hacky idea is to replace the references to global variables with function calls that use the exact same number of characters to return a mocked version of the thing, something like this

-const dateWrapper = Date
-const setTimeoutWrapper = setTimeout
-const fetchWrapper = fetch
-const reqUsers = async url => fetch(url)
+const dateWrapper = ZZ()
+const setTimeoutWrapper = ZZZZZZZZ()
+const fetchWrapper = ZZC()
+const reqUsers = async url => ZZC()(url)

export {
  dateWrapper,
  setTimeoutWrapper,
  fetchWrapper,
  reqUsers as default,
  child
}

export * from './globalsredefine.js'