8000 Warn that suites cannot return a value by plroebuck · Pull Request #3550 · mochajs/mocha · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

Warn that suites cannot return a value #3550

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

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions lib/interfaces/common.js
Original 8000 file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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");
});
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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();
});
});
});
0