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

feat: add support unstable_unmockModule #15080

Merged
merged 6 commits into from
Jun 30, 2024
Merged

Conversation

BondarenkoAlex
Copy link
Contributor

Closes #15079

Summary

Test plan

Copy link

linux-foundation-easycla bot commented May 21, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

netlify bot commented May 21, 2024

Deploy Preview for jestjs ready!

Name Link
🔨 Latest commit 818e185
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/668146177ebf5400085b75a0
😎 Deploy Preview https://deploy-preview-15080--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exciting start, thanks!

Could you:

  1. add an integration test
  2. fix CI failure (we need to add the new function to the types in @jest/environment)
  3. add docs
  4. add a changelog entry
  5. sign the CLA

the code itself looks correct, we just need a few things in addition to that 🙂

@mrazauskas
Copy link
Contributor

After implementing typings, could you add type tests as well, please? Similar tests of .unstable_mockModule(). They live in this file: https://github.com/jestjs/jest/blob/main/packages/jest-types/__typetests__/jest.test.ts

To run the type test: build the library and run yarn test-types.


By the way, .unstable_mockModule() is only documented here: https://jestjs.io/docs/ecmascript-modules#module-mocking-in-esm. Adding an example with .unstable_unmockModule() to this page should be enough.

@BondarenkoAlex
Copy link
Contributor Author

Where do I add integration test?
Where do i find file with?
P.S. Sorry for my English

@mrazauskas
Copy link
Contributor

Tests of .unstable_mockModule() live in this file:

