fix(Suite/Test): untitled suite/test-case #1632

bin/_mocha

@@ -58,7 +58,7 @@ var images = {
 program
   .version(JSON.parse(fs.readFileSync(__dirname + '/../package.json', 'utf8')).version)
   .usage('[debug] [options] [files]')
-  .option('-A, --async-only', "force all tests to take a callback (async)")
+  .option('-A, --async-only', "force all tests to take a callback (async) or return a promise")
   .option('-c, --colors', 'force enabling of colors')
   .option('-C, --no-colors', 'force disabling of colors')
   .option('-G, --growl', 'enable growl notification support')

lib/runnable.js

@@ -74,6 +74,9 @@ Runnable.prototype.timeout = function(ms) {
   if (ms === 0) {
     this._enableTimeouts = false;
   }
+  if (ms > Math.pow(2, 31)) {
+    throw new RangeError('Timeout too large, must be less than 2^31');
+  }
   if (typeof ms === 'string') {
     ms = milliseconds(ms);
   }
@@ -214,6 +217,7 @@ Runnable.prototype.run = function(fn) {
   var ctx = this.ctx;
   var finished;
   var emitted;
+  var result;
 
   // Sometimes the ctx exists, but it is not runnable
   if (ctx && ctx.runnable) {
@@ -256,7 +260,7 @@ Runnable.prototype.run = function(fn) {
     this.resetTimeout();
 
     try {
-      this.fn.call(ctx, function(err) {
+      result = this.fn.call(ctx, function(err) {
         if (err instanceof Error || toString.call(err) === '[object Error]') {
           return done(err);
         }
@@ -268,16 +272,16 @@ Runnable.prototype.run = function(fn) {
         }
         done();
       });
+
+      if (result && typeof result.then === 'function') {
+        return done(new Error('Asynchronous resolution method is overspecified. Specify a callback *or* return a Promise, not both.'));
+      }
     } catch (err) {
       done(utils.getError(err));
     }
     return;
   }
 
-  if (this.asyncOnly) {
-    return done(new Error('--async-only option in use without declaring `done()`'));
-  }
-
   // sync or promise-returning
   try {
     if (this.pending) {
@@ -301,6 +305,10 @@ Runnable.prototype.run = function(fn) {
           done(reason || new Error('Promise rejected with no or falsy reason'));
         });
     } else {
+      if (self.asyncOnly) {
+        return done(new Error('--async-only option in use without declaring `done()` or returning a promise'));
+      }
+
       done();
     }
   }

lib/suite.js

@@ -44,6 +44,9 @@ exports.create = function(parent, title) {
  * @param {Context} parentContext
  */
 function Suite(title, parentContext) {
+  if (!utils.isString(title)) {
+    throw new Error('Suite `title` should be a "string" but "' + typeof title + '" was given instead.');
+  }
   this.title = title;
   function Context() {}
   Context.prototype = parentContext;

lib/test.js

@@ -4,6 +4,7 @@
 
 var Runnable = require('./runnable');
 var create = require('lodash.create');
+var isString = require('./utils').isString;
 
 /**
  * Expose `Test`.
@@ -19,6 +20,9 @@ module.exports = Test;
  * @param {Function} fn
  */
 function Test(title, fn) {
+  if (!isString(title)) {
+    throw new Error('Test `title` should be a "string" but "' + typeof title + '" was given instead.');
+  }
   Runnable.call(this, title, fn);
   this.pending = !fn;
   this.type = 'test';

package.json

@@ -268,8 +268,10 @@
     "_mocha": "./bin/_mocha"
   },
   "engines": {
-    "node": ">= 0.8.x"
+    "node": ">= 0.8.x",
+    "npm": ">=1.4.3"
   },
+  "engineStrict": true,
   "scripts": {
     "test": "make test-all"
   },
@@ -278,7 +280,7 @@
     "debug": "2.0.0",
     "diff": "1.4.0",
     "escape-string-regexp": "1.0.2",
-    "glob": "3.2.3",
+    "glob": "4.0.6",
     "growl": "1.8.1",
     "jade": "0.26.3",
     "lodash.create": "^3.1.1",

test/acceptance/misc/asyncOnly.js

@@ -0,0 +1,28 @@
+describe('asyncOnly', function(){
+  it('should display an error', function(){
+
+  })
+
+  it('should pass', function(done){
+    done();
+  })
+
+  it('should ignore pending tests')
+
+  it('should fail when test throws an error', function(){
+    // the async warning only applies if the test would have otherwise passed
+    throw Error('you should see this error');
+  })
+
+  describe('with a function that returns a promise', function() {
+    it('should pass', function(){
+      var fulfilledPromise = {
+        then: function (fulfilled, rejected) {
+          process.nextTick(fulfilled);
+        }
+      };
+
+      return fulfilledPromise;
+    })
+  })
+})

test/acceptance/overspecified-async.js

@@ -0,0 +1,8 @@
+describe('overspecified asynchronous resolution method', function() {
+  it('should fail when multiple methods are used', function(done) {
+    setTimeout(done, 0);
+
+    // uncomment
+    // return { then: function() {} };
+  });
+});

test/acceptance/throw.js

@@ -7,7 +7,7 @@ describe('a test that throws', function () {
   var suite, runner;
 
   beforeEach(function(){
-    suite = new Suite(null, 'root');
+    suite = new Suite('Suite', 'root');
     runner = new Runner(suite);
   })
 

test/runnable.js

@@ -41,6 +41,13 @@ describe('Runnable(title, fn)', function(){
     })
   })
 
+  describe('#timeout(ms) when ms>2^31', function(){
+    it('should throw an error', function () {
+      var run = new Runnable;
+      run.timeout.bind(run, 1e10).should.throw(/Timeout too large/);
+    });
+  });
+
   describe('#enableTimeouts(enabled)', function(){
     it('should set enabled', function(){
       var run = new Runnable;

test/runner.js

@@ -7,7 +7,7 @@ describe('Runner', function(){
   var suite, runner;
 
   beforeEach(function(){
-    suite = new Suite(null, 'root');
+    suite = new Suite('Suite', 'root');
     runner = new Runner(suite);
   })
 

test/suite.js

@@ -393,4 +393,42 @@ describe('Suite', function(){
     });
 
   });
+
+  describe('initialization', function() {
+    it('should throw an error if the title isn\'t a string', function() {
+      (function() {
+        new Suite(undefined, 'root');
+      }).should.throw();
+
+      (function() {
+        new Suite(function(){}, 'root');
+      }).should.throw();
+    });
+
+    it('should not throw if the title is a string', function() {
+      (function() {
+        new Suite('Bdd suite', 'root');
+      }).should.not.throw();
+    });
+  });
 });
+
+describe('Test', function() {
+  describe('initialization', function() {
+    it('should throw an error if the title isn\'t a string', function() {
+      (function() {
+        new Test(function(){});
+      }).should.throw();
+
+      (function() {
+        new Test(undefined, function(){});
+      }).should.throw();
+    });
+
+    it('should not throw if the title is a string', function() {
+      (function() {
+        new Test('test-case',  function(){});
+      }).should.not.throw();
+    });
+  });
+});