guard against undefined timeout option in constructor; closes #3813

Signed-off-by: Christopher Hiller <boneskull@boneskull.com>
mochajs · Mar 7, 2019 · ca9eba6 · ca9eba6
1 parent 735161f
commit ca9eba6
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 17 deletions.
diff --git a/lib/mocha.js b/lib/mocha.js
@@ -120,12 +120,15 @@ function Mocha(options) {
     utils.deprecate(
       'enableTimeouts is DEPRECATED and will be removed from a future version of Mocha. Instead, use "timeout: false" to disable timeouts.'
     );
+    if (options.enableTimeouts === false) {
+      this.timeout(0);
+    }
+  }
+
+  // this guard exists because Suite#timeout does not consider `undefined` to be valid input
+  if (typeof options.timeout !== 'undefined') {
+    this.timeout(options.timeout === false ? 0 : options.timeout);
   }
-  this.timeout(
-    options.enableTimeouts === false || options.timeout === false
-      ? 0
-      : options.timeout
-  );
 
   if ('retries' in options) {
     this.retries(options.retries);

diff --git a/lib/suite.js b/lib/suite.js
@@ -114,6 +114,7 @@ Suite.prototype.clone = function() {
  * Set or get timeout `ms` or short-hand such as "2s".
  *
  * @private
+ * @todo Do not attempt to set value if `ms` is undefined
  * @param {number|string} ms
  * @return {Suite|number} for chaining
  */

diff --git a/test/unit/mocha.spec.js b/test/unit/mocha.spec.js
@@ -20,24 +20,49 @@ describe('Mocha', function() {
     beforeEach(function() {
       sandbox.stub(Mocha.prototype, 'useColors').returnsThis();
       sandbox.stub(utils, 'deprecate');
+      sandbox.stub(Mocha.prototype, 'timeout').returnsThis();
     });
 
-    it('should prefer "color" over "useColors"', function() {
-      // eslint-disable-next-line no-new
-      new Mocha({useColors: true, color: false});
-      expect(Mocha.prototype.useColors, 'to have a call satisfying', [false]);
+    describe('when "useColors" option is defined', function() {
+      it('should prefer "color" over "useColors"', function() {
+        // eslint-disable-next-line no-new
+        new Mocha({useColors: true, color: false});
+        expect(Mocha.prototype.useColors, 'to have a call satisfying', [
+          false
+        ]).and('was called once');
+      });
+
+      it('should assign "useColors" to "color"', function() {
+        // eslint-disable-next-line no-new
+        new Mocha({useColors: true});
+        expect(Mocha.prototype.useColors, 'to have a call satisfying', [
+          true
+        ]).and('was called once');
+      });
+
+      it('should call utils.deprecate()', function() {
+        // eslint-disable-next-line no-new
+        new Mocha({useColors: true});
+        expect(utils.deprecate, 'was called once');
+      });
     });
 
-    it('should assign "useColors" to "color"', function() {
-      // eslint-disable-next-line no-new
-      new Mocha({useColors: true});
-      expect(Mocha.prototype.useColors, 'to have a call satisfying', [true]);
+    describe('when "timeout" option is `undefined`', function() {
+      it('should not attempt to set timeout', function() {
+        // eslint-disable-next-line no-new
+        new Mocha({timeout: undefined});
+        expect(Mocha.prototype.timeout, 'was not called');
+      });
     });
 
-    it('should call utils.deprecate()', function() {
-      // eslint-disable-next-line no-new
-      new Mocha({useColors: true});
-      expect(utils.deprecate, 'was called');
+    describe('when "timeout" option is `false`', function() {
+      it('should set a timeout of 0', function() {
+        // eslint-disable-next-line no-new
+        new Mocha({timeout: false});
+        expect(Mocha.prototype.timeout, 'to have a call satisfying', [0]).and(
+          'was called once'
+        );
+      });
     });
   });