jestObject.unstable_mockModule('../mockedModule.mjs', () => ({foo: 'bar'}), {

I think you can add .unstable_unmockModule() test there too.

(Don’t worry about your the English skills. This is friendly open-source project. Many of users / contributors are not native speakers.)

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're getting close! 🙂

CI is currently failing on the new tests added. One thing is that the snapshots aren't updated (you can do so by running yarn jest nativeEsm.test -u). However, the main problem is that those new tests are failing. The best way to work on them locally is to cd into e2e/native-esm and run yarn followed by node --experimental-vm-modules --no-warnings ../../packages/jest/bin/jest.js.


jestObject.unstable_unmockModule('../index.js');

const importedMockAfterUnmock = await import('../reexport.js');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work as ../reexport.js already has a reference to the old (mocked) index.js.

Do you have an example of a test of ours using unmock that works in this way? If not, I think you can just delete the test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It is not work with "mock" and "unmock"

test('test', () => {
    jest.mock('./dependencyModule', () => {
        return {getSecretNumber: () => 1000};
    });

    const {getSecretNumber} = require("./dependencyModule");
    console.log(getSecretNumber()); // output original implementation. Not mock impl.

    jest.unmock('./dependencyModule'); // it is the reset implementation in the file
});

@SimenB
Copy link
Member

SimenB commented May 29, 2024

Also, I think it'd be easiest to move the mocking tests to its own file so you can use resetModules in between each test run as mocking and unmocking the same module leaks between tests. Something like this:

diff --git c/e2e/native-esm/__tests__/native-esm-mocks.test.js w/e2e/native-esm/__tests__/native-esm-mocks.test.js
new file mode 100644
index 0000000000..3670a3525d
--- /dev/null
+++ w/e2e/native-esm/__tests__/native-esm-mocks.test.js
@@ -0,0 +1,82 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+afterEach(() => {
+  import.meta.jest.resetModules();
+});
+
+test('can mock module', async () => {
+  import.meta.jest.unstable_mockModule(
+    '../mockedModule.mjs',
+    () => ({foo: 'bar'}),
+    {virtual: true},
+  );
+
+  const importedMock = await import('../mockedModule.mjs');
+
+  expect(Object.keys(importedMock)).toEqual(['foo']);
+  expect(importedMock.foo).toBe('bar');
+});
+
+test('can mock transitive module', async () => {
+  import.meta.jest.unstable_mockModule('../index.js', () => ({foo: 'bar'}));
+
+  const importedMock = await import('../reexport.js');
+
+  expect(Object.keys(importedMock)).toEqual(['foo']);
+  expect(importedMock.foo).toBe('bar');
+});
+
+test('can unmock module', async () => {
+  import.meta.jest.unstable_mockModule('../index.js', () => ({
+    double: num => num * 1000,
+  }));
+
+  const importedMock = await import('../index.js');
+  expect(importedMock.double(2)).toBe(2000);
+
+  import.meta.jest.unstable_unmockModule('../index.js');
+
+  const importedMockAfterUnmock = await import('../index.js');
+  expect(importedMockAfterUnmock.double(2)).toBe(4);
+});
+
+test('can unmock transitive module', async () => {
+  import.meta.jest.unstable_mockModule('../index.js', () => ({
+    double: num => num * 1000,
+  }));
+
+  const importedMock = await import('../reexport.js');
+  expect(importedMock.double(2)).toBe(2000);
+
+  import.meta.jest.unstable_unmockModule('../index.js');
+
+  const importedMockAfterUnmock = await import('../reexport.js');
+  expect(importedMockAfterUnmock.double(2)).toBe(4);
+});
+
+test("can unmock module; don't override mock", async () => {
+  import.meta.jest.unstable_mockModule('../index.js', () => ({
+    double: num => num * 1000,
+  }));
+
+  const importedMock = await import('../index.js');
+  expect(importedMock.double(2)).toBe(2000);
+
+  import.meta.jest.unstable_unmockModule('../index.js');
+
+  const importedMockAfterUnmock = await import('../index.js');
+  expect(importedMockAfterUnmock.double(2)).toBe(4);
+
+  import.meta.jest.unstable_mockModule('../index.js', () => ({
+    double: () => 'never call',
+  }));
+
+  const importedMockAfterOverride = await import('../index.js');
+  expect(importedMockAfterOverride.double(2)).not.toBe('never call');
+  expect(importedMockAfterOverride.double(2)).toBe(2000);
+});
diff --git c/e2e/native-esm/__tests__/native-esm.test.js w/e2e/native-esm/__tests__/native-esm.test.js
index 2f5070a826..0b2a0a009d 100644
--- c/e2e/native-esm/__tests__/native-esm.test.js
+++ w/e2e/native-esm/__tests__/native-esm.test.js
@@ -224,76 +224,6 @@ test('require of ESM should throw correct error', () => {
   );
 });
 
-test('can mock module', async () => {
-  jestObject.unstable_mockModule('../mockedModule.mjs', () => ({foo: 'bar'}), {
-    virtual: true,
-  });
-
-  const importedMock = await import('../mockedModule.mjs');
-
-  expect(Object.keys(importedMock)).toEqual(['foo']);
-  expect(importedMock.foo).toBe('bar');
-});
-
-test('can mock transitive module', async () => {
-  jestObject.unstable_mockModule('../index.js', () => ({foo: 'bar'}));
-
-  const importedMock = await import('../reexport.js');
-
-  expect(Object.keys(importedMock)).toEqual(['foo']);
-  expect(importedMock.foo).toBe('bar');
-});
-
-test('can unmock module', async () => {
-  jestObject.unstable_mockModule('../index.js', () => ({
-    double: num => num * 1000,
-  }));
-
-  const importedMock = await import('../index.js');
-  expect(importedMock.double(2)).toBe(2000);
-
-  jestObject.unstable_unmockModule('../index.js');
-
-  const importedMockAfterUnmock = await import('../index.js');
-  expect(importedMockAfterUnmock.double(2)).toBe(4);
-});
-
-test('can unmock transitive module', async () => {
-  jestObject.unstable_mockModule('../index.js', () => ({
-    double: num => num * 1000,
-  }));
-
-  const importedMock = await import('../reexport.js');
-  expect(importedMock.double(2)).toBe(2000);
-
-  jestObject.unstable_unmockModule('../index.js');
-
-  const importedMockAfterUnmock = await import('../reexport.js');
-  expect(importedMockAfterUnmock.double(2)).toBe(4);
-});
-
-test('can unmock module; don`t override mock', async () => {
-  jestObject.unstable_mockModule('../index.js', () => ({
-    double: num => num * 1000,
-  }));
-
-  const importedMock = await import('../index.js');
-  expect(importedMock.double(2)).toBe(2000);
-
-  jestObject.unstable_unmockModule('../index.js');
-
-  const importedMockAfterUnmock = await import('../index.js');
-  expect(importedMockAfterUnmock.double(2)).toBe(4);
-
-  jestObject.unstable_mockModule('../index.js', () => ({
-    double: () => 'never call',
-  }));
-
-  const importedMockAfterOverride = await import('../index.js');
-  expect(importedMockAfterOverride.double(2)).not.toBe('never call');
-  expect(importedMockAfterOverride.double(2)).toBe(2000);
-});
-
 test('supports imports using "node:" prefix', () => {
   expect(dns).toBe(prefixDns);
 });

@BondarenkoAlex
Copy link
Contributor Author

BondarenkoAlex commented Jun 2, 2024

CI is currently failing on the new tests added. One thing is that the snapshots aren't updated (you can do so by running yarn jest nativeEsm.test -u). However, the main problem is that those new tests are failing. The best way to work on them locally is to cd into e2e/native-esm and run yarn followed by node --experimental-vm-modules --no-warnings ../../packages/jest/bin/jest.js.

bondarenko@MacBook-Pro-Banki-2 native-esm % node --experimental-vm-modules --no-warnings ../../packages/jest/bin/jest.js
node:internal/modules/cjs/loader:435
      throw err;
      ^

Error: Cannot find module '/Users/bondarenko/Documents/Bond/jest/packages/jest-cli/build/index.js'. Please verify that the package.json has a valid "main" entry
    at tryPackage (node:internal/modules/cjs/loader:427:19)
    at Function.Module._findPath (node:internal/modules/cjs/loader:640:18)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1014:27)
    at Function.Module._load (node:internal/modules/cjs/loader:873:27)
    at Module.require (node:internal/modules/cjs/loader:1100:19)
    at require (node:internal/modules/cjs/helpers:108:18)
    at Object.<anonymous> (/Users/bondarenko/Documents/Bond/jest/packages/jest-cli/bin/jest.js:16:3)
    at Module._compile (node:internal/modules/cjs/loader:1198:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1252:10)
    at Module.load (node:internal/modules/cjs/loader:1076:32) {
  code: 'MODULE_NOT_FOUND',
  path: '/Users/bondarenko/Documents/Bond/jest/packages/jest-cli/package.json',
  requestPath: '..'
}

https://github.com/jestjs/jest/blob/main/packages/jest-cli/bin/jest.js#L16

@BondarenkoAlex
Copy link
Contributor Author

@SimenB @mrazauskas
All tests are passed.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great stuff, thanks!

CHANGELOG.md Outdated Show resolved Hide resolved
@SimenB SimenB merged commit 9c9ce8a into jestjs:main Jun 30, 2024
5 checks passed
Copy link

github-actions bot commented Aug 2, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: jest.resetModules does not works for jest.unstable_mockModule
3 participants