Skip to content

Include non-enumerable properties #482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexjeffburke merged 10 commits into from
Jun 29, 2018
Merged

Include non-enumerable properties #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9337c5a
Avoid deferring to the base type for object equality.
alexjeffburke Mar 11, 2018
8ecdf6e
Switch to retrieving all properties from objects and fixup Error.
alexjeffburke May 21, 2018
5ec7dc9
Catch up with enumerable properties being included.
alexjeffburke May 21, 2018
36b828c
Remove unnecessary loop given non-enumerable keys are now included.
alexjeffburke May 21, 2018
4557f85
Simplify key handling in "to satisfy" given the enumerability changes.
alexjeffburke May 21, 2018
6fb0a96
Rework the special case handling of value side expect.it.
alexjeffburke May 21, 2018
763607a
Handle errors with additional non-enumerable properties via bluebird.
alexjeffburke Jun 2, 2018
fa0be50
Work around issues with node 0.10
papandreou Jun 21, 2018
695226a
Fix documentation tests
papandreou Jun 21, 2018
4683469
Fix prettier 1.13 nit
papandreou Jun 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 27 additions & 27 deletions documentation/api/addType.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,32 +299,32 @@ block. The outputs below shows the contrast between setting the
inlineDiff = true;
expect(
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Janie Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Janie Doe', 24)
},
'to equal',
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Jane Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Jane Doe', 24)
}
);
```

```output
expected
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Janie Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Janie Doe', 24)
}
to equal
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Jane Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Jane Doe', 24)
}

{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person(
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person(
'Janie Doe', // should be 'Jane Doe'
// -Janie Doe
// +Jane Doe
Expand All @@ -337,39 +337,39 @@ to equal
inlineDiff = false;
expect(
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Janie Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Janie Doe', 24)
},
'to equal',
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Jane Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Jane Doe', 24)
}
);
```

```output
expected
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Janie Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('Janie Doe', 24)
}
to equal
{
'John Doe': new Person('John Doe', 42),
'Jane Doe': new Person('Jane Doe', 24)
JohnDoe: new Person('John Doe', 42),
JaneDoe: new Person('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
// )
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
// )
}
```

Expand Down
87 changes: 40 additions & 47 deletions lib/assertions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1796,27 +1796,35 @@ 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);
Copy link
Member

@sunesimonsen sunesimonsen Jun 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhh I know this is my mistake, but I think things will be much more clear if we called subject actual and value expected in this method. We can always fix that later.

Copy link
Member

@sunesimonsen sunesimonsen Jun 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe subject and spec.

// calculate the unique keys early given enumerability no
// longer affects what is included in the list of keys
const uniqueKeys = subjectType.uniqueKeys(subjectKeys, valueKeys);

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);
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';
Copy link
Member

@papandreou papandreou Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't Key be replaced by Value in the above 5 variable names?


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;
}
}

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);
Expand All @@ -1831,25 +1839,8 @@ module.exports = expect => {
return expect.promise
.all([
expect.promise(() => {
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
);
if (forceExhaustivelyComparison) {
throw new Error('exhaustive comparison failure');
}
}),
expect.promise.all(promiseByKey)
Expand All @@ -1860,16 +1851,13 @@ module.exports = expect => {
diff(output, diff, inspect, equal) {
output.inline = true;
const subjectIsArrayLike = subjectType.is('array-like');
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'
);
});
// 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 prefixOutput = subjectType.prefix(
output.clone(),
subject
Expand Down Expand Up @@ -2354,14 +2342,19 @@ module.exports = expect => {
}
});
},
err =>
err => {
if (err.isOperational && !err.propertyIsEnumerable('isOperational')) {
delete err.isOperational;
}

expect.withError(
() => expect.shift(err),
e => {
e.originalError = err;
throw e;
}
)
);
}
);
}
);
Expand Down
57 changes: 15 additions & 42 deletions lib/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ module.exports = function(expect) {
},
getKeys: Object.getOwnPropertySymbols
? obj => {
const keys = Object.keys(obj);
const keys = Object.getOwnPropertyNames(obj);
const symbols = Object.getOwnPropertySymbols(obj);
if (symbols.length > 0) {
return keys.concat(symbols);
} else {
return keys;
}
}
: Object.keys,
: Object.getOwnPropertyNames,
// If Symbol support is not detected default to undefined which, when
// passed to Array.prototype.sort, means "natural" (asciibetical) sort.
keyComparator:
Expand Down Expand Up @@ -154,44 +154,7 @@ module.exports = function(expect) {
}
: undefined,
equal(a, b, 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;
return utils.checkObjectEqualityUsingType(a, b, this, equal);
},
hasKey(obj, key) {
return key in obj;
Expand Down Expand Up @@ -799,12 +762,20 @@ module.exports = function(expect) {
'sourceId',
'sourceURL',
'stack',
'stackArray'
'stackArray',
'__stackCleaned__',
'isOperational' // added by the promise implementation
].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',
Expand All @@ -826,7 +797,9 @@ module.exports = function(expect) {
},
equal(a, b, equal) {
return (
a === b || (equal(a.message, b.message) && this.baseType.equal(a, b))
a === b ||
(equal(a.message, b.message) &&
utils.checkObjectEqualityUsingType(a, b, this, equal))
);
},
inspect(value, depth, output, inspect) {
Expand Down
42 changes: 42 additions & 0 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,48 @@ 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);

Expand Down
Loading