From 704986292680161be44253c323cd530b379b86c7 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Mon, 25 Feb 2019 15:41:57 -0500 Subject: [PATCH 1/5] expect: Fix non-object received value in toHaveProperty --- .../__snapshots__/matchers.test.js.snap | 149 ++++++++++++++++++ .../expect/src/__tests__/matchers.test.js | 33 ++++ packages/expect/src/matchers.ts | 6 +- packages/expect/src/utils.ts | 9 +- 4 files changed, 193 insertions(+), 4 deletions(-) diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap index 8e07feaf3198..5e91817dc3f2 100644 --- a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap @@ -2773,6 +2773,8 @@ exports[`.toHaveProperty() {error} expect({"a": {"b": {}}}).toHaveProperty('unde Expected has value: undefined" `; +exports[`.toHaveProperty() {error} expect({}).toHaveProperty('') 1`] = `"pass must be initialized"`; + exports[`.toHaveProperty() {error} expect(null).toHaveProperty('a.b') 1`] = ` "expect(received).toHaveProperty(path) @@ -2789,6 +2791,16 @@ exports[`.toHaveProperty() {error} expect(undefined).toHaveProperty('a') 1`] = ` Received has value: undefined" `; +exports[`.toHaveProperty() {pass: false} expect("").toHaveProperty('key') 1`] = ` +"expect(object).toHaveProperty(path) + +Expected the object: + \\"\\" +To have a nested property: + \\"key\\" +" +`; + exports[`.toHaveProperty() {pass: false} expect("abc").toHaveProperty('a.b.c') 1`] = ` "expect(object).toHaveProperty(path) @@ -2914,6 +2926,19 @@ Difference: Comparing two different types of values. Expected undefined but received number." `; +exports[`.toHaveProperty() {pass: false} expect({"a": {}}).toHaveProperty('a.b', undefined) 1`] = ` +"expect(object).toHaveProperty(path, value) + +Expected the object: + {\\"a\\": {}} +To have a nested property: + \\"a.b\\" +With a value of: + undefined +Received: + object.a: {}" +`; + exports[`.toHaveProperty() {pass: false} expect({"a": 1}).toHaveProperty('a.b.c.d') 1`] = ` "expect(object).toHaveProperty(path) @@ -2963,6 +2988,16 @@ Received: 1" `; +exports[`.toHaveProperty() {pass: false} expect({"key": 1}).toHaveProperty('not') 1`] = ` +"expect(object).toHaveProperty(path) + +Expected the object: + {\\"key\\": 1} +To have a nested property: + \\"not\\" +" +`; + exports[`.toHaveProperty() {pass: false} expect({}).toHaveProperty('a') 1`] = ` "expect(object).toHaveProperty(path) @@ -3019,6 +3054,16 @@ Difference: Comparing two different types of values. Expected undefined but received string." `; +exports[`.toHaveProperty() {pass: false} expect(0).toHaveProperty('key') 1`] = ` +"expect(object).toHaveProperty(path) + +Expected the object: + 0 +To have a nested property: + \\"key\\" +" +`; + exports[`.toHaveProperty() {pass: false} expect(1).toHaveProperty('a.b.c') 1`] = ` "expect(object).toHaveProperty(path) @@ -3041,6 +3086,50 @@ With a value of: " `; +exports[`.toHaveProperty() {pass: false} expect(Symbol()).toHaveProperty('key') 1`] = ` +"expect(object).toHaveProperty(path) + +Expected the object: + Symbol() +To have a nested property: + \\"key\\" +" +`; + +exports[`.toHaveProperty() {pass: false} expect(false).toHaveProperty('key') 1`] = ` +"expect(object).toHaveProperty(path) + +Expected the object: + false +To have a nested property: + \\"key\\" +" +`; + +exports[`.toHaveProperty() {pass: true} expect("").toHaveProperty('length', 0) 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + \\"\\" +Not to have a nested property: + \\"length\\" +With a value of: + 0 +" +`; + +exports[`.toHaveProperty() {pass: true} expect([Function memoized]).toHaveProperty('memo', []) 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + [Function memoized] +Not to have a nested property: + \\"memo\\" +With a value of: + [] +" +`; + exports[`.toHaveProperty() {pass: true} expect({"a": {"b": [1, 2, 3]}}).toHaveProperty('a,b,1') 1`] = ` "expect(object).not.toHaveProperty(path) @@ -3185,6 +3274,18 @@ With a value of: " `; +exports[`.toHaveProperty() {pass: true} expect({"nodeName": "DIV"}).toHaveProperty('nodeType', 1) 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + {\\"nodeName\\": \\"DIV\\"} +Not to have a nested property: + \\"nodeType\\" +With a value of: + 1 +" +`; + exports[`.toHaveProperty() {pass: true} expect({"property": 1}).toHaveProperty('property', 1) 1`] = ` "expect(object).not.toHaveProperty(path, value) @@ -3197,6 +3298,42 @@ With a value of: " `; +exports[`.toHaveProperty() {pass: true} expect({"val": true}).toHaveProperty('a', undefined) 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + {\\"val\\": true} +Not to have a nested property: + \\"a\\" +With a value of: + undefined +" +`; + +exports[`.toHaveProperty() {pass: true} expect({"val": true}).toHaveProperty('c', "c") 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + {\\"val\\": true} +Not to have a nested property: + \\"c\\" +With a value of: + \\"c\\" +" +`; + +exports[`.toHaveProperty() {pass: true} expect({"val": true}).toHaveProperty('val', true) 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + {\\"val\\": true} +Not to have a nested property: + \\"val\\" +With a value of: + true +" +`; + exports[`.toHaveProperty() {pass: true} expect({}).toHaveProperty('a', undefined) 1`] = ` "expect(object).not.toHaveProperty(path, value) @@ -3221,6 +3358,18 @@ With a value of: " `; +exports[`.toHaveProperty() {pass: true} expect({}).toHaveProperty('setter', undefined) 1`] = ` +"expect(object).not.toHaveProperty(path, value) + +Expected the object: + {} +Not to have a nested property: + \\"setter\\" +With a value of: + undefined +" +`; + exports[`.toMatch() {pass: true} expect(Foo bar).toMatch(/^foo/i) 1`] = ` "expect(received).not.toMatch(expected) diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js index 89435ff47c1d..60e270a892f5 100644 --- a/packages/expect/src/__tests__/matchers.test.js +++ b/packages/expect/src/__tests__/matchers.test.js @@ -1206,7 +1206,26 @@ describe('.toHaveProperty()', () => { get b() { return 'b'; } + set setter(val) { + this.val = val; + } + } + + class Foo2 extends Foo { + get c() { + return 'c'; + } } + const foo2 = new Foo2(); + foo2.setter = true; + + function E(nodeName) { + this.nodeName = nodeName.toUpperCase(); + } + E.prototype.nodeType = 1; + + const memoized = function() {}; + memoized.memo = []; [ [{a: {b: {c: {d: 1}}}}, 'a.b.c.d', 1], @@ -1219,6 +1238,13 @@ describe('.toHaveProperty()', () => { [Object.assign(Object.create(null), {property: 1}), 'property', 1], [new Foo(), 'a', undefined], [new Foo(), 'b', 'b'], + [new Foo(), 'setter', undefined], + [foo2, 'a', undefined], + [foo2, 'c', 'c'], + [foo2, 'val', true], + [new E('div'), 'nodeType', 1], + ['', 'length', 0], + [memoized, 'memo', []], ].forEach(([obj, keyPath, value]) => { test(`{pass: true} expect(${stringify( obj, @@ -1245,6 +1271,7 @@ describe('.toHaveProperty()', () => { [{a: {b: {c: 5}}}, 'a.b', {c: 4}], [new Foo(), 'a', 'a'], [new Foo(), 'b', undefined], + [{a: {}}, 'a.b', undefined], ].forEach(([obj, keyPath, value]) => { test(`{pass: false} expect(${stringify( obj, @@ -1280,6 +1307,11 @@ describe('.toHaveProperty()', () => { [{}, 'a'], [1, 'a.b.c'], ['abc', 'a.b.c'], + [false, 'key'], + [0, 'key'], + ['', 'key'], + [Symbol(), 'key'], + [Object.assign(Object.create(null), {key: 1}), 'not'], ].forEach(([obj, keyPath]) => { test(`{pass: false} expect(${stringify( obj, @@ -1297,6 +1329,7 @@ describe('.toHaveProperty()', () => { [{a: {b: {}}}, undefined], [{a: {b: {}}}, null], [{a: {b: {}}}, 1], + [{}, []], // Residue: pass must be initialized ].forEach(([obj, keyPath]) => { test(`{error} expect(${stringify( obj, diff --git a/packages/expect/src/matchers.ts b/packages/expect/src/matchers.ts index e76bd64c63c6..dbf2ddb80ea9 100644 --- a/packages/expect/src/matchers.ts +++ b/packages/expect/src/matchers.ts @@ -602,9 +602,9 @@ const matchers: MatchersObject = { const result = getPath(object, keyPath); const {lastTraversedObject, hasEndProp} = result; - const pass = valuePassed - ? equals(result.value, value, [iterableEquality]) - : hasEndProp; + const pass = + hasEndProp && + (!valuePassed || equals(result.value, value, [iterableEquality])); const traversedPath = result.traversedPath.join('.'); diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 23d1fe1af797..555717efda76 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -80,7 +80,14 @@ export const getPath = ( result.traversedPath.unshift(prop); if (lastProp) { - result.hasEndProp = prop in object; + try { + result.hasEndProp = newObject !== undefined || prop in object; + } catch (e) { + // A primitive value usually throws TypeError: Cannot use 'in' operator + // Beware: 'length' in string throws, but property does exist. + // However, string['length'] is number, not undefined. + result.hasEndProp = false; + } if (!result.hasEndProp) { result.traversedPath.shift(); } From 08d3066b9093bc9b18b160402c0f9fde0290161b Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Mon, 25 Feb 2019 16:18:10 -0500 Subject: [PATCH 2/5] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ff03985fa81..5fda42aa520c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - `[jest-worker]` Fix `jest-worker` when using pre-allocated jobs ([#7934](https://github.com/facebook/jest/pull/7934)) - `[jest-changed-files]` Fix `getChangedFilesFromRoots` to not return parts of the commit messages as if they were files, when the commit messages contained multiple paragraphs ([#7961](https://github.com/facebook/jest/pull/7961)) - `[static]` Remove console log '-' on the front page +- `[expect]` Fix non-object received value in toHaveProperty ([#7986](https://github.com/facebook/jest/pull/7986)) ### Chore & Maintenance From b9aa00bdd8dc3570e96bb363be8b97d4c816a642 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Tue, 26 Feb 2019 16:25:38 -0500 Subject: [PATCH 3/5] Replace try-catch with typeof and conditional and --- packages/expect/src/utils.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 555717efda76..8253f4d50d94 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -80,14 +80,22 @@ export const getPath = ( result.traversedPath.unshift(prop); if (lastProp) { - try { - result.hasEndProp = newObject !== undefined || prop in object; - } catch (e) { - // A primitive value usually throws TypeError: Cannot use 'in' operator - // Beware: 'length' in string throws, but property does exist. - // However, string['length'] is number, not undefined. - result.hasEndProp = false; + if (newObject === undefined) { + // Does object have the property with an undefined value? + // At this point, object cannot be null nor undefined. + // Although primitive values support bracket notation (above) + // they would throw TypeError for in operator (below). + const type = typeof object; + result.hasEndProp = + type !== 'boolean' && + type !== 'number' && + type !== 'string' && + type !== 'symbol' && + prop in object; + } else { + result.hasEndProp = true; } + if (!result.hasEndProp) { result.traversedPath.shift(); } From a65dc2c108a639895aa57f80c382e3a378b92b88 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Thu, 28 Feb 2019 15:22:10 -0500 Subject: [PATCH 4/5] import isPrimitive from jest-get-type --- packages/expect/src/utils.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 8253f4d50d94..66f2b5412d29 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -6,6 +6,7 @@ * */ +import {isPrimitive} from 'jest-get-type'; import { equals, isA, @@ -82,16 +83,9 @@ export const getPath = ( if (lastProp) { if (newObject === undefined) { // Does object have the property with an undefined value? - // At this point, object cannot be null nor undefined. // Although primitive values support bracket notation (above) // they would throw TypeError for in operator (below). - const type = typeof object; - result.hasEndProp = - type !== 'boolean' && - type !== 'number' && - type !== 'string' && - type !== 'symbol' && - prop in object; + result.hasEndProp = !isPrimitive(object) && prop in object; } else { result.hasEndProp = true; } From 538dbcce12f86ee355e56b381335cfa095cf7f73 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Thu, 28 Feb 2019 15:29:12 -0500 Subject: [PATCH 5/5] Rewrite if-else as logical expression --- packages/expect/src/utils.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 66f2b5412d29..13047d555ff1 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -81,14 +81,11 @@ export const getPath = ( result.traversedPath.unshift(prop); if (lastProp) { - if (newObject === undefined) { - // Does object have the property with an undefined value? - // Although primitive values support bracket notation (above) - // they would throw TypeError for in operator (below). - result.hasEndProp = !isPrimitive(object) && prop in object; - } else { - result.hasEndProp = true; - } + // Does object have the property with an undefined value? + // Although primitive values support bracket notation (above) + // they would throw TypeError for in operator (below). + result.hasEndProp = + newObject !== undefined || (!isPrimitive(object) && prop in object); if (!result.hasEndProp) { result.traversedPath.shift();