From d613cd5151d2fd1515efea6d4dfd5d53a7d8f552 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Mon, 4 Feb 2019 05:26:09 -0600 Subject: [PATCH 01/10] feat(lib/utils.js): Add utility routines Added two functions, sQuote and dQuote, for quoting text. Added ngettext function for message translation for dealing with plurality. --- lib/utils.js | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/lib/utils.js b/lib/utils.js index bf50ee8ee9..b9954e2635 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -736,6 +736,76 @@ exports.clamp = function clamp(value, range) { return Math.min(Math.max(value, range[0]), range[1]); }; +/** + * Single quote text by combining with undirectional ASCII quotation marks. + * + * @description + * Provides a simple means of markup for quoting text to be used in output. + * Use this to quote names of variables, methods, and packages. + * + * package 'foo' cannot be found + * + * @private + * @param {string} str - Value to be quoted. + * @returns {string} quoted value + * @example + * sQuote('n') // => 'n' + */ +exports.sQuote = function(str) { + return "'" + str + "'"; +}; + +/** + * Double quote text by combining with undirectional ASCII quotation marks. + * + * @description + * Provides a simple means of markup for quoting text to be used in output. + * Use this to quote names of datatypes, classes, and strings. + * + * argument 'value' must be "string" or "number" + * + * @private + * @param {string} str - Value to be quoted. + * @returns {string} quoted value + * @example + * dQuote('number') // => "number" + */ +exports.dQuote = function(str) { + return '"' + str + '"'; +}; + +/** + * Provides simplistic message translation for dealing with plurality. + * + * @description + * Use this to create messages which need to be singular or plural. + * Some languages have several plural forms, so _complete_ message clauses + * are preferable to generating the message on the fly. + * + * @private + * @param {number} n - Non-negative integer + * @param {string} msg1 - Message to be used in English for `n = 1` + * @param {string} msg2 - Message to be used in English for `n = 0, 2, 3, ...` + * @returns {string} message corresponding to value of `n` + * @example + * var sprintf = require('util').format; + * var pkgs = ['one', 'two']; + * var msg = sprintf( + * ngettext( + * pkgs.length, + * 'cannot load package: %s', + * 'cannot load packages: %s' + * ), + * pkgs.map(sQuote).join(', ') + * ); + * console.log(msg); // => cannot load packages: 'one', 'two' + */ +exports.ngettext = function(n, msg1, msg2) { + if (typeof n === 'number' && n >= 0) { + return n === 1 ? msg1 : msg2; + } +}; + /** * It's a noop. * @public From 081641755d23e44879bdc7f02cd45c737ec8ff35 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Mon, 4 Feb 2019 05:32:04 -0600 Subject: [PATCH 02/10] refactor(lib/runner.js): Refactor error creation of detected leaks Simplified logic related to plurality in `checkGlobals`. Refactored `failHook` to use quotation for hook title. --- lib/runner.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 7c4435219e..ad2c3e84e2 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -6,6 +6,7 @@ /** * Module dependencies. */ +var util = require('util'); var EventEmitter = require('events').EventEmitter; var Pending = require('./pending'); var utils = require('./utils'); @@ -13,6 +14,9 @@ var inherits = utils.inherits; var debug = require('debug')('mocha:runner'); var Runnable = require('./runnable'); var createStatsCollector = require('./stats-collector'); +var dQuote = utils.dQuote; +var ngettext = utils.ngettext; +var sQuote = utils.sQuote; var stackFilter = utils.stackTraceFilter(); var stringify = utils.stringify; var type = utils.type; @@ -204,13 +208,14 @@ Runner.prototype.checkGlobals = function(test) { leaks = filterLeaks(ok, globals); this._globals = this._globals.concat(leaks); - if (leaks.length > 1) { - this.fail( - test, - new Error('global leaks detected: ' + leaks.join(', ') + '') + if (leaks.length) { + var format = ngettext( + leaks.length, + 'global leak detected: %s', + 'global leaks detected: %s' ); - } else if (leaks.length) { - this.fail(test, new Error('global leak detected: ' + leaks[0])); + var error = new Error(util.format(format, leaks.map(sQuote).join(', '))); + this.fail(test, error); } }; @@ -268,7 +273,7 @@ Runner.prototype.failHook = function(hook, err) { hook.originalTitle = hook.originalTitle || hook.title; if (hook.ctx && hook.ctx.currentTest) { hook.title = - hook.originalTitle + ' for "' + hook.ctx.currentTest.title + '"'; + hook.originalTitle + ' for ' + dQuote(hook.ctx.currentTest.title); } else { var parentTitle; if (hook.parent.title) { @@ -276,7 +281,7 @@ Runner.prototype.failHook = function(hook, err) { } else { parentTitle = hook.parent.root ? '{root}' : ''; } - hook.title = hook.originalTitle + ' in "' + parentTitle + '"'; + hook.title = hook.originalTitle + ' in ' + dQuote(parentTitle); } this.fail(hook, err); From ff04d1951b9336c83e13a5f1884cd9a013c55795 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Mon, 4 Feb 2019 05:34:47 -0600 Subject: [PATCH 03/10] test(unit/runner.spec.js): Update spec for leak detection change --- test/unit/runner.spec.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 0908e95e1e..2d6d05542d 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -103,9 +103,9 @@ describe('Runner', function() { var test = new Test('im a test', noop); runner.checkGlobals(); global.foo = 'bar'; - runner.on('fail', function(_test, err) { + runner.on('fail', function(_test, _err) { expect(_test, 'to be', test); - expect(err.message, 'to be', 'global leak detected: foo'); + expect(_err, 'to have message', "global leak detected: 'foo'"); delete global.foo; done(); }); @@ -157,9 +157,9 @@ describe('Runner', function() { runner.checkGlobals(); global.foo = 'bar'; global.bar = 'baz'; - runner.on('fail', function(_test, err) { + runner.on('fail', function(_test, _err) { expect(_test, 'to be', test); - expect(err.message, 'to be', 'global leaks detected: foo, bar'); + expect(_err, 'to have message', "global leaks detected: 'foo', 'bar'"); delete global.foo; delete global.bar; done(); @@ -191,10 +191,11 @@ describe('Runner', function() { global.foo = 'bar'; global.bar = 'baz'; - runner.on('fail', function(test, err) { - expect(test.title, 'to be', 'im a test about lions'); - expect(err.message, 'to be', 'global leak detected: bar'); + runner.on('fail', function(_test, _err) { + expect(_test.title, 'to be', 'im a test about lions'); + expect(_err, 'to have message', "global leak detected: 'bar'"); delete global.foo; + // :BUG?: 'global.bar' still exists when next test run... done(); }); runner.checkGlobals(test); @@ -206,7 +207,7 @@ describe('Runner', function() { delete global.derp; done(); }); - runner.checkGlobals(new Test('herp', function() {})); + runner.checkGlobals(new Test('herp', noop)); }); }); From 17845e7448ad4354d1acbaf391688ca78a75e290 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 5 Feb 2019 10:24:49 -0600 Subject: [PATCH 04/10] test(unit/runner.spec.js): Fix after rebase Fixed for 'global.derp' test modifications. Removed test global previously uncleaned. --- test/unit/runner.spec.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index e431f575ec..67bd1faf4b 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -194,23 +194,23 @@ describe('Runner', function() { suite.addTest(test); - global.foo = 'bar'; - global.bar = 'baz'; + global.foo = 'whitelisted'; + global.bar = 'detect-me'; runner.on(EVENT_TEST_FAIL, function(_test, _err) { expect(_test.title, 'to be', 'im a test about lions'); expect(_err, 'to have message', "global leak detected: 'bar'"); delete global.foo; - // :BUG?: 'global.bar' still exists when next test run... + delete global.bar; done(); }); runner.checkGlobals(test); }); - it('should emit "fail" when a global beginning with d is introduced', function(done) { + it('should emit "fail" when a global beginning with "d" is introduced', function(done) { global.derp = 'bar'; - runner.on(EVENT_TEST_FAIL, function(test, err) { - expect(test.title, 'to be', 'herp'); - expect(err.message, 'to be', 'global leak detected: derp'); + runner.on(EVENT_TEST_FAIL, function(_test, _err) { + expect(_test.title, 'to be', 'herp'); + expect(_err, 'to have message', "global leak detected: 'derp'"); delete global.derp; done(); }); From 470e562535c293e8c35493b053f2e85b13f5338b Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Thu, 7 Feb 2019 11:19:42 -0600 Subject: [PATCH 05/10] test(unit/runner.spec.js): Fix to emit using constant names --- test/unit/runner.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js index 67bd1faf4b..1adf5a0cbc 100644 --- a/test/unit/runner.spec.js +++ b/test/unit/runner.spec.js @@ -563,7 +563,7 @@ describe('Runner', function() { // Fake stack-trace err.stack = [message].concat(stack).join('\n'); - runner.on('fail', function(_hook, _err) { + runner.on(EVENT_TEST_FAIL, function(_hook, _err) { var filteredErrStack = _err.stack.split('\n').slice(1); expect( filteredErrStack.join('\n'), @@ -583,7 +583,7 @@ describe('Runner', function() { // Fake stack-trace err.stack = [message].concat(stack).join('\n'); - runner.on('fail', function(_hook, _err) { + runner.on(EVENT_TEST_FAIL, function(_hook, _err) { var filteredErrStack = _err.stack.split('\n').slice(-3); expect( filteredErrStack.join('\n'), From 5bb337dde2254013367b4d3cc51ffb8832b9a835 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Thu, 7 Feb 2019 11:22:04 -0600 Subject: [PATCH 06/10] test(integration/reporters.spec.js): Replace inline with call to utils.dQuote Use the 'utils' version of the function. --- test/integration/reporters.spec.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/integration/reporters.spec.js b/test/integration/reporters.spec.js index d44a222324..363b9304c3 100644 --- a/test/integration/reporters.spec.js +++ b/test/integration/reporters.spec.js @@ -5,6 +5,7 @@ var fs = require('fs'); var crypto = require('crypto'); var path = require('path'); var run = require('./helpers').runMocha; +var dQuote = require('./utils').dQuote; describe('reporters', function() { describe('markdown', function() { @@ -213,13 +214,9 @@ describe('reporters', function() { return; } - function dquote(s) { - return '"' + s + '"'; - } - var pattern = '^Error: invalid or unsupported TAP version: ' + - dquote(invalidTapVersion); + dQuote(invalidTapVersion); expect(res, 'to satisfy', { code: 1, output: new RegExp(pattern, 'm') From 36dbfbf01df7943ff553ab09876e43470655fe9b Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Thu, 7 Feb 2019 11:23:21 -0600 Subject: [PATCH 07/10] test(unit/utils.spec.js): Add tests for new methods --- test/unit/utils.spec.js | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/unit/utils.spec.js b/test/unit/utils.spec.js index bd0360362d..970c77c125 100644 --- a/test/unit/utils.spec.js +++ b/test/unit/utils.spec.js @@ -706,6 +706,45 @@ describe('lib/utils', function() { }); }); + describe('sQuote/dQuote', function() { + var str = 'xxx'; + + it('should return its input as string wrapped in single quotes', function() { + var expected = "'xxx'"; + expect(utils.sQuote(str), 'to be', expected); + }); + + it('should return its input as string wrapped in double quotes', function() { + var expected = '"xxx"'; + expect(utils.dQuote(str), 'to be', expected); + }); + }); + + describe('ngettext', function() { + var singular = 'singular'; + var plural = 'plural'; + + it("should return plural string if 'n' is 0", function() { + expect(utils.ngettext(0, singular, plural), 'to be', plural); + }); + + it("should return singular string if 'n' is 1", function() { + expect(utils.ngettext(1, singular, plural), 'to be', singular); + }); + + it("should return plural string if 'n' is greater than 1", function() { + var arr = ['aaa', 'bbb']; + expect(utils.ngettext(arr.length, singular, plural), 'to be', plural); + }); + + it("should return undefined if 'n' is not a non-negative integer", function() { + expect(utils.ngettext('', singular, plural), 'to be undefined'); + expect(utils.ngettext(-1, singular, plural), 'to be undefined'); + expect(utils.ngettext(true, singular, plural), 'to be undefined'); + expect(utils.ngettext({}, singular, plural), 'to be undefined'); + }); + }); + describe('createMap', function() { it('should return an object with a null prototype', function() { expect(Object.getPrototypeOf(utils.createMap()), 'to be', null); From 7ec4f1be00060e87a4c538306d46348dcca8906d Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 04:20:34 -0600 Subject: [PATCH 08/10] refactor(lib/mocha.js): Use sQuote rather than hardcoded inline --- lib/mocha.js | 14 +++++++------- test/unit/mocha.spec.js | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index 9886ded56d..1537196167 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -20,6 +20,7 @@ var createInvalidInterfaceError = errors.createInvalidInterfaceError; var EVENT_FILE_PRE_REQUIRE = Suite.constants.EVENT_FILE_PRE_REQUIRE; var EVENT_FILE_POST_REQUIRE = Suite.constants.EVENT_FILE_POST_REQUIRE; var EVENT_FILE_REQUIRE = Suite.constants.EVENT_FILE_REQUIRE; +var sQuote = utils.sQuote; exports = module.exports = Mocha; @@ -227,24 +228,23 @@ Mocha.prototype.reporter = function(reporter, reporterOptions) { } catch (_err) { _err.code !== 'MODULE_NOT_FOUND' || _err.message.indexOf('Cannot find module') !== -1 - ? console.warn('"' + reporter + '" reporter not found') + ? console.warn(sQuote(reporter) + ' reporter not found') : console.warn( - '"' + - reporter + - '" reporter blew up with error:\n' + + sQuote(reporter) + + ' reporter blew up with error:\n' + err.stack ); } } else { console.warn( - '"' + reporter + '" reporter blew up with error:\n' + err.stack + sQuote(reporter) + ' reporter blew up with error:\n' + err.stack ); } } } if (!_reporter) { throw createInvalidReporterError( - 'invalid reporter "' + reporter + '"', + 'invalid reporter ' + sQuote(reporter), reporter ); } @@ -273,7 +273,7 @@ Mocha.prototype.ui = function(name) { this._ui = require(name); } catch (err) { throw createInvalidInterfaceError( - 'invalid interface "' + name + '"', + 'invalid interface ' + sQuote(name), name ); } diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js index 580533622c..beb5e2b5e0 100644 --- a/test/unit/mocha.spec.js +++ b/test/unit/mocha.spec.js @@ -234,7 +234,7 @@ describe('Mocha', function() { new Mocha(updatedOpts); }; expect(throwError, 'to throw', { - message: 'invalid reporter "invalidReporter"', + message: "invalid reporter 'invalidReporter'", code: 'ERR_MOCHA_INVALID_REPORTER', reporter: 'invalidReporter' }); From dceaf8e49ae9baa6b917cf411cf0c8ca8e6287ac Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 04:25:37 -0600 Subject: [PATCH 09/10] test(integration/reporters.spec.js): Tweak access to dQuote --- test/integration/reporters.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/reporters.spec.js b/test/integration/reporters.spec.js index 363b9304c3..944f94f552 100644 --- a/test/integration/reporters.spec.js +++ b/test/integration/reporters.spec.js @@ -5,7 +5,8 @@ var fs = require('fs'); var crypto = require('crypto'); var path = require('path'); var run = require('./helpers').runMocha; -var dQuote = require('./utils').dQuote; +var utils = require('../../lib/utils'); +var dQuote = utils.dQuote; describe('reporters', function() { describe('markdown', function() { From c7c7214367b65105b879414bf54357b4407de8df Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 8 Feb 2019 04:50:25 -0600 Subject: [PATCH 10/10] refactor(lib/utils.js): Use quotation methods --- lib/utils.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 85fa657593..c637648442 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -13,6 +13,7 @@ var debug = require('debug')('mocha:watch'); var fs = require('fs'); var glob = require('glob'); var path = require('path'); +var util = require('util'); var join = path.join; var he = require('he'); var errors = require('./errors'); @@ -531,7 +532,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { files = glob.sync(filepath); if (!files.length) { throw createNoFilesMatchPatternError( - 'Cannot find any files matching pattern "' + filepath + '"', + 'Cannot find any files matching pattern ' + exports.dQuote(filepath), filepath ); } @@ -565,7 +566,11 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) { } if (!extensions) { throw createMissingArgumentError( - 'Argument "extensions" required when argument "filepath" is a directory', + util.format( + 'Argument %s required when argument %s is a directory', + exports.sQuote('extensions'), + exports.sQuote('filepath') + ), 'extensions', 'array' ); @@ -763,7 +768,7 @@ exports.sQuote = function(str) { * * @description * Provides a simple means of markup for quoting text to be used in output. - * Use this to quote names of datatypes, classes, and strings. + * Use this to quote names of datatypes, classes, pathnames, and strings. * * argument 'value' must be "string" or "number" *