node icon indicating copy to clipboard operation
node copied to clipboard

Mocking import, import(), and require()

Open isaacs opened this issue 2 years ago • 12 comments

What is the problem this feature will solve?

Node-tap today has a feature called t.mock(), based on require-inject, which allows a test to load a fresh copy of a module, with its require() method overridden to return specific values.

This is incredibly useful for tests, allowing one to simulate all manner of error conditions, edge cases, and so on.

However, I cannot see any way to make this work with es modules. The module loading machinery is not exposed in any way that would allow it to be overridden or injected. This is safer and preferable in many ways to the way that Module._require can be just blown away by plain old JavaScript, however, since import is a part of the language, we can no longer easily just pass a different require method to the code.

What is the feature you are proposing to solve the problem?

Add a new configuration to vm.runInContext (et al) allowing the caller to override import.

const mockedReadFile = async (filename) => Buffer.from('hello')
import * as fs from 'node:fs'
const { readFile } = fs.promises
import { runInThisContext } from 'node:vm'
import { fileURLToPath, pathToFileURL } from 'node:url'

const runTest = async () => {
  const modulePath = resolve(fileURLToPath(import.meta.url), '../index.js')
  const moduleURL = pathToFileURL(modulePath)
  const moduleContents = await readFile('index.js')
  const mockedModule = vm.runInThisContext(moduleContents, moduleURL, {
    // the 'import' key is a set of module URLs to override values
    // override values are used as the result of the import.
    importOverrides: {
      'node:fs': {
        ...fs,
        promises: {
          ...fs.promises,
          readFile: mockedReadFile,
        },
      },
    },
  }
  const result = mockedModule.reverseFile('blah.txt')
  assert.equal(result, 'elloh') // module got 'hello' when it called fs.promises.readFile()
}
runTest()

Any import calls which are not overridden in this way fall through to the current implementation.

  1. Import overrides only take effect within the module graph starting from the vm.runIn*Context code.
  2. Any non-overridden modules within this module graph would still load their imports from the override set. (That is, if ../index.js loaded other-module that did import {readFile} from 'node:fs', it would get our mocked copy.) It's possible to work around this (eg require-inject does not provide this guarantee) but it makes the loading much more complicated when you want to override a module that's used in many places.

This is of course extremely verbose and low-level, but serves as a building block upon which more ergonomic features can be built. The way this would look in node-tap, for example, would be something like:

const mockedModule = t.mock('../index.js', {
  fs: {
    ...fs,
    promises: {
      ...fs.promises,
      readFile: async () => Buffer.from('hello'),
    },
  },
})

which is what I present today, using a Module subclass with a custom Module.require method. See https://github.com/tapjs/libtap/blob/main/lib/mock.js

What alternatives have you considered?

  1. Just not supporting ESM, swimming upstream forever against the inevitable.
  2. Forking node, quitting my job, and moving into the woods to write a new JS platform with an extensible module system.
  3. Transpiling any ESM code encountered to CJS and then doing the thing I'm doing now.

Of these, (3) is the most likely to succeed, but (2) is the most attractive. (1) is not really working any more.

isaacs avatar Mar 07 '22 06:03 isaacs

It seems like https://www.npmjs.com/package/mock-import might get most of the way there? Maybe I just wasn't searching for the right words. I'll see if I can get what I need from that, it seems to be built on the loader API which is experimental, but looks to have settled a lot in recent versions.

isaacs avatar Mar 07 '22 07:03 isaacs

@nodejs/loaders @giltayar

benjamingr avatar Mar 07 '22 07:03 benjamingr

Forking node, quitting my job, and moving into the woods to write a new JS platform with an extensible module system.

Why does that sound so appealing 😅

Anyway - you are indeed looking for loaders. I would check what https://www.npmjs.com/package/testdouble does since it does this (and is pretty popular).

benjamingr avatar Mar 07 '22 08:03 benjamingr

(You can also use other machinery like policies https://nodejs.org/api/policy.html#dependency-redirection to do this but this is "right up the alley" of loaders )

benjamingr avatar Mar 07 '22 08:03 benjamingr

You can also use package.json#imports to do this as well:

https://twitter.com/bradleymeck/status/1493610704515051528

There are a bunch of ways to do this but having some mocking built into core seems more reasonable than low level machinery. The reason things like --loader remain experimental is stuff keeps changing underneath it due to it being a language level construct and things like cache keys keep getting punched by the spec.

We could look into actual ways to expose the mocking use case much more easily than exposing low level highly unstable language level integrations. We already have a test/fixture for mocking. Expanding this to act more like import.meta.hot as a PR seems reasonable if none of the alternatives work but there are problems like lack of a restart mechanism that import.meta.hot expects to be available (there are a couple open issues on various features that would enable that restart workflow though).

I would note that at the VM/language level mocking ESM WILL LEAK MEMORY. Whatever mocks get made, they will not be able to be GC'd. I am very far into -7 or even worse on trying to expose this as low level machinery since Node doesn't control the problematic things we have seen coming out of language evolution but -0 on exposing some kind of mocking API since controlling at the high level hasn't seemed to be too big of a problem as long as the expectation isn't to deal w/ multi-part cache keys from things like import assertions, evaluators, realms, compartments, etc. I think having those multi-part keys be unsupported and expected to remain unsupported is fine to move forward with.

bmeck avatar Mar 07 '22 14:03 bmeck

You can also use package.json#imports to do this as well: https://twitter.com/bradleymeck/status/1493610704515051528

That's a cute approach, I think I'd heard about doing that some time ago. Unfortunately, you'd pretty often want to have lots of different mocks for the same module, even within a single test file (make sure it does the expected behavior if we get any of these 15 error conditions from the API server...) And users get rightfully twitchy about tests editing their package.json files. And it means a separate node subprocess invocation for every unique combination of mocks, which means running test setup code repeatedly which is unexpected. All in all, not really a solution, I don't think, but it feels close.

If there was a way to define all the package.json imports dynamically, for the context of a single module import or require instead of the whole process, without editing package.json, and these dynamic imports could set the module exports rather than defining the file to load, then that would be pretty much ideal.

// const fnCaller = await t.mockESM('./fn-caller.js', {'./call-fn.js': ...}) would do this:
const module = require('module') // or wherever it gets hung
let called = false
module.setImports({
  './call-fn.js': () => called = true
})
const fnCaller = await import('./fn-caller.js')
module.unsetImports() // unhook the mocks

fnCaller.doSomething()
t.equal(called, true)

Something like that, and we're pretty close to the interface t.mock() offers today.

I would note that at the VM/language level mocking ESM WILL LEAK MEMORY.

Yeah, this is already the case with t.mock() today mocking require(). You'd have to be pretty adventurous to do that kind of thing at run-time anyway, and for tests, leaking a little function a thousand times is still better performance than actually reaching out over the network or hitting disk, if you can avoid it. And it lets you push your mocks as far away from your system under test as possible, since you can just mock the bits that do the slow/nondeterministic thing.

isaacs avatar Mar 08 '22 23:03 isaacs

What am I missing? ESM mocking of modules has been successfully implemented in testdouble. So I must be missing something in the requirements.

I have a blog post that discusses the technical aspects of it: https://gils-blog.tayar.org/posts/mock-all-you-want-supporting-esm-in-testdouble-js-mocking-library/

(it is somewhat out of date because the loader interface has changed since then, but the technique used there still applies.

giltayar avatar Mar 10 '22 17:03 giltayar

@giltayar testdouble and mock-import both work by requiring the use of a process-level loader, and transpiling source of the importing module to replace imports with their mocked contents. I'm trying to avoid transpiling altogether.

For example:

// foo.mjs
import { barFS } from './bar.mjs'
import fs from 'fs'
import assert from 'assert'
assert.equal(fs, barFS)
export const fooFS = fs
// bar.mjs
import fs from 'fs'
export const barFS = fs
// test.mjs
import assert from 'assert'
const firstMock = { fake: 'not actually fs' }
const secondMock = { fake: 'another fake fs' }
import fs from 'fs'
// these also assert that bar got the same mocked fs
// continues all the way down the import graph
const foo1 = await mock('./foo.mjs', { fs: firstMock })
const foo2 = await mock('./foo.mjs', { fs: secondMock })
await foo3 = await mock('./foo.mjs') // no mock fs provided
assert.equal(foo1.fooFS, firstMock)
assert.equal(foo2.fooFS, secondMock)
assert.equal(foo3.fooFS, fs)

Does the quibble approach you posted do this, without transpiling the foo.mjs in this example? It seems to me like you'd have to provide different source for foo.mjs, to replace its import calls with calls to the module that provides a result from your global mock collection.

This isn't a lot of transpilation, but it isn't none. Maybe I'm missing something?

isaacs avatar Mar 18 '22 17:03 isaacs

I don't know exactly your usage of the term "transpiling" here. Do you mean without creating a new Module using source text? Then no, there isn't a way to do it robustly even using v8::SyntheticModule (which fails in the presence of cycles). Making a new Module using source text rather than a reflective Object API is the robust way to do this for now.

bmeck avatar Mar 18 '22 18:03 bmeck

@isaacs

Does the quibble approach you posted do this, without transpiling the foo.mjs in this example?

Yes, it does (different API, but still yes). What you're missing is in how it works without transpiling. Whenever you tell quibble you want to mock a module, it remembers that information in its state. Then, when the import(...) of that module comes along, it uses the resolve hook of ESM to change the url of the import to something with a query parameter that "points" the latest mock. That way, every time you change how you want to mock the module, the import url of that module will be changed to a different one. It's all described in the (admittedly long) technical blog post linked to above.

Note: this method is still process-global: you can switch a mock in the middle, but you can't "scope" a mock so that two async threads will have different mocks.

So...

work by requiring the use of a process-level loader, and transpiling source of the importing module to replace imports with their mocked contents

Yes to the first point --loader quibble and no to the second point (transpiling).

giltayar avatar Mar 19 '22 06:03 giltayar

Ah! I was missing the fact that the "url" returned by the resolve() hook can be literally anything, and then the load hook can provide whatever source it wants. I think I have a way forward on this, but it'd still be nice if it was something that could be used without having to start with a --loader hook, especially given the scary experimental warning.

isaacs avatar Mar 23 '22 00:03 isaacs

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Sep 19 '22 01:09 github-actions[bot]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Oct 19 '22 01:10 github-actions[bot]