Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn that suites cannot return a value #3550

Merged
merged 15 commits into from Nov 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions lib/interfaces/common.js
@@ -1,6 +1,7 @@
'use strict';

var Suite = require('../suite');
var utils = require('../utils');

/**
* Functions common to more than one interface.
Expand Down Expand Up @@ -92,6 +93,7 @@ module.exports = function(suites, context, mocha) {

/**
* Creates a suite.
*
* @param {Object} opts Options
* @param {string} opts.title Title of Suite
* @param {Function} [opts.fn] Suite Function (not always applicable)
Expand All @@ -109,13 +111,22 @@ module.exports = function(suites, context, mocha) {
suite.parent._onlySuites = suite.parent._onlySuites.concat(suite);
}
if (typeof opts.fn === 'function') {
opts.fn.call(suite);
var result = opts.fn.call(suite);
if (typeof result !== 'undefined') {
utils.deprecate(
'Deprecation Warning: Suites do not support a return value;' +
opts.title +
' returned :' +
result
);
}
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.'
'" was defined but no callback was supplied. ' +
'Supply a callback or explicitly skip the suite.'
);
} else if (!opts.fn && suite.pending) {
suites.shift();
Expand Down
28 changes: 28 additions & 0 deletions lib/utils.js
Expand Up @@ -584,6 +584,34 @@ exports.getError = function(err) {
return err || exports.undefinedError();
};

/**
* Show a deprecation warning. Each distinct message is only displayed once.
*
* @param {string} msg
*/
exports.deprecate = function(msg) {
msg = String(msg);
if (seenDeprecationMsg.hasOwnProperty(msg)) {
return;
}
seenDeprecationMsg[msg] = true;
doDeprecationWarning(msg);
};

var seenDeprecationMsg = {};

var doDeprecationWarning = process.emitWarning
? function(msg) {
// Node.js v6+
process.emitWarning(msg, 'DeprecationWarning');
}
: function(msg) {
// Everything else
process.nextTick(function() {
console.warn(msg);
});
};

/**
* @summary
* This Filter based on `mocha-clean` module.(see: `github.com/rstacruz/mocha-clean`)
Expand Down
18 changes: 18 additions & 0 deletions test/integration/deprecate.spec.js
@@ -0,0 +1,18 @@
'use strict';

var assert = require('assert');
var run = require('./helpers').runMocha;
var args = [];

describe('utils.deprecate test', function() {
it('should print unique deprecation only once', function(done) {
run('deprecate.fixture.js', args, function(err, res) {
if (err) {
return done(err);
}
var result = res.output.match(/deprecated thing/g) || [];
assert.equal(result.length, 2);
done();
});
});
});
9 changes: 9 additions & 0 deletions test/integration/fixtures/deprecate.fixture.js
@@ -0,0 +1,9 @@
'use strict';

var utils = require("../../../lib/utils");

it('consolidates identical calls to deprecate', function() {
utils.deprecate("suite foo did a deprecated thing");
utils.deprecate("suite foo did a deprecated thing");
utils.deprecate("suite bar did a deprecated thing");
});
@@ -0,0 +1,5 @@
'use strict';

describe('a suite returning a value', function () {
return Promise.resolve();
});
24 changes: 18 additions & 6 deletions test/integration/suite.spec.js
Expand Up @@ -9,8 +9,7 @@ describe('suite w/no callback', function() {
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) {
if (err) {
done(err);
return;
return done(err);
}
var result = res.output.match(/no callback was supplied/) || [];
assert.equal(result.length, 1);
Expand All @@ -24,8 +23,7 @@ describe('skipped suite w/no callback', function() {
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) {
if (err) {
done(err);
return;
return done(err);
}
var pattern = new RegExp('Error', 'g');
var result = res.output.match(pattern) || [];
Expand All @@ -40,8 +38,7 @@ describe('skipped suite w/ callback', function() {
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) {
if (err) {
done(err);
return;
return done(err);
}
var pattern = new RegExp('Error', 'g');
var result = res.output.match(pattern) || [];
Expand All @@ -50,3 +47,18 @@ describe('skipped suite w/ callback', function() {
});
});
});

describe('suite returning a value', function() {
this.timeout(2000);
it('should give a deprecation warning for suite callback returning a value', function(done) {
run('suite/suite-returning-value.fixture.js', args, function(err, res) {
if (err) {
return done(err);
}
var pattern = new RegExp('Deprecation Warning', 'g');
var result = res.output.match(pattern) || [];
assert.equal(result.length, 1);
done();
});
});
});