Repository: chaijs/chai (js)

Issue:
deep.equal() and deep.include() doesn't work with Error
`deep.equal()` and `deep.include()` doesn't compare the object keys as I expect it to. I found the issue when upgrading to v4 as the `deep.equal()` works fine in v3.5. It stopped working in v4+.

Here is a code to confirm the issue: https://jsfiddle.net/wLzrucae/3/.

I tried adding `error` to the allowed types for `include()` assertion, but it still failed on comparison. Any ideas?



Pull request description:
fix: `.include` to work with all objects
Overly strict type-checking was causing `.include` to reject `Error`
objects and objects with a custom `@@toStringTag`.

Fixes #1009 

Patch:
diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js
index 085d209..1600b2c 100644
--- a/lib/chai/core/assertions.js
+++ b/lib/chai/core/assertions.js
@@ -469,53 +469,18 @@ module.exports = function (chai, _) {
 
   function include (val, msg) {
     if (msg) flag(this, 'message', msg);
-
-    _.expectTypes(this, [
-      'array', 'object', 'string',
-      'map', 'set', 'weakset',
-    ]);
-
+    
     var obj = flag(this, 'object')
-      , objType = _.type(obj).toLowerCase();
-
-    // This block is for asserting a subset of properties in an object.
-    if (objType === 'object') {
-      var props = Object.keys(val)
-        , negate = flag(this, 'negate')
-        , firstErr = null
-        , numErrs = 0;
-
-      props.forEach(function (prop) {
-        var propAssertion = new Assertion(obj);
-        _.transferFlags(this, propAssertion, true);
-        flag(propAssertion, 'lockSsfi', true);
-
-        if (!negate || props.length === 1) {
-          propAssertion.property(prop, val[prop]);
-          return;
-        }
-
-        try {
-          propAssertion.property(prop, val[prop]);
-        } catch (err) {
-          if (!_.checkError.compatibleConstructor(err, AssertionError)) throw err;
-          if (firstErr === null) firstErr = err;
-          numErrs++;
-        }
-      }, this);
-
-      // When validating .not.include with multiple properties, we only want
-      // to throw an assertion error if all of the properties are included,
-      // in which case we throw the first property assertion error that we
-      // encountered.
-      if (negate && props.length > 1 && numErrs === props.length) throw firstErr;
+      , objType = _.type(obj).toLowerCase()
+      , flagMsg = flag(this, 'message')
+      , negate = flag(this, 'negate')
+      , ssfi = flag(this, 'ssfi')
+      , isDeep = flag(this, 'deep')
+      , descriptor = isDeep ? 'deep ' : '';
 
-      return;
-    }
+    flagMsg = flagMsg ? flagMsg + ': ' : '';
 
-    var isDeep = flag(this, 'deep')
-      , descriptor = isDeep ? 'deep ' : ''
-      , included = false;
+    var included = false;
 
     switch (objType) {
       case 'string':
@@ -524,10 +489,6 @@ module.exports = function (chai, _) {
 
       case 'weakset':
         if (isDeep) {
-          var flagMsg = flag(this, 'message')
-            , ssfi = flag(this, 'ssfi');
-          flagMsg = flagMsg ? flagMsg + ': ' : '';
-
           throw new AssertionError(
             flagMsg + 'unable to use .deep.include with WeakSet',
             undefined,
@@ -564,6 +525,53 @@ module.exports = function (chai, _) {
           included = obj.indexOf(val) !== -1;
         }
         break;
+
+      default:
+        // This block is for asserting a subset of properties in an object.
+        // `_.expectTypes` isn't used here because `.include` should work with
+        // objects with a custom `@@toStringTag`.
+        if (val !== Object(val)) {
+          throw new AssertionError(
+            flagMsg + 'object tested must be an array, a map, an object,'
+              + ' a set, a string, or a weakset, but ' + objType + ' given',
+            undefined,
+            ssfi
+          );
+        }
+
+        var props = Object.keys(val)
+          , firstErr = null
+          , numErrs = 0;
+  
+        props.forEach(function (prop) {
+          var propAssertion = new Assertion(obj);
+          _.transferFlags(this, propAssertion, true);
+          flag(propAssertion, 'lockSsfi', true);
+  
+          if (!negate || props.length === 1) {
+            propAssertion.property(prop, val[prop]);
+            return;
+          }
+  
+          try {
+            propAssertion.property(prop, val[prop]);
+          } catch (err) {
+            if (!_.checkError.compatibleConstructor(err, AssertionError)) {
+              throw err;
+            }
+            if (firstErr === null) firstErr = err;
+            numErrs++;
+          }
+        }, this);
+  
+        // When validating .not.include with multiple properties, we only want
+        // to throw an assertion error if all of the properties are included,
+        // in which case we throw the first property assertion error that we
+        // encountered.
+        if (negate && props.length > 1 && numErrs === props.length) {
+          throw firstErr;
+        }
+        return;
     }
 
     // Assert inclusion in collection or substring in a string.


Test patch:
diff --git a/test/assert.js b/test/assert.js
index 5994d88..ab12e29 100644
--- a/test/assert.js
+++ b/test/assert.js
@@ -638,6 +638,17 @@ describe('assert', function () {
     assert.include('', '');
     assert.include([ 1, 2, 3], 3);
 
+    // .include should work with Error objects and objects with a custom
+    // `@@toStringTag`.
+    assert.include(new Error('foo'), {message: 'foo'});
+    if (typeof Symbol !== 'undefined'
+        && typeof Symbol.toStringTag !== 'undefined') {
+      var customObj = {a: 1};
+      customObj[Symbol.toStringTag] = 'foo';
+
+      assert.include(customObj, {a: 1});
+    }
+
     var obj1 = {a: 1}
       , obj2 = {b: 2};
     assert.include([obj1, obj2], obj1);
diff --git a/test/expect.js b/test/expect.js
index c14fdfa..043c734 100644
--- a/test/expect.js
+++ b/test/expect.js
@@ -1884,6 +1884,17 @@ describe('expect', function () {
 
     expect({a: 1}).to.include({'toString': Object.prototype.toString});
 
+    // .include should work with Error objects and objects with a custom
+    // `@@toStringTag`. 
+    expect(new Error('foo')).to.include({message: 'foo'});
+    if (typeof Symbol !== 'undefined'
+        && typeof Symbol.toStringTag !== 'undefined') {
+      var customObj = {a: 1};
+      customObj[Symbol.toStringTag] = 'foo';
+
+      expect(customObj).to.include({a: 1});
+    }
+
     var obj1 = {a: 1}
       , obj2 = {b: 2};
     expect([obj1, obj2]).to.include(obj1);
diff --git a/test/should.js b/test/should.js
index 4ed205a..4585bfc 100644
--- a/test/should.js
+++ b/test/should.js
@@ -1540,6 +1540,17 @@ describe('should', function() {
     ['foo', 'bar'].should.not.include('baz');
     ['foo', 'bar'].should.not.include(1);
 
+    // .include should work with Error objects and objects with a custom
+    // `@@toStringTag`.
+    (new Error('foo')).should.include({message: 'foo'});
+    if (typeof Symbol !== 'undefined'
+        && typeof Symbol.toStringTag !== 'undefined') {
+      var customObj = {a: 1};
+      customObj[Symbol.toStringTag] = 'foo';
+
+      customObj.should.include({a: 1});
+    }
+
     ({a: 1}).should.include({'toString': Object.prototype.toString});
 
     var obj1 = {a: 1}