Skip to content

Commit

Permalink
fix inaccurate diff output due to post-assertion object mutation (#3075)
Browse files Browse the repository at this point in the history
* have Base generate the strings for error messages immediately instead 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

* re-add stringify during list to appease tests, open for discussion if this is correct behavior
  • Loading branch information
abrady0 authored and boneskull committed Dec 7, 2017
1 parent 709a09e commit 4508386
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 8 deletions.
24 changes: 16 additions & 8 deletions lib/reporters/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ exports.cursor = {
}
};

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

function stringifyDiffObjs (err) {
if (!utils.isString(err.actual) || !utils.isString(err.expected)) {
err.actual = utils.stringify(err.actual);
err.expected = utils.stringify(err.expected);
}
}

/**
* Output the given `failures` as a list.
*
Expand Down Expand Up @@ -183,8 +194,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;

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

if (showDiff(err)) {
stringifyDiffObjs(err);
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 @@ -290,6 +295,9 @@ function Base (runner) {
runner.on('fail', function (test, err) {
stats.failures = stats.failures || 0;
stats.failures++;
if (showDiff(err)) {
stringifyDiffObjs(err);
}
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 4508386

Please sign in to comment.