-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mocking import, import(), and require() #42242
Comments
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. |
@nodejs/loaders @giltayar |
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). |
(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 ) |
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 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 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. |
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 If there was a way to define all the package.json // 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
Yeah, this is already the case with |
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 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 This isn't a lot of transpilation, but it isn't none. Maybe I'm missing something? |
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. |
Yes, it does (different API, but still yes). What you're missing is in how it works without transpiling. Whenever you tell 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...
Yes to the first point |
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 |
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. |
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. |
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 itsrequire()
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, sinceimport
is a part of the language, we can no longer easily just pass a differentrequire
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 overrideimport
.Any
import
calls which are not overridden in this way fall through to the current implementation.vm.runIn*Context
code.../index.js
loadedother-module
that didimport {readFile} from 'node:fs'
, it would get our mocked copy.) It's possible to work around this (egrequire-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:
which is what I present today, using a
Module
subclass with a customModule.require
method. See https://github.com/tapjs/libtap/blob/main/lib/mock.jsWhat alternatives have you considered?
Of these, (3) is the most likely to succeed, but (2) is the most attractive. (1) is not really working any more.
The text was updated successfully, but these errors were encountered: