From 55d042e0242fa19324f116a3af6fb4066e5a85c1 Mon Sep 17 00:00:00 2001 From: Kartones Date: Thu, 30 May 2024 13:58:51 +0200 Subject: [PATCH] fix: full import/require cache blast when running non-parallel watch --- lib/cli/watch-run.js | 44 +++++++++---- lib/nodejs/esm-utils.js | 10 ++- .../watch/dependency-with-state.fixture.js | 11 ++++ .../watch/hook-mutating-dependency.fixture.js | 9 +++ .../watch/test-mutating-dependency.fixture.js | 9 +++ ...st-with-hook-mutated-dependency.fixture.js | 17 +++++ test/integration/options/watch.spec.js | 65 +++++++++++++++++++ 7 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 test/integration/fixtures/options/watch/dependency-with-state.fixture.js create mode 100644 test/integration/fixtures/options/watch/hook-mutating-dependency.fixture.js create mode 100644 test/integration/fixtures/options/watch/test-mutating-dependency.fixture.js create mode 100644 test/integration/fixtures/options/watch/test-with-hook-mutated-dependency.fixture.js diff --git a/lib/cli/watch-run.js b/lib/cli/watch-run.js index a77ed7a91a..857fef35da 100644 --- a/lib/cli/watch-run.js +++ b/lib/cli/watch-run.js @@ -6,7 +6,9 @@ const path = require('path'); const chokidar = require('chokidar'); const Context = require('../context'); const collectFiles = require('./collect-files'); - +const {handleRequires} = require('./handle-requires'); +const {setCacheBusterKey} = require('../nodejs/esm-utils'); +const {uniqueID} = require('../utils'); /** * Exports the `watchRun` function that runs mocha in "watch" mode. * @see module:lib/cli/run-helpers @@ -97,9 +99,14 @@ exports.watchRun = (mocha, {watchFiles, watchIgnore}, fileCollectParams) => { return createWatcher(mocha, { watchFiles, watchIgnore, - beforeRun({mocha}) { + async beforeRun({mocha}) { mocha.unloadFiles(); + // Reload hooks. If not done, global hooks keep their state between watch runs, but test files always get a new + // instance, making state mutation impossible via global hooks. + const plugins = await handleRequires(mocha.options.require); + mocha.options.rootHooks = plugins.rootHooks; + // I don't know why we're cloning the root suite. const rootSuite = mocha.suite.clone(); @@ -257,13 +264,17 @@ const createRerunner = (mocha, watcher, {beforeRun} = {}) => { // true if a file has changed during a test run let rerunScheduled = false; - const run = () => { + const run = async () => { try { - mocha = beforeRun ? beforeRun({mocha, watcher}) || mocha : mocha; + mocha = beforeRun ? (await beforeRun({mocha, watcher})) || mocha : mocha; + const blastFullCache = !mocha.options.parallel; runner = mocha.run(() => { debug('finished watch run'); runner = null; - blastCache(watcher); + if (blastFullCache) { + setCacheBusterKey(uniqueID()); + } + blastCache(watcher, blastFullCache); if (rerunScheduled) { rerun(); } else { @@ -348,15 +359,26 @@ const eraseLine = () => { /** * Blast all of the watched files out of `require.cache` * @param {FSWatcher} watcher - chokidar FSWatcher + * @param {boolean} blastAll * @ignore * @private */ -const blastCache = watcher => { - const files = getWatchedFiles(watcher); - files.forEach(file => { - delete require.cache[file]; - }); - debug('deleted %d file(s) from the require cache', files.length); +const blastCache = (watcher, blastAll) => { + if (blastAll) { + Object.keys(require.cache) + // Avoid deleting mocha binary (at minimum, breaks Mocha's watch tests) + .filter(file => !file.includes('/mocha/bin/')) + .forEach(file => { + delete require.cache[file]; + }); + debug('deleted all files from the require cache'); + } else { + const files = getWatchedFiles(watcher); + files.forEach(file => { + delete require.cache[file]; + }); + debug('deleted %d file(s) from the require cache', files.length); + } }; /** diff --git a/lib/nodejs/esm-utils.js b/lib/nodejs/esm-utils.js index 5318099365..ce707254b6 100644 --- a/lib/nodejs/esm-utils.js +++ b/lib/nodejs/esm-utils.js @@ -3,6 +3,12 @@ const url = require('url'); const forward = x => x; +let cacheBusterKey = ''; + +exports.setCacheBusterKey = key => { + cacheBusterKey = key; +}; + const formattedImport = async (file, esmDecorator = forward) => { if (path.isAbsolute(file)) { try { @@ -32,7 +38,9 @@ const formattedImport = async (file, esmDecorator = forward) => { return exports.doImport(esmDecorator(file)); }; -exports.doImport = async file => import(file); +// When changing the key, old items won't be garbage collected, but there is no alternative with import(). +// More info: https://github.com/nodejs/node/issues/49442 +exports.doImport = async file => import(`${file}?cache=${cacheBusterKey}`); exports.requireOrImport = async (file, esmDecorator) => { if (path.extname(file) === '.mjs') { diff --git a/test/integration/fixtures/options/watch/dependency-with-state.fixture.js b/test/integration/fixtures/options/watch/dependency-with-state.fixture.js new file mode 100644 index 0000000000..093b027810 --- /dev/null +++ b/test/integration/fixtures/options/watch/dependency-with-state.fixture.js @@ -0,0 +1,11 @@ +let flag = false; + +module.exports.getFlag = () => flag; + +module.exports.enableFlag = () => { + flag = true; +}; + +module.exports.disableFlag = () => { + flag = false; +}; diff --git a/test/integration/fixtures/options/watch/hook-mutating-dependency.fixture.js b/test/integration/fixtures/options/watch/hook-mutating-dependency.fixture.js new file mode 100644 index 0000000000..247378bfd4 --- /dev/null +++ b/test/integration/fixtures/options/watch/hook-mutating-dependency.fixture.js @@ -0,0 +1,9 @@ +const dependency = require('./lib/dependency-with-state'); + +module.exports = { + mochaHooks: { + beforeEach: () => { + dependency.enableFlag(); + } + } +}; diff --git a/test/integration/fixtures/options/watch/test-mutating-dependency.fixture.js b/test/integration/fixtures/options/watch/test-mutating-dependency.fixture.js new file mode 100644 index 0000000000..4357be7d64 --- /dev/null +++ b/test/integration/fixtures/options/watch/test-mutating-dependency.fixture.js @@ -0,0 +1,9 @@ +const dependency = require('./lib/dependency-with-state'); + +it('checks and mutates dependency', () => { + if (dependency.getFlag()) { + throw new Error('test failed'); + } + // Will pass 1st run, fail on subsequent ones + dependency.enableFlag(); +}); diff --git a/test/integration/fixtures/options/watch/test-with-hook-mutated-dependency.fixture.js b/test/integration/fixtures/options/watch/test-with-hook-mutated-dependency.fixture.js new file mode 100644 index 0000000000..b8086ae55d --- /dev/null +++ b/test/integration/fixtures/options/watch/test-with-hook-mutated-dependency.fixture.js @@ -0,0 +1,17 @@ +const dependency = require('./lib/dependency-with-state'); + +// Will fail 1st run, unless hook runs +before(() => { + dependency.disableFlag(); +}); + +// Will pass 1st run, fail on subsequent ones, unless hook runs +afterEach(() => { + dependency.disableFlag(); +}); + +it('hook should have mutated dependency', () => { + if (!dependency.getFlag()) { + throw new Error('test failed'); + } +}); diff --git a/test/integration/options/watch.spec.js b/test/integration/options/watch.spec.js index 957b4938c3..457f6a5e4d 100644 --- a/test/integration/options/watch.spec.js +++ b/test/integration/options/watch.spec.js @@ -316,6 +316,71 @@ describe('--watch', function () { }); }); + // Regression test for https://github.com/mochajs/mocha/issues/5149 + it('reloads all required dependencies between runs', function () { + const testFile = path.join(tempDir, 'test-mutating-dependency.js'); + copyFixture('options/watch/test-mutating-dependency', testFile); + + const dependency = path.join(tempDir, 'lib', 'dependency-with-state.js'); + copyFixture('options/watch/dependency-with-state', dependency); + + // Notice we are watching only the test file, skipping the dependency file. + // This is a simplification of a scenario where there's an unwatched file somewhere in the dependency tree + // that is mutated between runs, and not properly reset. + return runMochaWatchJSONAsync( + [testFile, '--watch-files', 'test-mutating-dependency.js'], + tempDir, + () => { + replaceFileContents( + testFile, + '// Will pass 1st run, fail on subsequent ones', + '// Will pass 1st run, fail on subsequent runs' + ); + } + ).then(results => { + expect(results, 'to have length', 2); + expect(results[0].passes, 'to have length', 1); + expect(results[0].failures, 'to have length', 0); + expect(results[1].passes, 'to have length', 1); + expect(results[1].failures, 'to have length', 0); + }); + }); + + // Regression test for https://github.com/mochajs/mocha/issues/5149 + it('reloads all required dependencies between runs when mutated from a hook', function () { + const testFile = path.join( + tempDir, + 'test-with-hook-mutated-dependency.js' + ); + copyFixture('options/watch/test-with-hook-mutated-dependency', testFile); + + const dependency = path.join(tempDir, 'lib', 'dependency-with-state.js'); + copyFixture('options/watch/dependency-with-state', dependency); + + const hookFile = path.join(tempDir, 'hook-mutating-dependency.js'); + copyFixture('options/watch/hook-mutating-dependency', hookFile); + + return runMochaWatchJSONAsync( + [ + testFile, + '--require', + hookFile, + '--watch-files', + 'test-with-hook-mutated-dependency.js' + ], + tempDir, + () => { + touchFile(testFile); + } + ).then(results => { + expect(results.length, 'to equal', 2); + expect(results[0].passes, 'to have length', 1); + expect(results[0].failures, 'to have length', 0); + expect(results[1].passes, 'to have length', 1); + expect(results[1].failures, 'to have length', 0); + }); + }); + // Regression test for https://github.com/mochajs/mocha/issues/2027 it('respects --fgrep on re-runs', async function () { const testFile = path.join(tempDir, 'test.js');