iambumblehead / esmock

ESM import and globals mocking for unit tests
ISC License
198 stars 18 forks source link

Question / Bug - Unable to mock commonjs dependencies #312

Open Filipoliko opened 1 month ago

Filipoliko commented 1 month ago

Hi,

thank you for this great library, thanks to this, we were able to actually use native node test runner in some of our projects!

I ran into an issue, but maybe it is expected behaviour. When I am trying to mock a dependency of a dependency, it does not really work. Maybe the issue is that both dependencies are commonjs modules?

I prepared a reproduction repository - https://github.com/Filipoliko/esmock-bug - but the use-case is quite straightforward. Ultimately, I am trying to mock fs module globally (even within dependencies like changelog-parser), but I am failing to do so. Being desperate, I tried to mock line-reader module, which is the one using the fs module in my case, but this failed also. This is a simplified example, where I was unable to mock the 3rd party dependency of a dependency.

Code

import { test, mock } from 'node:test';
import assert from 'node:assert';

import esmock from 'esmock';

test('parseChangelog', async () => {
    const content = 'content';
    const parseChangelog = await esmock('changelog-parser', {}, {
        fs: { // This does not get mocked in `line-reader` module
            open: mock.fn(),
            close: mock.fn(),
            read: mock.fn(() => content),
        },
        'line-reader': { // To my surprise, even this does not get mocked in `changelog-parser` module
            readLine: mock.fn(() => content),
        }
    });

    // This leads to a real `line-reader`.readLine call, which leads to `fs.open`
    const changelog = await parseChangelog({
        filePath: 'fake',
    });

    assert.equal(changelog, content);
});

Output (Node 22.9.0)

$ node --test

✖ parseChangelog (32.4075ms)
  [Error: ENOENT: no such file or directory, open 'fake'] { errno: -2, code: 'ENOENT', syscall: 'open', path: 'fake' }

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 358.095541

✖ failing tests:

test at repro.test.mjs:6:1
✖ parseChangelog (32.4075ms)
  [Error: ENOENT: no such file or directory, open 'fake'] { errno: -2, code: 'ENOENT', syscall: 'open', path: 'fake' }

The result is, that there is no mocked fs, nor line-reader and the test really tries to open fake file, which fails.

I am trying to use memfs as a replacement for native node fs, since we are trying to create some sort of an integration test.

iambumblehead commented 1 month ago

Adding console.log({ parentURL }) to esmockLoader's resolve function normally logs every module that is loaded, however in the reproduction, the conole.log is never called again when changelog-parser is loaded.

This is un-expected because the resolve function is a hook function called directly by node.js.

I've tried various things but with no success. Tried specifying type: "module" at the reproduction package.json, tried importing changelog-parser indirectly through an additional "import-then-export-change-parser.js" file, tried removing the content of changelog-parser/index.js and replacing it with something that imports "fs" and calls fs.read, tried updating the test to only mock node:fs... no luck.

The full tests from a fresh cloned esmock are passing here, so the module loader hooks are working normally for many tests there.

Not sure what the solution is but I will keep thinking about it.

Thanks for opening the issue and creating the reproduction sample.

iambumblehead commented 1 month ago

I added the test inside the existing esmock test files and it fails there. I removed the contents of changlog-parser's index.js to be sure that the readline package is not interfering with the terminal output in some way.

Added this inside the "load" hook function and when changelog-parser is loaded, the first the console.log appears in the process output, but the subsequent calls are not seen...

  console.log('PASS1', { url })
  console.log('PASS2', { url }) // not logged
  console.log('PASS3', { url }) // not logged

My intuition is a simplified test could probably be made to demonstrate this as a node.js bug, but I don't have bandwidth at the moment to make one. Perhaps, under certain conditions, node.js wrongly continues the stack before module-loader hooks have returned values.

If you don't mind using unstable newer releases of node, you could try this new module mocking functionality https://github.com/nodejs/node/pull/52848

Filipoliko commented 1 month ago

