helpful error when necessary suite callback omitted; closes #1744

- works for QUnit as well
- renamed some files
- fixed unclear "only" tests

Author: boneskull

Diff for: lib/interfaces/common.js

@@ -113,6 +113,8 @@ module.exports = function(suites, context, mocha) {
         if (typeof opts.fn === 'function') {
           opts.fn.call(suite);
           suites.shift();
+        } else if (typeof opts.fn === 'undefined' && !suite.pending) {
+          throw new Error('Suite "' + suite.fullTitle() + '" was defined but no callback was supplied. Supply a callback or explicitly skip the suite.');
         }
 
         return suite;

Diff for: lib/interfaces/qunit.js

@@ -50,7 +50,8 @@ module.exports = function(suite) {
       }
       return common.suite.create({
         title: title,
-        file: file
+        file: file,
+        fn: false
       });
     };
 
@@ -64,7 +65,8 @@ module.exports = function(suite) {
       }
       return common.suite.only({
         title: title,
-        file: file
+        file: file,
+        fn: false
       });
     };
 

Diff for: test/integration/fixtures/suite/describe.callback.js renamed to test/integration/fixtures/suite/suite-no-callback.fixture.js

@@ -0,0 +1 @@
+xdescribe('a pending suite with a callback', function () {});

Diff for: test/integration/fixtures/suite/suite-skipped-callback.fixture.js

@@ -2,36 +2,44 @@ var run = require('./helpers').runMochaJSON;
 var assert = require('assert');
 
 describe('.only()', function() {
-  it('should run only tests that marked as `only`', function(done) {
-    run('options/only/bdd.fixture.js', ['--ui', 'bdd'], function(err, res) {
-      assert(!err);
-      assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 11);
-      assert.equal(res.stats.failures, 0);
-      assert.equal(res.code, 0);
-      done();
+  describe('bdd', function() {
+    it('should run only tests that marked as `only`', function(done) {
+      run('options/only/bdd.fixture.js', ['--ui', 'bdd'], function(err, res) {
+        assert(!err);
+        assert.equal(res.stats.pending, 0);
+        assert.equal(res.stats.passes, 11);
+        assert.equal(res.stats.failures, 0);
+        assert.equal(res.code, 0);
+        done();
+      });
     });
   });
 
-  it('should run only tests that marked as `only`', function(done) {
-    run('options/only/tdd.fixture.js', ['--ui', 'tdd'], function(err, res) {
-      assert(!err);
-      assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 8);
-      assert.equal(res.stats.failures, 0);
-      assert.equal(res.code, 0);
-      done();
+  describe('tdd', function() {
+    it('should run only tests that marked as `only`', function(done) {
+      run('options/only/tdd.fixture.js', ['--ui', 'tdd'], function(err, res) {
+        assert(!err);
+        assert.equal(res.stats.pending, 0);
+        assert.equal(res.stats.passes, 8);
+        assert.equal(res.stats.failures, 0);
+        assert.equal(res.code, 0);
+        done();
+      });
     });
   });
 
-  it('should run only tests that marked as `only`', function(done) {
-    run('options/only/qunit.fixture.js', ['--ui', 'qunit'], function(err, res) {
-      assert(!err);
-      assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 5);
-      assert.equal(res.stats.failures, 0);
-      assert.equal(res.code, 0);
-      done();
+  describe('qunit', function() {
+    it('should run only tests that marked as `only`', function(done) {
+      run('options/only/qunit.fixture.js', ['--ui', 'qunit'], function(err, res) {
+        console.log(err);
+
+        assert(!err);
+        assert.equal(res.stats.pending, 0);
+        assert.equal(res.stats.passes, 5);
+        assert.equal(res.stats.failures, 0);
+        assert.equal(res.code, 0);
+        done();
+      });
     });
   });
 });

Diff for: test/integration/fixtures/suite/xdescribe.callback.js renamed to test/integration/fixtures/suite/suite-skipped-no-callback.fixture.js

@@ -0,0 +1,42 @@
+var assert = require('assert');
+var run = require('./helpers').runMocha;
+var args = [];
+
+describe('suite w/no callback', function() {
+  this.timeout(1000);
+  it('should throw a helpful error message when a callback for suite is not supplied', function(done) {
+    run('suite/suite-no-callback.fixture.js', args, function(err, res) {
+      assert(!err);
+      var result = res.output.match(/no callback was supplied/) || [];
+      assert.equal(result.length, 1);
+      done();
+    });
+  });
+});
+
+describe('skipped suite w/no callback', function() {
+  this.timeout(1000);
+  it('should not throw an error when a callback for skipped suite is not supplied', function(done) {
+    run('suite/suite-skipped-no-callback.fixture.js', args, function(err, res) {
+      assert(!err);
+      pattern = new RegExp("Error", 'g');
+      var result = res.output.match(pattern) || [];
+      assert.equal(result.length, 0);
+      done();
+    });
+  });
+});
+
+
+describe('skipped suite w/ callback', function() {
+  this.timeout(1000);
+  it('should not throw an error when a callback for skipped suite is supplied', function(done) {
+    run('suite/suite-skipped-callback.fixture.js', args, function(err, res) {
+      assert(!err);
+      pattern = new RegExp("Error", 'g');
+      var result = res.output.match(pattern) || [];
+      assert.equal(result.length, 0);
+      done();
+    });
+  });
+});