Refactor checkGlobals() error message creation

lib/runner.js

@@ -6,13 +6,17 @@
 /**
  * Module dependencies.
  */
+var util = require('util');
 var EventEmitter = require('events').EventEmitter;
 var Pending = require('./pending');
 var utils = require('./utils');
 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,15 +273,15 @@ 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) {
       parentTitle = hook.parent.title;
     } else {
       parentTitle = hook.parent.root ? '{root}' : '';
     }
-    hook.title = hook.originalTitle + ' in "' + parentTitle + '"';
+    hook.title = hook.originalTitle + ' in ' + dQuote(parentTitle);
   }
 
   this.fail(hook, err);

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.
+ *
+ * <samp>package 'foo' cannot be found</samp>
+ *
+ * @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.
+ *
+ * <samp>argument 'value' must be "string" or "number"</samp>
+ *
+ * @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

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));
     });
   });