Thank you for your quick reply, I tried using the native mock.module, but it has some other issues for a change 😃

https://github.com/nodejs/node/issues/49442#issuecomment-2393456121

iambumblehead commented 1 month ago

Thanks for the update I hope a solution will appear

iambumblehead commented 1 month ago

readline imports fs inside of an iffe and that may be what causes it to early-load fs before it is processed by the module-loader

(function () {
  var fs = require('fs')
  // ...
}());
Filipoliko commented 1 month ago

The very same use-case seems to be happening with graceful-fs library. I also cannot mock the fs dependency inside for some reason.

import { test, mock } from 'node:test';
import assert from 'node:assert';

import esmock from 'esmock';

test('graceful-fs', async () => {
    const content = 'content';
    const fs = await esmock('graceful-fs', {}, {
        fs: {
            readFileSync: mock.fn(() => content),
        },
    });

    assert.equal(fs.readFileSync(), content);
});

Not sure if patching changelog-parser will solve the issue as this seems to be happening in more than one dependency. But thank you for trying to solve this, really appreciate it and I am amazed by your activity. The problem seems to be mostly in older libraries with little maintenance, so maybe it is time to let the old ways die and focus on the new stuff :D

iambumblehead commented 1 month ago

I noticed graceful-fs does not require/import readFileSync, so I tried to to test that with readFile which is required by graceful-fs, however, my attempt also failed.

Maybe I hold a misunderstanding about the way the moduleLoader works or maybe this is a bug in the nodejs runtime... the next step should be to make a smaller test isolating the inability of the 'resolve' and 'load' hooks to indertict require usages.

There are many esmock tests and it is surprising they are missing whatever is happening here.

iambumblehead commented 1 month ago

related https://github.com/nodejs/node/issues/55307

iambumblehead commented 1 month ago

This single file mocks the fs module using a similar flow esmock should follow, but no success updating esmock to do this

repro.mjs ```javascript import { register } from 'node:module'; import assert from 'node:assert'; import fs from 'node:fs' function log (...args) { return fs.writeSync(1, JSON.stringify(args, null, ' ').slice(2, -1)) } async function resolve(referrer, context, next) { const result = await next(referrer); const url = new URL(result.url) if (result.url === 'node:fs') { result.url = import.meta.url + '#-#' + result.url } return result } async function load(url, context, next) { log('load: ' + url.slice(0, 60)) if (url.endsWith('#-#node:fs')) { log('second: node:fs is imported from dummy url import.meta.url', { url: url.slice(0, 60) }) return { shortCircuit: true, format: 'commonjs', responseURL: url, source: ` module.exports = { readFileSync: () => 'test' }` } } if (url.endsWith('file.js')) { log('first: parent that includes node:fs is returned as cjs', { url: url.slice(0, 60) }) return { shortCircuit: true, format: 'commonjs', responseURL: url, source: ` const fs = require('node:fs') module.exports = p => fs.readFileSync(p, 'utf8')` } } if (url.endsWith('#-#node:fs')) { log('second: node:fs is imported from dummy url import.meta.url', { url: url.slice(0, 60) }) return { shortCircuit: true, format: 'commonjs', responseURL: url, source: ` module.exports = { readFileSync: () => 'test' }` } } return next(url, context); } register([ `data:text/javascript,`, `import fs from 'node:fs';`, `${encodeURIComponent(log)};`, `export ${encodeURIComponent(resolve)};`, `export ${encodeURIComponent(load)}` ].join('')); const testread = (await import('./file.js')).default assert.strictEqual(testread('testpath'), 'test') ```
iambumblehead commented 1 month ago

For anyone who encounters this issue, you should be fine if your tests apply mocks to ESM targets Per the description, esmock provides: "ESM import and globals mocking for unit tests" but sample tests associated with this issue attempt to mock to import trees that are entirely commonjs.


Many subtle time-consuming adjustments are needed for esmock to possibly handle this one and there's no certainty they would be succesful.