Repository: mochajs/mocha

Problem statement:
Objects prematurely coerced into string primitives
This is regarding an issue I reported on the WebStorm tracker: https://youtrack.jetbrains.com/issue/WEB-23383

Differences between symbols that are not strictly equal, appear as being equal and are thus hard to read and confusing.

Here is a simplified version of a test where I noticed this behavior:

``` js
"use strict";

const chai           = require( "chai" );
const expect         = chai.expect;

chai.config.truncateThreshold = 0;

describe( "Strings", () => {
    it( "should correctly display differences", () => {
        expect( new String( "foo" ) ).to.equal( "foo" );
    } );
} );
```

Because `new String( "foo" )` is not equal to the primitive `"foo"`, the test will correctly fail. However, in the resulting output, the difference is not immediately apparent.

In this simple test, the `AssertionError` _will_ pretty clearly identify the issue. However, in larger objects, this can be much harder to identify, _especially_ if the truncation threshold is not raised.



Hints:
How assertion libraries generate error messages is outside of the scope of Mocha. I suggest taking this up with chai or the relevant chai plugin you might be using

@Munter Apparently the behavior is related to https://github.com/mochajs/mocha/blob/master/lib/utils.js#L435 

Also, @boneskull suggested to post this here

Quite reproducible:

``` js
it('should foo', function () {
    var err = new Error('oh no');
    err.actual = new String('foo');
    err.expected = 'foo';
    throw err;
});
```

Output:

```
$ mocha foo.js 


  1) should foo

  0 passing (8ms)
  1 failing

  1)  should foo:

      Error: oh no
      + expected - actual


      at Context.<anonymous> (foo.js:2:15)
```

