Skip to content
This repository was archived by the owner on Dec 4, 2023. It is now read-only.

Commit 8770904

Browse files
abrady0boneskull
authored andcommitted
fix inaccurate diff output due to post-assertion object mutation (mochajs#3075)
* 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
1 parent 81fc405 commit 8770904

File tree

2 files changed

+38
-8
lines changed

2 files changed

+38
-8
lines changed

lib/reporters/base.js

+16-8
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,17 @@ exports.cursor = {
155155
}
156156
};
157157

158+
function showDiff (err) {
159+
return err && err.showDiff !== false && sameType(err.actual, err.expected) && err.expected !== undefined;
160+
}
161+
162+
function stringifyDiffObjs (err) {
163+
if (!utils.isString(err.actual) || !utils.isString(err.expected)) {
164+
err.actual = utils.stringify(err.actual);
165+
err.expected = utils.stringify(err.expected);
166+
}
167+
}
168+
158169
/**
159170
* Output the given `failures` as a list.
160171
*
@@ -183,8 +194,6 @@ exports.list = function (failures) {
183194
}
184195
var stack = err.stack || message;
185196
var index = message ? stack.indexOf(message) : -1;
186-
var actual = err.actual;
187-
var expected = err.expected;
188197

189198
if (index === -1) {
190199
msg = message;
@@ -200,12 +209,8 @@ exports.list = function (failures) {
200209
msg = 'Uncaught ' + msg;
201210
}
202211
// explicitly show diff
203-
if (err.showDiff !== false && sameType(actual, expected) && expected !== undefined) {
204-
if (!(utils.isString(actual) && utils.isString(expected))) {
205-
err.actual = actual = utils.stringify(actual);
206-
err.expected = expected = utils.stringify(expected);
207-
}
208-
212+
if (showDiff(err)) {
213+
stringifyDiffObjs(err);
209214
fmt = color('error title', ' %s) %s:\n%s') + color('error stack', '\n%s\n');
210215
var match = message.match(/^([^:]+): expected/);
211216
msg = '\n ' + color('error message', match ? match[1] : msg);
@@ -290,6 +295,9 @@ function Base (runner) {
290295
runner.on('fail', function (test, err) {
291296
stats.failures = stats.failures || 0;
292297
stats.failures++;
298+
if (showDiff(err)) {
299+
stringifyDiffObjs(err);
300+
}
293301
test.err = err;
294302
failures.push(test);
295303
});

test/reporters/list.spec.js

+22
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,28 @@ describe('List reporter', function () {
192192

193193
Base.cursor = cachedCursor;
194194
});
195+
it('should immediately construct fail strings', function () {
196+
var actual = { a: 'actual' };
197+
var expected = { a: 'expected' };
198+
var test = {};
199+
var checked = false;
200+
var err;
201+
runner.on = function (event, callback) {
202+
if (!checked && event === 'fail') {
203+
err = new Error('fake failure object with actual/expected');
204+
err.actual = actual;
205+
err.expected = expected;
206+
err.showDiff = true;
207+
callback(test, err);
208+
checked = true;
209+
}
210+
};
211+
List.call({epilogue: function () {}}, runner);
212+
213+
process.stdout.write = stdoutWrite;
214+
expect(typeof err.actual).to.equal('string');
215+
expect(typeof err.expected).to.equal('string');
216+
});
195217
});
196218

197219
describe('on end', function () {

0 commit comments

Comments
 (0)