Skip to content

Commit

Permalink
have Base generate the strings for error messages immediately instead…
Browse files Browse the repository at this point in the history
… of delaying until epilogue, the reason for this is that it is possible to reference objects in errors that could be mutated after the error occurs, causing the printed error message to be misleading
  • Loading branch information
abrady0 committed Oct 20, 2017
1 parent da901da commit 09226eb
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
19 changes: 11 additions & 8 deletions lib/reporters/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ exports.cursor = {
}
};

function showDiff(err) {
return err && err.showDiff !== false && sameType(err.actual, err.expected) && err.expected !== undefined;
}

/**
* Output the given `failures` as a list.
*
Expand Down Expand Up @@ -183,8 +187,6 @@ exports.list = function (failures) {
}
var stack = err.stack || message;
var index = message ? stack.indexOf(message) : -1;
var actual = err.actual;
var expected = err.expected;
var escape = true;

if (index === -1) {
Expand All @@ -201,13 +203,8 @@ exports.list = function (failures) {
msg = 'Uncaught ' + msg;
}
// explicitly show diff
if (err.showDiff !== false && sameType(actual, expected) && expected !== undefined) {
if (showDiff(err)) {
escape = false;
if (!(utils.isString(actual) && utils.isString(expected))) {
err.actual = actual = utils.stringify(actual);
err.expected = expected = utils.stringify(expected);
}

fmt = color('error title', ' %s) %s:\n%s') + color('error stack', '\n%s\n');
var match = message.match(/^([^:]+): expected/);
msg = '\n ' + color('error message', match ? match[1] : msg);
Expand Down Expand Up @@ -292,6 +289,12 @@ function Base (runner) {
runner.on('fail', function (test, err) {
stats.failures = stats.failures || 0;
stats.failures++;
if (showDiff(err)) {
if (!utils.isString(err.actual) || !utils.isString(err.expected)) {
err.actual = utils.stringify(err.actual);
err.expected = utils.stringify(err.expected);
}
}
test.err = err;
failures.push(test);
});
Expand Down
22 changes: 22 additions & 0 deletions test/reporters/list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,28 @@ describe('List reporter', function () {

Base.cursor = cachedCursor;
});
it('should immediately construct fail strings', function() {
var actual = { a: 'actual' };
var expected = { a: 'expected' };
var test = {};
var checked = false;
var err;
runner.on = function (event, callback) {
if (!checked && event === 'fail') {
err = new Error('fake failure object with actual/expected');
err.actual = actual;
err.expected = expected;
err.showDiff = true;
callback(test, err);
checked = true;
}
};
List.call({epilogue: function () {}}, runner);

process.stdout.write = stdoutWrite;
expect(typeof err.actual).to.equal('string');
expect(typeof err.expected).to.equal('string');
});
});

describe('on end', function () {
Expand Down

0 comments on commit 09226eb

Please sign in to comment.