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

ensure --debug becomes --inspect; closes #3697 #3698

Merged
merged 2 commits into from Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion bin/mocha
Expand Up @@ -39,8 +39,9 @@ Object.keys(opts).forEach(opt => {
}
});

// allow --debug to invoke --inspect on Node.js v8 or newer nodeOpts.inspect = childOpts.debug;
// allow --debug to invoke --inspect on Node.js v8 or newer
if (childOpts.debug) {
nodeOpts.inspect = childOpts.debug;
childOpts.timeout = false;
delete childOpts.debug;
debug('--debug -> --inspect');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand where the distinction is made between Node >=v8 and Node v6.
We will deprecate --debug anyway once support of Node v6 is dropped, right? We could prepare this deprecation and pass the responsibility of choosing the correct Node flag to the user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree, we should provide a deprecation message when debug is used outside of Node.js v6.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I changed my mind. a warning should be printed, but it's a nice convenience for users until Node.js v6 drops out of active maintenance. at that point, we can remove it, because we'll drop v6 support as well.

Expand Down
17 changes: 5 additions & 12 deletions test/assertions.js
Expand Up @@ -59,9 +59,9 @@ exports.mixinMochaAssertions = function(expect) {
}
)
.addAssertion(
'<RawRunResult|JSONRunResult> [not] to have completed with [exit] code <number>',
'<RawResult|RawRunResult|JSONRunResult> [not] to have [completed with] [exit] code <number>',
function(expect, result, code) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why those additional "[ ... ]"?

Copy link
Member Author

@boneskull boneskull Jan 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule was duplicated above, so I consolidated it.

So this allows you to say expect <var> to have completed with exit code <code> or expect <var> to have exit code <code> or expect <var> to have completed with code <code>.

I find it's nicer to provide several different ways of expressing the same thing.

expect(result.code, '[not] to be', code);
expect(result, '[not] to have property', 'code', code);
}
)
.addAssertion(
Expand Down Expand Up @@ -290,17 +290,10 @@ exports.mixinMochaAssertions = function(expect) {
});
}
)
.addAssertion('<RawRunResult> [not] to contain output <any>', function(
expect,
result,
output
) {
expect(result.output, '[not] to satisfy', output);
})
.addAssertion(
'<RawRunResult|JSONRunResult> to have [exit] code <number>',
function(expect, result, code) {
expect(result.code, 'to be', code);
'<RawResult|RawRunResult> [not] to contain [output] <any>',
function(expect, result, output) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be "[output]" or "output"? I don't know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are more examples: "test count" or "[test] count"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think [output] and [test]. Feel free to improve these as you see fit

expect(result.output, '[not] to satisfy', output);
}
);
};
9 changes: 9 additions & 0 deletions test/integration/fixtures/__default__.fixture.js
@@ -0,0 +1,9 @@
// this generic fixture does nothing special and will be used if no fixture is supplied

'use strict';

describe('a suite', function() {
it('should succeed', function(done) {
done();
});
});
53 changes: 47 additions & 6 deletions test/integration/helpers.js
Expand Up @@ -5,7 +5,12 @@ var spawn = require('cross-spawn').spawn;
var path = require('path');
var baseReporter = require('../../lib/reporters/base');

var DEFAULT_FIXTURE = resolveFixturePath('__default__');
var MOCHA_EXECUTABLE = require.resolve('../../bin/mocha');
var _MOCHA_EXECUTABLE = require.resolve('../../bin/_mocha');

module.exports = {
DEFAULT_FIXTURE: DEFAULT_FIXTURE,
/**
* Invokes the mocha binary for the given fixture with color output disabled.
* Accepts an array of additional command line args to pass. The callback is
Expand Down Expand Up @@ -177,20 +182,56 @@ function toJSONRunResult(result) {
return result;
}

function invokeMocha(args, fn, opts) {
args = [path.join(__dirname, '..', '..', 'bin', 'mocha')].concat(args);
/**
* Creates arguments loading a default fixture if none provided
*
* @param {string[]|*} [args] - Arguments to `spawn`
* @returns string[]
*/
function defaultArgs(args) {
return !args || !args.length ? ['--file', DEFAULT_FIXTURE] : args;
}

return _spawnMochaWithListeners(args, fn, opts);
function invokeMocha(args, fn, opts) {
if (typeof args === 'function') {
opts = fn;
fn = args;
args = [];
}
return _spawnMochaWithListeners(
defaultArgs([MOCHA_EXECUTABLE].concat(args)),
fn,
opts
);
}

function invokeSubMocha(args, fn, opts) {
args = [path.join(__dirname, '..', '..', 'bin', '_mocha')].concat(args);

return _spawnMochaWithListeners(args, fn, opts);
if (typeof args === 'function') {
opts = fn;
fn = args;
args = [];
}
return _spawnMochaWithListeners(
defaultArgs([_MOCHA_EXECUTABLE].concat(args)),
fn,
opts
);
}

/**
* Spawns Mocha in a subprocess and returns an object containing its output and exit code
*
* @param {string[]} args - Path to executable and arguments
* @param {Function} fn - Callback
* @param {Object|string} [opts] - Options to `cross-spawn`, or 'pipe' for shortcut to `{stdio: pipe}`
* @returns {ChildProcess}
* @private
*/
function _spawnMochaWithListeners(args, fn, opts) {
var output = '';
if (opts === 'pipe') {
opts = {stdio: 'pipe'};
}
opts = Object.assign(
{
cwd: process.cwd(),
Expand Down
32 changes: 32 additions & 0 deletions test/integration/options/debug.spec.js
@@ -0,0 +1,32 @@
'use strict';

var helpers = require('../helpers');
var invokeMocha = helpers.invokeMocha;
var DEFAULT_FIXTURE = helpers.DEFAULT_FIXTURE;

describe('--debug', function() {
describe('Node.js v8+', function() {
before(function() {
if (process.version.substring(0, 2) === 'v6') {
this.skip();
}
});

it('should invoke --inspect', function(done) {
invokeMocha(
['--debug', '--file', DEFAULT_FIXTURE],
function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have passed').and(
'to contain output',
/Debugger listening/i
);
done();
},
{stdio: 'pipe'}
);
});
});
});