Repository: unexpectedjs/unexpected
Pull request #508 (2018-09-05 18:39:23)

Revert "Merge pull request #482 from alexjeffburke/feature/includeNonEnumerableProperties"
Reverts https://github.com/unexpectedjs/unexpected/pull/482 to push it on a v11 branch.

This reverts commit 4986d6a530a3e65003c7776c929cd038304ffd7d, reversing
changes made to 5884c76fdcd711a2ba4fd1ba5667c15fd60a9e5f.


Patch:
diff --git a/documentation/api/addType.md b/documentation/api/addType.md
index 9f858232..6e02de1d 100644
--- a/documentation/api/addType.md
+++ b/documentation/api/addType.md
@@ -299,13 +299,13 @@ block. The outputs below shows the contrast between setting the
 inlineDiff = true;
 expect(
   {
-    JohnDoe: new Person('John Doe', 42),
-    JaneDoe: new Person('Janie Doe', 24)
+    'John Doe': new Person('John Doe', 42),
+    'Jane Doe': new Person('Janie Doe', 24)
   },
   'to equal',
   {
-    JohnDoe: new Person('John Doe', 42),
-    JaneDoe: new Person('Jane Doe', 24)
+    'John Doe': new Person('John Doe', 42),
+    'Jane Doe': new Person('Jane Doe', 24)
   }
 );
 ```
@@ -313,18 +313,18 @@ expect(
 ```output
 expected
 {
-  JohnDoe: new Person('John Doe', 42),
-  JaneDoe: new Person('Janie Doe', 24)
+  'John Doe': new Person('John Doe', 42),
+  'Jane Doe': new Person('Janie Doe', 24)
 }
 to equal
 {
-  JohnDoe: new Person('John Doe', 42),
-  JaneDoe: new Person('Jane Doe', 24)
+  'John Doe': new Person('John Doe', 42),
+  'Jane Doe': new Person('Jane Doe', 24)
 }
 
 {
-  JohnDoe: new Person('John Doe', 42),
-  JaneDoe: new Person(
+  'John Doe': new Person('John Doe', 42),
+  'Jane Doe': new Person(
     'Janie Doe', // should be 'Jane Doe'
                  // -Janie Doe
                  // +Jane Doe
@@ -337,13 +337,13 @@ to equal
 inlineDiff = false;
 expect(
   {
-    JohnDoe: new Person('John Doe', 42),
-    JaneDoe: new Person('Janie Doe', 24)
+    'John Doe': new Person('John Doe', 42),
+    'Jane Doe': new Person('Janie Doe', 24)
   },
   'to equal',
   {
-    JohnDoe: new Person('John Doe', 42),
-    JaneDoe: new Person('Jane Doe', 24)
+    'John Doe': new Person('John Doe', 42),
+    'Jane Doe': new Person('Jane Doe', 24)
   }
 );
 ```
@@ -351,25 +351,25 @@ expect(
 ```output
 expected
 {
-  JohnDoe: new Person('John Doe', 42),
-  JaneDoe: new Person('Janie Doe', 24)
+  'John Doe': new Person('John Doe', 42),
+  'Jane Doe': new Person('Janie Doe', 24)
 }
 to equal
 {
-  JohnDoe: new Person('John Doe', 42),
-  JaneDoe: new Person('Jane Doe', 24)
+  'John Doe': new Person('John Doe', 42),
+  'Jane Doe': new Person('Jane Doe', 24)
 }
 
 {
-  JohnDoe: new Person('John Doe', 42),
-  JaneDoe: new Person('Janie Doe', 24) // should equal new Person('Jane Doe', 24)
-                                       //
-                                       // new Person(
-                                       //   'Janie Doe', // should be 'Jane Doe'
-                                       //                // -Janie Doe
-                                       //                // +Jane Doe
-                                       //   24
-                                       // )
+  'John Doe': new Person('John Doe', 42),
+  'Jane Doe': new Person('Janie Doe', 24) // should equal new Person('Jane Doe', 24)
+                                          //
+                                          // new Person(
+                                          //   'Janie Doe', // should be 'Jane Doe'
+                                          //                // -Janie Doe
+                                          //                // +Jane Doe
+                                          //   24
+                                          // )
 }
 ```
 
diff --git a/lib/assertions.js b/lib/assertions.js
index 58770465..8e356b8b 100644
--- a/lib/assertions.js
+++ b/lib/assertions.js
@@ -1799,35 +1799,27 @@ module.exports = expect => {
       if (valueType.is('array-like') && !subjectIsArrayLike) {
         expect.fail();
       }
-
+      const promiseByKey = {};
+      const keys = valueType.getKeys(value);
       const subjectKeys = subjectType.getKeys(subject);
-      const valueKeys = valueType.getKeys(value);
-      // calculate the unique keys early given enumerability no
-      // longer affects what is included in the list of keys
-      const uniqueKeys = subjectType.uniqueKeys(subjectKeys, valueKeys);
 
-      const promiseByKey = {};
-      let forceExhaustivelyComparison = false;
-      uniqueKeys.forEach((key, index) => {
-        const subjectKey = subjectType.valueForKey(subject, key);
-        const valueKey = valueType.valueForKey(value, key);
-        const valueKeyType = expect.findTypeOf(valueKey);
-        const isDefinedSubjectKey = typeof subjectKey !== 'undefined';
-        const isDefinedValueKey = typeof valueKey !== 'undefined';
-
-        if (expect.flags.exhaustively) {
-          if (valueKeyType.is('expect.it') && !isDefinedSubjectKey) {
-            // ensure value only expect.it key is marked missing
-            forceExhaustivelyComparison = true;
-          }
-        } else {
-          if (isDefinedSubjectKey && !isDefinedValueKey) {
-            // ignore subject only keys unless we are being exhaustive
-            return;
+      if (!subjectIsArrayLike) {
+        // Find all non-enumerable subject keys present in value, but not returned by subjectType.getKeys:
+        keys.forEach(key => {
+          if (
+            Object.prototype.hasOwnProperty.call(subject, key) &&
+            subjectKeys.indexOf(key) === -1
+          ) {
+            subjectKeys.push(key);
           }
-        }
+        });
+      }
 
+      keys.forEach((key, index) => {
         promiseByKey[key] = expect.promise(() => {
+          const subjectKey = subjectType.valueForKey(subject, key);
+          const valueKey = valueType.valueForKey(value, key);
+          const valueKeyType = expect.findTypeOf(valueKey);
           if (valueKeyType.is('expect.it')) {
             expect.context.thisObject = subject;
             return valueKey(subjectKey, expect.context);
@@ -1842,8 +1834,25 @@ module.exports = expect => {
       return expect.promise
         .all([
           expect.promise(() => {
-            if (forceExhaustivelyComparison) {
-              throw new Error('exhaustive comparison failure');
+            if (expect.flags.exhaustively) {
+              const nonOwnKeysWithDefinedValues = keys.filter(
+                key =>
+                  !Object.prototype.hasOwnProperty.call(subject, key) &&
+                  typeof subjectType.valueForKey(subject, key) !== 'undefined'
+              );
+              const valueKeysWithDefinedValues = keys.filter(
+                key => typeof valueType.valueForKey(value, key) !== 'undefined'
+              );
+              const subjectKeysWithDefinedValues = subjectKeys.filter(
+                key =>
+                  typeof subjectType.valueForKey(subject, key) !== 'undefined'
+              );
+              expect(
+                valueKeysWithDefinedValues.length -
+                  nonOwnKeysWithDefinedValues.length,
+                'to equal',
+                subjectKeysWithDefinedValues.length
+              );
             }
           }),
           expect.promise.all(promiseByKey)
@@ -1854,13 +1863,16 @@ module.exports = expect => {
               diff(output, diff, inspect, equal) {
                 output.inline = true;
                 const subjectIsArrayLike = subjectType.is('array-like');
-                // Skip missing keys expected to be missing so they don't get rendered in the diff
-                const keys = uniqueKeys.filter(key => {
-                  return (
-                    subjectType.hasKey(subject, key) ||
-                    typeof valueType.valueForKey(value, key) !== 'undefined'
-                  );
-                });
+                const valueKeys = valueType.getKeys(value);
+                const keys = subjectType
+                  .uniqueKeys(subjectKeys, valueKeys)
+                  .filter(key => {
+                    // Skip missing keys expected to be missing so they don't get rendered in the diff
+                    return (
+                      subjectType.hasKey(subject, key) ||
+                      typeof valueType.valueForKey(value, key) !== 'undefined'
+                    );
+                  });
                 const prefixOutput = subjectType.prefix(
                   output.clone(),
                   subject
@@ -2348,19 +2360,14 @@ module.exports = expect => {
             }
           });
         },
-        err => {
-          if (err.isOperational && !err.propertyIsEnumerable('isOperational')) {
-            delete err.isOperational;
-          }
-
+        err =>
           expect.withError(
             () => expect.shift(err),
             e => {
               e.originalError = err;
               throw e;
             }
-          );
-        }
+          )
       );
     }
   );
diff --git a/lib/types.js b/lib/types.js
index e7002d6d..132942b0 100644
--- a/lib/types.js
+++ b/lib/types.js
@@ -114,7 +114,7 @@ module.exports = function(expect) {
     },
     getKeys: Object.getOwnPropertySymbols
       ? obj => {
-          const keys = Object.getOwnPropertyNames(obj);
+          const keys = Object.keys(obj);
           const symbols = Object.getOwnPropertySymbols(obj);
           if (symbols.length > 0) {
             return keys.concat(symbols);
@@ -122,7 +122,7 @@ module.exports = function(expect) {
             return keys;
           }
         }
-      : Object.getOwnPropertyNames,
+      : Object.keys,
     // If Symbol support is not detected default to undefined which, when
     // passed to Array.prototype.sort, means "natural" (asciibetical) sort.
     keyComparator:
@@ -154,7 +154,44 @@ module.exports = function(expect) {
           }
         : undefined,
     equal(a, b, equal) {
-      return utils.checkObjectEqualityUsingType(a, b, this, equal);
+      if (a === b) {
+        return true;
+      }
+
+      if (b.constructor !== a.constructor) {
+        return false;
+      }
+
+      const actualKeys = this.getKeys(a).filter(
+        key => typeof this.valueForKey(a, key) !== 'undefined'
+      );
+      const expectedKeys = this.getKeys(b).filter(
+        key => typeof this.valueForKey(b, key) !== 'undefined'
+      );
+
+      // having the same number of owned properties (keys incorporates hasOwnProperty)
+      if (actualKeys.length !== expectedKeys.length) {
+        return false;
+      }
+      //the same set of keys (although not necessarily the same order),
+      actualKeys.sort(this.keyComparator);
+      expectedKeys.sort(this.keyComparator);
+      // cheap key test
+      for (let i = 0; i < actualKeys.length; i += 1) {
+        if (actualKeys[i] !== expectedKeys[i]) {
+          return false;
+        }
+      }
+
+      //equivalent values for every corresponding key, and
+      // possibly expensive deep test
+      for (let j = 0; j < actualKeys.length; j += 1) {
+        const key = actualKeys[j];
+        if (!equal(this.valueForKey(a, key), this.valueForKey(b, key))) {
+          return false;
+        }
+      }
+      return true;
     },
     hasKey(obj, key) {
       return key in obj;
@@ -762,20 +799,12 @@ module.exports = function(expect) {
     'sourceId',
     'sourceURL',
     'stack',
-    'stackArray',
-    '__stackCleaned__',
-    'isOperational' // added by the promise implementation
+    'stackArray'
   ].reduce((result, prop) => {
     result[prop] = true;
     return result;
   }, {});
 
-  if (new Error().hasOwnProperty('arguments')) {
-    // node.js 0.10 adds two extra non-enumerable properties to Error instances:
-    errorMethodBlacklist.arguments = true;
-    errorMethodBlacklist.type = true;
-  }
-
   expect.addType({
     base: 'object',
     name: 'Error',
@@ -797,9 +826,7 @@ module.exports = function(expect) {
     },
     equal(a, b, equal) {
       return (
-        a === b ||
-        (equal(a.message, b.message) &&
-          utils.checkObjectEqualityUsingType(a, b, this, equal))
+        a === b || (equal(a.message, b.message) && this.baseType.equal(a, b))
       );
     },
     inspect(value, depth, output, inspect) {
diff --git a/lib/utils.js b/lib/utils.js
index 20c23bb0..6b8217f8 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -27,48 +27,6 @@ const utils = (module.exports = {
       return a === b;
     }),
 
-  checkObjectEqualityUsingType(a, b, type, isEqual) {
-    if (a === b) {
-      return true;
-    }
-
-    if (b.constructor !== a.constructor) {
-      return false;
-    }
-
-    const actualKeys = type
-      .getKeys(a)
-      .filter(key => typeof type.valueForKey(a, key) !== 'undefined');
-    const expectedKeys = type
-      .getKeys(b)
-      .filter(key => typeof type.valueForKey(b, key) !== 'undefined');
-
-    // having the same number of owned properties (keys incorporates hasOwnProperty)
-    if (actualKeys.length !== expectedKeys.length) {
-      return false;
-    }
-
-    //the same set of keys (although not necessarily the same order),
-    actualKeys.sort(type.keyComparator);
-    expectedKeys.sort(type.keyComparator);
-    // cheap key test
-    for (let i = 0; i < actualKeys.length; i += 1) {
-      if (actualKeys[i] !== expectedKeys[i]) {
-        return false;
-      }
-    }
-
-    //equivalent values for every corresponding key, and
-    // possibly expensive deep test
-    for (let j = 0; j < actualKeys.length; j += 1) {
-      const key = actualKeys[j];
-      if (!isEqual(type.valueForKey(a, key), type.valueForKey(b, key))) {
-        return false;
-      }
-    }
-    return true;
-  },
-
   duplicateArrayLikeUsingType(obj, type) {
     const keys = type.getKeys(obj);
 


Test patch:
diff --git a/test/api/inspect.spec.js b/test/api/inspect.spec.js
index 92a941b5..bd6877ac 100644
--- a/test/api/inspect.spec.js
+++ b/test/api/inspect.spec.js
@@ -207,9 +207,9 @@ describe('inspect', function() {
             deeply: {
               nested: { object: 'This should not be shown' },
               string: 'should be shown',
-              list: [1, 2, 3]
+              'a list': [1, 2, 3]
             },
-            list: [1, 2, 3]
+            'a list': [1, 2, 3]
           }
         }
       }
@@ -269,8 +269,8 @@ describe('inspect', function() {
         '    circular: { self: [Circular] },\n' +
         '    this: {\n' +
         '      is: {\n' +
-        "        deeply: { nested: ..., string: 'should be shown', list: ... },\n" +
-        '        list: [ 1, 2, 3 ]\n' +
+        "        deeply: { nested: ..., string: 'should be shown', 'a list': ... },\n" +
+        "        'a list': [ 1, 2, 3 ]\n" +
         '      }\n' +
         '    }\n' +
         '  }\n' +
@@ -321,7 +321,7 @@ describe('inspect', function() {
         "      { id: 4, name: 'Barbara Lynn' },\n" +
         "      { id: 5, name: 'Sharpe Downs' }\n" +
         '    ],\n' +
-        "    circular: { self: [Circular] }, this: { is: { deeply: { nested: ..., string: 'should be shown', list: ... }, list: [ 1, 2, 3 ] } }\n" +
+        "    circular: { self: [Circular] }, this: { is: { deeply: { nested: ..., string: 'should be shown', 'a list': ... }, 'a list': [ 1, 2, 3 ] } }\n" +
         '  }\n' +
         ']'
     );
diff --git a/test/assertions/to-have-property.spec.js b/test/assertions/to-have-property.spec.js
index 5460683a..6c255fc4 100644
--- a/test/assertions/to-have-property.spec.js
+++ b/test/assertions/to-have-property.spec.js
@@ -42,24 +42,21 @@ describe('to have property assertion', function() {
           expect(subject, 'to have enumerable property', 'enumFalse');
         },
         'to throw exception',
-        "expected { a: 'b', enumFalse: 't', configFalse: 't', writableFalse: 't' }\n" +
-          "to have enumerable property 'enumFalse'"
+        "expected { a: 'b' } to have enumerable property 'enumFalse'"
       );
       expect(
         function() {
           expect(subject, 'to have configurable property', 'configFalse');
         },
         'to throw exception',
-        "expected { a: 'b', enumFalse: 't', configFalse: 't', writableFalse: 't' }\n" +
-          "to have configurable property 'configFalse'"
+        "expected { a: 'b' } to have configurable property 'configFalse'"
       );
       expect(
         function() {
           expect(subject, 'to have writable property', 'writableFalse');
         },
         'to throw exception',
-        "expected { a: 'b', enumFalse: 't', configFalse: 't', writableFalse: 't' }\n" +
-          "to have writable property 'writableFalse'"
+        "expected { a: 'b' } to have writable property 'writableFalse'"
       );
     });
   });
diff --git a/test/assertions/to-satisfy.spec.js b/test/assertions/to-satisfy.spec.js
index a0a7e0bd..458b29d1 100644
--- a/test/assertions/to-satisfy.spec.js
+++ b/test/assertions/to-satisfy.spec.js
@@ -2194,8 +2194,7 @@ describe('to satisfy assertion', function() {
               });
             },
             'to throw',
-            "expected { nonEnumerable: 'theValue' }\n" +
-              "to exhaustively satisfy { nonEnumerable: 'wrong' }\n" +
+            "expected {} to exhaustively satisfy { nonEnumerable: 'wrong' }\n" +
               '\n' +
               '{\n' +
               '  nonEnumerable:\n' +
@@ -2209,6 +2208,10 @@ describe('to satisfy assertion', function() {
       });
 
       describe('when not matching the non-enumerable property', function() {
+        it('should succeed', function() {
+          expect(bar, 'to exhaustively satisfy', {});
+        });
+
         it('should fail with a diff', function() {
           expect(
             function() {
@@ -2217,11 +2220,9 @@ describe('to satisfy assertion', function() {
               });
             },
             'to throw',
-            "expected { nonEnumerable: 'theValue' }\n" +
-              "to exhaustively satisfy { somethingElse: 'wrong' }\n" +
+            "expected {} to exhaustively satisfy { somethingElse: 'wrong' }\n" +
               '\n' +
               '{\n' +
-              "  nonEnumerable: 'theValue' // should be removed\n" +
               "  // missing somethingElse: 'wrong'\n" +
               '}'
           );
diff --git a/test/types/error-type.spec.js b/test/types/error-type.spec.js
index 230f3ab7..10650341 100644
--- a/test/types/error-type.spec.js
+++ b/test/types/error-type.spec.js
@@ -61,14 +61,8 @@ describe('Error type', function() {
     }
 
     function MyError(message) {
-      var instance = new Error(message);
-      var proto = Object.getPrototypeOf(this);
-      if (Object.setPrototypeOf) {
-        Object.setPrototypeOf(instance, proto);
-      } else {
-        instance.__proto__ = proto; // eslint-disable-line no-proto
-      }
-      return instance;
+      Error.call(this);
+      this.message = message;
     }
 
     inherits(MyError, Error);
@@ -146,16 +140,4 @@ describe('Error type', function() {
       });
     });
   });
-
-  describe('when comparing Error objects with differing enumerable keys', () => {
-    it('should not break', () => {
-      var e1 = new Error('foo');
-      var e2 = new Error();
-      e2.message = 'foo';
-
-      expect(() => {
-        expect(e1, 'to equal', e2);
-      }, 'not to throw');
-    });
-  });
 });