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

Add ability to unload files from require cache (redux) #3726

Merged
merged 7 commits into from Feb 14, 2019
11 changes: 5 additions & 6 deletions lib/cli/run-helpers.js
Expand Up @@ -8,12 +8,13 @@
*/

const fs = require('fs');
const path = require('path');
const ansi = require('ansi-colors');
const debug = require('debug')('mocha:cli:run:helpers');
const minimatch = require('minimatch');
const Context = require('../context');
const path = require('path');
const Mocha = require('../mocha');
const utils = require('../utils');
const minimatch = require('minimatch');
const ansi = require('ansi-colors');

const cwd = (exports.cwd = process.cwd());

Expand Down Expand Up @@ -249,9 +250,7 @@ exports.watchRun = (
};

const purge = () => {
watchFiles.forEach(file => {
delete require.cache[file];
});
watchFiles.forEach(Mocha.unloadFile);
};

loadAndRun();
Expand Down
42 changes: 38 additions & 4 deletions lib/mocha.js
Expand Up @@ -301,7 +301,6 @@ Mocha.prototype.ui = function(name) {
};

/**
* @summary
* Loads `files` prior to execution.
*
* @description
Expand All @@ -310,6 +309,8 @@ Mocha.prototype.ui = function(name) {
*
* @private
* @see {@link Mocha#addFile}
* @see {@link Mocha#run}
* @see {@link Mocha#unloadFiles}
* @param {Function} [fn] - Callback invoked upon completion.
*/
Mocha.prototype.loadFiles = function(fn) {
Expand All @@ -324,6 +325,39 @@ Mocha.prototype.loadFiles = function(fn) {
fn && fn();
};

/**
* Removes a previously loaded file from Node's `require` cache.
*
* @private
* @static
* @see {@link Mocha#unloadFiles}
* @param {string} file - Pathname of file to be unloaded.
*/
Mocha.unloadFile = function(file) {
delete require.cache[require.resolve(file)];
};

/**
* Unloads `files` from Node's `require` cache.
*
* @description
* This allows files to be "freshly" reloaded, providing the ability
* to reuse a Mocha instance programmatically.
*
* <strong>Intended for consumers &mdash; not used internally</strong>
*
* @public
* @see {@link Mocha.unloadFile}
* @see {@link Mocha#loadFiles}
* @see {@link Mocha#run}
* @returns {Mocha} this
* @chainable
*/
Mocha.prototype.unloadFiles = function() {
this.files.forEach(Mocha.unloadFile);
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
return this;
};

/**
* Sets `grep` filter after escaping RegExp special characters.
*
Expand Down Expand Up @@ -741,8 +775,7 @@ Object.defineProperty(Mocha.prototype, 'version', {
*/

/**
* @summary
* Runs tests and invokes `fn()` when complete.
* Runs root suite and invokes `fn()` when complete.
*
* @description
* To run tests multiple times (or to run tests in files that are
Expand All @@ -751,6 +784,7 @@ Object.defineProperty(Mocha.prototype, 'version', {
*
* @public
* @see {@link Mocha#loadFiles}
* @see {@link Mocha#unloadFiles}
* @see {@link Runner#run}
* @param {DoneCB} [fn] - Callback invoked when test execution completed.
* @return {Runner} runner instance
Expand Down Expand Up @@ -787,7 +821,7 @@ Mocha.prototype.run = function(fn) {
exports.reporters.Base.hideDiff = options.hideDiff;

function done(failures) {
fn = fn || function fn() {};
fn = fn || utils.noop;
if (reporter.done) {
reporter.done(failures, fn);
} else {
Expand Down
72 changes: 72 additions & 0 deletions test/node-unit/mocha.spec.js
@@ -0,0 +1,72 @@
'use strict';

const path = require('path');
const Mocha = require('../../lib/mocha');
const utils = require('../../lib/utils');

describe('Mocha', function() {
const opts = {reporter: utils.noop}; // no output
const testFiles = [
__filename,
path.join(__dirname, 'cli', 'config.spec.js'),
path.join(__dirname, 'cli', 'run.spec.js')
];
const resolvedTestFiles = testFiles.map(require.resolve);

describe('#addFile', function() {
it('should add the given file to the files array', function() {
const mocha = new Mocha(opts);
mocha.addFile(__filename);
expect(mocha.files, 'to have length', 1).and('to contain', __filename);
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
});

it('should be chainable', function() {
const mocha = new Mocha(opts);
expect(mocha.addFile(__filename), 'to be', mocha);
});
});

describe('#loadFiles', function() {
it('should load all files from the files array', function() {
const mocha = new Mocha(opts);

testFiles.forEach(mocha.addFile, mocha);
mocha.loadFiles();
expect(require.cache, 'to have keys', resolvedTestFiles);
});

it('should execute the optional callback if given', function() {
const mocha = new Mocha(opts);
expect(cb => {
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
mocha.loadFiles(cb);
}, 'to call the callback');
});
});

describe('.unloadFile', function() {
it('should unload a specific file from cache', function() {
const resolvedFilePath = require.resolve(__filename);
require(__filename);
expect(require.cache, 'to have key', resolvedFilePath);

Mocha.unloadFile(__filename);
expect(require.cache, 'not to have key', resolvedFilePath);
});
});

describe('#unloadFiles', function() {
it('should unload all test files from cache', function() {
const mocha = new Mocha(opts);

testFiles.forEach(mocha.addFile, mocha);
mocha.loadFiles();
mocha.unloadFiles();
expect(require.cache, 'not to have keys', resolvedTestFiles);
Copy link
Member

Choose a reason for hiding this comment

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

can you just do require(__filename) instead here, like in L49?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was your idea to test against multiple files, a good one. This is somewhat how I would expect it to be used in the wild, so think this test should reflect expected usage black-box style, since the other was done as a white-box test.

});

it('should be chainable', function() {
const mocha = new Mocha(opts);
expect(mocha.unloadFiles(), 'to be', mocha);
});
});
});
32 changes: 24 additions & 8 deletions test/unit/mocha.spec.js
Expand Up @@ -21,6 +21,7 @@ describe('Mocha', function() {
sandbox.stub(Mocha.prototype, 'useColors').returnsThis();
sandbox.stub(utils, 'deprecate');
});

it('should prefer "color" over "useColors"', function() {
// eslint-disable-next-line no-new
new Mocha({useColors: true, color: false});
Expand Down Expand Up @@ -70,14 +71,6 @@ describe('Mocha', function() {
});
});

describe('.addFile()', function() {
it('should add the given file to the files array', function() {
var mocha = new Mocha(opts);
mocha.addFile('myFile.js');
expect(mocha.files, 'to have length', 1).and('to contain', 'myFile.js');
});
});

describe('.invert()', function() {
it('should set the invert option to true', function() {
var mocha = new Mocha(opts);
Expand Down Expand Up @@ -153,6 +146,7 @@ describe('Mocha', function() {
expect(mocha.options, 'to have property', 'growl', true);
});
});

describe('if not capable of notifications', function() {
it('should set the growl option to false', function() {
var mocha = new Mocha(opts);
Expand All @@ -163,6 +157,7 @@ describe('Mocha', function() {
expect(mocha.options, 'to have property', 'growl', false);
});
});

it('should be chainable', function() {
var mocha = new Mocha(opts);
expect(mocha.growl(), 'to be', mocha);
Expand Down Expand Up @@ -195,11 +190,17 @@ describe('Mocha', function() {
});

describe('.noHighlighting()', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, should this test be here -- or in "browser-unit"?

// :NOTE: Browser-only option...
it('should set the noHighlighting option to true', function() {
var mocha = new Mocha(opts);
mocha.noHighlighting();
expect(mocha.options, 'to have property', 'noHighlighting', true);
});

it('should be chainable', function() {
var mocha = new Mocha(opts);
expect(mocha.noHighlighting(), 'to be', mocha);
});
});

describe('.allowUncaught()', function() {
Expand All @@ -208,6 +209,11 @@ describe('Mocha', function() {
mocha.allowUncaught();
expect(mocha.options, 'to have property', 'allowUncaught', true);
});

it('should be chainable', function() {
var mocha = new Mocha(opts);
expect(mocha.allowUncaught(), 'to be', mocha);
});
});

describe('.delay()', function() {
Expand All @@ -216,6 +222,11 @@ describe('Mocha', function() {
mocha.delay();
expect(mocha.options, 'to have property', 'delay', true);
});

it('should be chainable', function() {
var mocha = new Mocha(opts);
expect(mocha.delay(), 'to be', mocha);
});
});

describe('.bail()', function() {
Expand All @@ -224,6 +235,11 @@ describe('Mocha', function() {
mocha.bail();
expect(mocha.suite._bail, 'to be', true);
});

it('should be chainable', function() {
plroebuck marked this conversation as resolved.
Show resolved Hide resolved
var mocha = new Mocha(opts);
expect(mocha.bail(), 'to be', mocha);
});
});

describe('error handling', function() {
Expand Down