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

jest.resetModules breaks after upgrading from v24 #11426

Closed
AriPerkkio opened this issue May 20, 2021 · 5 comments
Closed

jest.resetModules breaks after upgrading from v24 #11426

AriPerkkio opened this issue May 20, 2021 · 5 comments

Comments

@AriPerkkio
Copy link

πŸ› Bug Report

jest.resetModules does not reset dynamically generated files. It works perfectly with jest@24 but breaks after >=25.

With jest@24:

 PASS  ./index.test.js
  repros
    βœ“ First (7ms)
    βœ“ Second (4ms)

With jest@25, jest@26 and jest@next:

 FAIL  ./index.test.js
  repros
    βœ“ First (5 ms)
    βœ• Second (7 ms)

  ● repros β€Ί Second

    expect(received).toBe(expected) // Object.is equality

    Expected: "Second"
    Received: "First"

      16 |
      17 |     test('Second', () => {
    > 18 |         expect(method('Second')).toBe('Second');
         |                                  ^
      19 |     });
      20 | });
      21 |

      at Object.<anonymous> (index.test.js:18:34)

To Reproduce

Steps to reproduce the behavior:

// index.js
const fs = require('fs');

module.exports = function method(value) {
    fs.writeFileSync(
        './dynamically-generated-file.js',
        `module.exports = "${value}";`
    );

    return require('./dynamically-generated-file.js');
}
// index.test.js
const fs = require('fs');
const method = require('./index');

describe('repros', () => {
    beforeEach(() => {
        jest.resetModules();

        if(fs.existsSync('./dynamically-generated-file.js')) {
            fs.unlinkSync('./dynamically-generated-file.js');
        }
    })

    test('First', () => {
        expect(method('First')).toBe('First');
    });

    test('Second', () => {
        expect(method('Second')).toBe('Second');
    });
});

Commenting out jest.resetModules with jest@24 makes the test fail -> works exactly as expected.

Expected behavior

The cached dynamically-generated-file.js should be purged exactly as it is in jest@24.

Link to repl or repo (highly encouraged)

https://github.com/AriPerkkio/jest-resetModules-repro

Use yarn add --dev jest@25 or yarn add --dev jest@26 to make the test fail.

envinfo

Run while jest@24 is installed:

  System:
    OS: Linux 5.4 Debian GNU/Linux 10 (buster) 10 (buster)
    CPU: (4) x64 06/8e
  Binaries:
    Node: 14.16.1 - /usr/bin/node
    Yarn: 1.22.5 - /usr/bin/yarn
    npm: 6.14.12 - /usr/bin/npm
  npmPackages:
    jest: 24 => 24.9.0 

Actual project where I'm running into this issue: https://github.com/AriPerkkio/eslint-remote-tester-run-action/blob/master/src/run-tester.ts

If there is a workaround available please let me know.

@walisc
Copy link

walisc commented Jul 18, 2021

Yes, I am experiencing the same issue. Could this please be addressed . From my investigation it seem like the introduction of a file caching about 7 months ago is the main issue. I'm not sure of all the linked caches associated with it, but it seem like _cacheFS (which caches the raw file content) and _fileTransforms (which caches the transformed versions) in jest-runtime (packages/jest-runtime/src/index.ts) are the main culprits. I do get the motivation for this in terms of performance, but it does make the resetModules() contract untrue/misleading.

As simple acid test for this is clearing out _cacheFS and _fileTransforms when resetModules() is called, which cannot be viable as a solution as the caches contains more than just your test files.

  • A more elegant solution could be to have a config endpoint in which you can specify files you don't want cached (using regex, or referencing a function).

  • Alternatively, you could set a flag when isolateModules() is called that disables use of the _cacheFS (and associated caches) in this mode (although your resetModules() contract will still be misleading).

  • Another solution could be to use cascading caches i.e invalidating the moduleRegisty should cascade to invalidate the associated entry in the _cacheFS cache, and in turn invalidate the _fileTransforms cached (and the _sourceMapRegistry and other associated caches).

As it is now, I'm force to create sub-directories in which to output the generated files, which is turn creates inconsistencies between with actual code, and the test code. Please could address this.

@walisc
Copy link

walisc commented Jul 18, 2021

@AriPerkkio I ended up just patching jest to have a config endpoint that accepts a list of reg patterns (similar coveragePathIgnorePatterns) to determine which files not to cache if you are interested. You can see these patched files here:- https://gist.github.com/walisc/9879b14fdbc41c40237cdadb33dad2e8
To apply them you can use patch-package or git apply (although patch-package is designed for these type of scenarios)

p.s I did not extensively look into this, and there could be better solutions for dealing with this. My main aim was just to get my tests to pass, with as little changes as possible so I can continue with the project I am working on. All this to say, use at your own risk.

bpinto added a commit to bpinto/netlify-lambda that referenced this issue Aug 18, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Aug 18, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Aug 18, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Aug 18, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Aug 18, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Sep 1, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Sep 1, 2021
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
bpinto added a commit to bpinto/netlify-lambda that referenced this issue Apr 4, 2022
It's not a direct replacement, however this may cover all the needs. If
needed, we can work on the merge strategy.

Jest has a file system cache that is not cleared, not even using
`jest.resetModules()`. As a workaround, we create the webpack config
files with a unique suffix.

Ref:
jestjs/jest#11426 (comment)
@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 17, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2023
@github-actions
Copy link

This issue 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 Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants