From 877090427d687f94741b5ebe3e05e5a6edd76dff Mon Sep 17 00:00:00 2001
From: Aaron Brady <aaron@mori.com>
Date: Wed, 6 Dec 2017 17:39:53 -0800
Subject: [PATCH] fix inaccurate diff output due to post-assertion object
 mutation  (#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
---
 lib/reporters/base.js       | 24 ++++++++++++++++--------
 test/reporters/list.spec.js | 22 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/lib/reporters/base.js b/lib/reporters/base.js
index 8b68a3d51d..0ebc5e7db3 100644
--- a/lib/reporters/base.js
+++ b/lib/reporters/base.js
@@ -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.
  *
@@ -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;
@@ -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);
@@ -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);
   });
diff --git a/test/reporters/list.spec.js b/test/reporters/list.spec.js
index 4b3704204d..87978de011 100644
--- a/test/reporters/list.spec.js
+++ b/test/reporters/list.spec.js
@@ -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 () {