Skip to content
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

docs: clarify that mockImport is "load with a mocked import" and not "mock the inported thing" #1022

Open
3 tasks done
onosendi opened this issue Apr 21, 2024 · 8 comments
Open
3 tasks done
Labels
bug something not go good

Comments

@onosendi
Copy link

onosendi commented Apr 21, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Have you read the CONTRIBUTING guide on posting bugs, and CODE_OF_CONDUCT?

  • yes I read the things

This issue exists in the latest tap version

  • I am using the latest tap

Description

mockImport doesn't seem to mock the module as expected.

Reproduction

// a.js

const foo = 'bar';

export { foo };

// a.spec.js

import t from 'tap';

const foo = await t.mockImport('./a.js', {
  foo: 'baz',
});

t.test('', async () => {
  console.log(foo);
  // [Module: null prototype] { foo: 'bar' }
})

Environment


127 ~/repos/foo [develop] % npm ls tap
foo@ /Users/foo/repos/foo
└── tap@18.7.2 extraneous

~/repos/foo [develop] % npx tap versions
tap: 18.7.2
"@tapjs/config": 2.4.17
"@tapjs/core": 1.5.2
"@tapjs/run": 1.5.2
"@tapjs/stack": 1.2.8
"@tapjs/test": 1.4.2
tap-parser: 15.3.2
tap-yaml: 2.2.2
tcompare: 6.4.6
plugins:
  "@tapjs/after": 1.1.20
  "@tapjs/after-each": 1.1.20
  "@tapjs/asserts": 1.1.20
  "@tapjs/before": 1.1.20
  "@tapjs/before-each": 1.1.20
  "@tapjs/filter": 1.2.20
  "@tapjs/fixture": 1.2.20
  "@tapjs/intercept": 1.2.20
  "@tapjs/mock": 1.3.2
  "@tapjs/node-serialize": 1.3.2
  "@tapjs/snapshot": 1.2.20
  "@tapjs/spawn": 1.1.20
  "@tapjs/stdin": 1.1.20
  "@tapjs/typescript": 1.4.2
  "@tapjs/worker": 1.1.20

  ~/repos/foo [develop] % npx tap config list
# vim: set filetype=yaml :
color: true
coverage-report:
  - text
exclude:
  - "**/@(fixture*(s)|dist)/**"
include:
  - "**/@(test?(s)|__test?(s)__)/**/*.@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/*.@(test?(s)|spec).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/test?(s).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
jobs: 8
reporter: base
snapshot-clean-cwd: true
timeout: 30

~/repos/foo [develop] % npx tap plugin list

@tapjs/after
@tapjs/after-each
@tapjs/asserts
@tapjs/before
@tapjs/before-each
@tapjs/filter
@tapjs/fixture
@tapjs/intercept
@tapjs/mock
@tapjs/node-serialize
@tapjs/snapshot
@tapjs/spawn
@tapjs/stdin
@tapjs/typescript
@tapjs/worker

~/repos/foo [develop] % uname -a

Darwin Foos-M1 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000 arm64

@onosendi onosendi added the bug something not go good label Apr 21, 2024
@isaacs
Copy link
Member

isaacs commented Apr 24, 2024

That's not what mock import does.

It's for mocking the things that that module imports.

Example, if a.js imported b.js, you could do this:

// a.js
import { foo } from './b.js'
export { foo }
// b.js
export const foo = 'bar'
// a.spec.js
import t from 'tap'
const a = await t.mockImport('./a.js', {
  './b.js': {
    foo: 'baz'
  }
})
console.log(a.foo) // 'baz'

In other words, it's not "a thing that you import and then mock", it's "load this module, but with a mocked import() method".

If you just want to import something and then swap out some fields, check out t.capture and t.intercept.

For example:

import t from 'tap'

const a = await import('./a.js')
t.equal(a.foo, 'bar')
const fooAccesses = t.intercept(a, 'foo', { value: 'baz' })
t.equal(a.foo, 'baz')
console.error(fooAccesses) // [ { type: 'get', value: 'baz', ... } ]

@isaacs isaacs closed this as completed Apr 24, 2024
@onosendi
Copy link
Author

Ah, roger that.

@isaacs
Copy link
Member

isaacs commented Apr 29, 2024

Actually, you know, "mock the imported thing" is a reasonable guess as to what a method named mockInport() does. Maybe the docs need to clarify this a bit better.

Reopening as a doc issue.

@isaacs isaacs reopened this Apr 29, 2024
@isaacs isaacs changed the title [BUG] Imported modules aren't getting mocked. docs: clarify that mockImport is "load with a mocked import" and not "mock the inported thing" Apr 29, 2024
@onosendi
Copy link
Author

@isaacs it was especially confusing because I was coming from jest's jest.mock.

@isaacs
Copy link
Member

isaacs commented Apr 29, 2024

it was especially confusing because I was coming from jest's jest.mock.

It's a matter of taste, so there's really no right or wrong answer per se, and the word "mock" is super overloaded in the testing space so it's a reasonable approach I guess, but personally I find jest's import mocking pretty confusing to reason about. It's sort of global, but not exactly, and feels very magical. Tap's import mocking is just a specific function that loads the actual module code, but with its dependencies injected with known values, so you can do the mocked import behavior in a clearer and more isolated scope.

It's also handy when you want to have a single test file that mocks things in a bunch of different ways, so eg you can load a module with { path: path.posix } and then a second time with { path: path.win32 } to test different branches, or just load the module fresh with different globals/env values, etc.

@isaacs
Copy link
Member

isaacs commented Apr 29, 2024

I'd thought about naming it t.import(), probably would have made it more clear that it's "import this module (with some tweaked deps)". But the concern there was import { import } from 'tap' wouldn't work, you'd have this special keyword making footguns etc.

@onosendi
Copy link
Author

Hmm, using your example gives me:

const fooAccesses = t.intercept(a, 'foo', { value: 'baz' });

Error: Cannot intercept property 'foo', defined {configurable:false}

import t from 'tap';

const a = await import('./a.js');
t.equal(a.foo, 'bar');
const fooAccesses = t.intercept(a, 'foo', { value: 'baz' });
t.equal(a.foo, 'baz');
console.error(fooAccesses);

@isaacs
Copy link
Member

isaacs commented May 1, 2024

Oh, yeah, if a thing is {configurable:false} then there'd be no way to intercept it.

But you could always put it on a different object, and intercept that.

import t from 'tap';

const a = { ...(await import('./a.js')) };
t.equal(a.foo, 'bar');
const fooAccesses = t.intercept(a, 'foo', { value: 'baz' });
t.equal(a.foo, 'baz');
console.error(fooAccesses);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something not go good
Projects
None yet
Development

No branches or pull requests

2 participants