@Munter Yeah, the problem seems to be in our diffing code; Chai actually outputs the difference correctly.  See [original issue](https://youtrack.jetbrains.com/issue/WEB-23383).


Patch:
diff --git a/lib/utils.js b/lib/utils.js
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -89,7 +89,7 @@ exports.map = function(arr, fn, scope) {
  * @param {number} start
  * @return {number}
  */
-exports.indexOf = function(arr, obj, start) {
+var indexOf = exports.indexOf = function(arr, obj, start) {
   for (var i = start || 0, l = arr.length; i < l; i++) {
     if (arr[i] === obj) {
       return i;
@@ -107,7 +107,7 @@ exports.indexOf = function(arr, obj, start) {
  * @param {Object} val Initial value.
  * @return {*}
  */
-exports.reduce = function(arr, fn, val) {
+var reduce = exports.reduce = function(arr, fn, val) {
   var rval = val;
 
   for (var i = 0, l = arr.length; i < l; i++) {
@@ -313,7 +313,7 @@ exports.trim = function(str) {
  * @return {Object}
  */
 exports.parseQuery = function(qs) {
-  return exports.reduce(qs.replace('?', '').split('&'), function(obj, pair) {
+  return reduce(qs.replace('?', '').split('&'), function(obj, pair) {
     var i = pair.indexOf('=');
     var key = pair.slice(0, i);
     var val = pair.slice(++i);
@@ -366,13 +366,11 @@ exports.highlightTags = function(name) {
  *
  * @api private
  * @param {*} value The value to inspect.
- * @param {string} [type] The type of the value, if known.
+ * @param {string} typeHint The type of the value
  * @returns {string}
  */
-function emptyRepresentation(value, type) {
-  type = type || exports.type(value);
-
-  switch (type) {
+function emptyRepresentation(value, typeHint) {
+  switch (typeHint) {
     case 'function':
       return '[Function]';
     case 'object':
@@ -391,7 +389,7 @@ function emptyRepresentation(value, type) {
  * @api private
  * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/toString
  * @param {*} value The value to test.
- * @returns {string}
+ * @returns {string} Computed type
  * @example
  * type({}) // 'object'
  * type([]) // 'array'
@@ -403,8 +401,9 @@ function emptyRepresentation(value, type) {
  * type(/foo/) // 'regexp'
  * type('type') // 'string'
  * type(global) // 'global'
+ * type(new String('foo') // 'object'
  */
-exports.type = function type(value) {
+var type = exports.type = function type(value) {
   if (value === undefined) {
     return 'undefined';
   } else if (value === null) {
@@ -433,25 +432,36 @@ exports.type = function type(value) {
  * @return {string}
  */
 exports.stringify = function(value) {
-  var type = exports.type(value);
+  var typeHint = type(value);
+
+  if (!~indexOf(['object', 'array', 'function'], typeHint)) {
+    if (typeHint === 'buffer') {
+      var json = value.toJSON();
+      // Based on the toJSON result
+      return jsonStringify(json.data && json.type ? json.data : json, 2)
+        .replace(/,(\n|$)/g, '$1');
+    }
 
-  if (!~exports.indexOf(['object', 'array', 'function'], type)) {
-    if (type !== 'buffer') {
+    // IE7/IE8 has a bizarre String constructor; needs to be coerced
+    // into an array and back to obj.
+    if (typeHint === 'string' && typeof value === 'object') {
+      value = reduce(value.split(''), function(acc, char, idx) {
+        acc[idx] = char;
+        return acc;
+      }, {});
+      typeHint = 'object';
+    } else {
       return jsonStringify(value);
     }
-    var json = value.toJSON();
-    // Based on the toJSON result
-    return jsonStringify(json.data && json.type ? json.data : json, 2)
-      .replace(/,(\n|$)/g, '$1');
   }
 
   for (var prop in value) {
     if (Object.prototype.hasOwnProperty.call(value, prop)) {
-      return jsonStringify(exports.canonicalize(value), 2).replace(/,(\n|$)/g, '$1');
+      return jsonStringify(exports.canonicalize(value, null, typeHint), 2).replace(/,(\n|$)/g, '$1');
     }
   }
 
-  return emptyRepresentation(value, type);
+  return emptyRepresentation(value, typeHint);
 };
 
 /**
@@ -480,7 +490,7 @@ function jsonStringify(object, spaces, depth) {
   }
 
   function _stringify(val) {
-    switch (exports.type(val)) {
+    switch (type(val)) {
       case 'null':
       case 'undefined':
         val = '[' + val + ']';
@@ -563,14 +573,15 @@ exports.isBuffer = function(value) {
  * @see {@link exports.stringify}
  * @param {*} value Thing to inspect.  May or may not have properties.
  * @param {Array} [stack=[]] Stack of seen values
+ * @param {string} [typeHint] Type hint
  * @return {(Object|Array|Function|string|undefined)}
  */
-exports.canonicalize = function(value, stack) {
+exports.canonicalize = function canonicalize(value, stack, typeHint) {
   var canonicalizedObj;
   /* eslint-disable no-unused-vars */
   var prop;
   /* eslint-enable no-unused-vars */
-  var type = exports.type(value);
+  typeHint = typeHint || type(value);
   function withStack(value, fn) {
     stack.push(value);
     fn();
@@ -579,11 +590,11 @@ exports.canonicalize = function(value, stack) {
 
   stack = stack || [];
 
-  if (exports.indexOf(stack, value) !== -1) {
+  if (indexOf(stack, value) !== -1) {
     return '[Circular]';
   }
 
-  switch (type) {
+  switch (typeHint) {
     case 'undefined':
     case 'buffer':
     case 'null':
@@ -604,7 +615,7 @@ exports.canonicalize = function(value, stack) {
       }
       /* eslint-enable guard-for-in */
       if (!canonicalizedObj) {
-        canonicalizedObj = emptyRepresentation(value, type);
+        canonicalizedObj = emptyRepresentation(value, typeHint);
         break;
       }
     /* falls through */
@@ -745,7 +756,7 @@ exports.stackTraceFilter = function() {
   return function(stack) {
     stack = stack.split('\n');
 
-    stack = exports.reduce(stack, function(list, line) {
+    stack = reduce(stack, function(list, line) {
       if (isMochaInternal(line)) {
         return list;
       }


Test patch:
diff --git a/test/acceptance/utils.spec.js b/test/acceptance/utils.spec.js
--- a/test/acceptance/utils.spec.js
+++ b/test/acceptance/utils.spec.js
@@ -77,6 +77,10 @@ describe('lib/utils', function () {
 
     var stringify = utils.stringify;
 
+    it('should return an object representation of a string created with a String constructor', function() {
+      expect(stringify(new String('foo'))).to.equal('{\n  "0": "f"\n  "1": "o"\n  "2": "o"\n}');
+    });
+
     it('should return Buffer with .toJSON representation', function() {
       expect(stringify(new Buffer([0x01]))).to.equal('[\n  1\n]');
       expect(stringify(new Buffer([0x01, 0x02]))).to.equal('[\n  1\n  2\n]');