From 3a2dae8b360bf3ac58f79530c5b35413dd6ed417 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Sun, 18 Sep 2016 23:22:53 -0700 Subject: [PATCH] helpful error when necessary suite callback omitted; closes #1744 - works for QUnit as well - renamed some files - fixed unclear "only" tests --- lib/interfaces/common.js | 2 + lib/interfaces/qunit.js | 6 +- ...llback.js => suite-no-callback.fixture.js} | 0 .../suite/suite-skipped-callback.fixture.js | 1 + ...s => suite-skipped-no-callback.fixture.js} | 0 test/integration/only.spec.js | 56 +++++++++++-------- test/integration/suite.js | 29 ---------- test/integration/suite.spec.js | 42 ++++++++++++++ 8 files changed, 81 insertions(+), 55 deletions(-) rename test/integration/fixtures/suite/{describe.callback.js => suite-no-callback.fixture.js} (100%) create mode 100644 test/integration/fixtures/suite/suite-skipped-callback.fixture.js rename test/integration/fixtures/suite/{xdescribe.callback.js => suite-skipped-no-callback.fixture.js} (100%) delete mode 100644 test/integration/suite.js create mode 100644 test/integration/suite.spec.js diff --git a/lib/interfaces/common.js b/lib/interfaces/common.js index b367544a3f..447458487b 100644 --- a/lib/interfaces/common.js +++ b/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 --git a/lib/interfaces/qunit.js b/lib/interfaces/qunit.js index a2d67ef90f..22e8080e2e 100644 --- a/lib/interfaces/qunit.js +++ b/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 --git a/test/integration/fixtures/suite/describe.callback.js b/test/integration/fixtures/suite/suite-no-callback.fixture.js similarity index 100% rename from test/integration/fixtures/suite/describe.callback.js rename to test/integration/fixtures/suite/suite-no-callback.fixture.js diff --git a/test/integration/fixtures/suite/suite-skipped-callback.fixture.js b/test/integration/fixtures/suite/suite-skipped-callback.fixture.js new file mode 100644 index 0000000000..b30b67ee2e --- /dev/null +++ b/test/integration/fixtures/suite/suite-skipped-callback.fixture.js @@ -0,0 +1 @@ +xdescribe('a pending suite with a callback', function () {}); diff --git a/test/integration/fixtures/suite/xdescribe.callback.js b/test/integration/fixtures/suite/suite-skipped-no-callback.fixture.js similarity index 100% rename from test/integration/fixtures/suite/xdescribe.callback.js rename to test/integration/fixtures/suite/suite-skipped-no-callback.fixture.js diff --git a/test/integration/only.spec.js b/test/integration/only.spec.js index 592ea17ec2..61254ce1d3 100644 --- a/test/integration/only.spec.js +++ b/test/integration/only.spec.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 --git a/test/integration/suite.js b/test/integration/suite.js deleted file mode 100644 index 369f08a7ba..0000000000 --- a/test/integration/suite.js +++ /dev/null @@ -1,29 +0,0 @@ -var assert = require('assert'); -var run = require('./helpers').runMocha; -var args = []; - -describe('.describe()', function() { - this.timeout(1000); - it('should throw a helpful error message when a callback for describe is not supplied', function(done) { - run('suite/describe.callback.js', args, function(err, res) { - assert(!err); - pattern = new RegExp('TypeError: a callback is not supplied', 'g'); - var result = res.output.match(pattern) || []; - assert.equal(result.length, 1); - done(); - }); - }); -}); - -describe('.xdescribe()', function() { - this.timeout(1000); - it('should not throw an error when a callback for xdescribe is not supplied', function(done) { - run('suite/xdescribe.callback.js', args, function(err, res) { - assert(!err); - pattern = new RegExp("Error", 'g'); - var result = res.output.match(pattern) || []; - assert.equal(result.length, 0); - done(); - }); - }); -}); diff --git a/test/integration/suite.spec.js b/test/integration/suite.spec.js new file mode 100644 index 0000000000..00066a34f4 --- /dev/null +++ b/test/integration/suite.spec.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(); + }); + }); +});