From 4055d7438f8b7ac75d34b8b3e340f8a917a00ec4 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Fri, 5 Oct 2018 15:05:35 -0400 Subject: [PATCH 1/4] =?UTF-8?q?expect:=20Return=20false=20from=20asymmetri?= =?UTF-8?q?c=20matchers=20if=20received=20value=20isn=E2=80=99t=20string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/ExpectAPI.md | 12 +++---- .../src/__tests__/asymmetric_matchers.test.js | 32 ++++++++----------- packages/expect/src/asymmetric_matchers.js | 14 ++------ 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/docs/ExpectAPI.md b/docs/ExpectAPI.md index 3f2c285c5bf3..838735be5e7e 100644 --- a/docs/ExpectAPI.md +++ b/docs/ExpectAPI.md @@ -347,7 +347,7 @@ describe('not.objectContaining', () => { ### `expect.not.stringContaining(string)` -`expect.not.stringContaining(string)` matches the received string that does not contain the exact expected string. +`expect.not.stringContaining(string)` matches the received value if it is not a string or if it is a string that does not contain the exact expected string. It is the inverse of `expect.stringContaining`. @@ -355,7 +355,7 @@ It is the inverse of `expect.stringContaining`. describe('not.stringContaining', () => { const expected = 'Hello world!'; - it('matches if the actual string does not contain the expected substring', () => { + it('matches if the received value does not contain the expected substring', () => { expect('How are you?').toEqual(expect.not.stringContaining(expected)); }); }); @@ -363,7 +363,7 @@ describe('not.stringContaining', () => { ### `expect.not.stringMatching(string | regexp)` -`expect.not.stringMatching(string | regexp)` matches the received string that does not match the expected regexp. +`expect.not.stringMatching(string | regexp)` matches the received value if it is not a string or if it is a string that does not match the expected string or regular expression. It is the inverse of `expect.stringMatching`. @@ -371,7 +371,7 @@ It is the inverse of `expect.stringMatching`. describe('not.stringMatching', () => { const expected = /Hello world!/; - it('matches if the actual string does not match the expected regex', () => { + it('matches if the received value does not match the expected regex', () => { expect('How are you?').toEqual(expect.not.stringMatching(expected)); }); }); @@ -400,11 +400,11 @@ test('onPress gets called with the right thing', () => { ### `expect.stringContaining(string)` -`expect.stringContaining(string)` matches the received string that contains the exact expected string. +`expect.stringContaining(string)` matches the received value if it is a string that contains the exact expected string. ### `expect.stringMatching(string | regexp)` -`expect.stringMatching(string | regexp)` matches the received string that matches the expected regexp. +`expect.stringMatching(string | regexp)` matches the received value if it is a string that matches the expected string or regular expression. You can use it instead of a literal value: diff --git a/packages/expect/src/__tests__/asymmetric_matchers.test.js b/packages/expect/src/__tests__/asymmetric_matchers.test.js index 9b7a030d4d54..47feb2b60b77 100644 --- a/packages/expect/src/__tests__/asymmetric_matchers.test.js +++ b/packages/expect/src/__tests__/asymmetric_matchers.test.js @@ -199,14 +199,14 @@ test('StringContaining matches string against string', () => { jestExpect(stringContaining('en').asymmetricMatch('queue')).toBe(false); }); -test('StringContaining throws for non-strings', () => { +test('StringContaining throws if expected value is not string', () => { jestExpect(() => { stringContaining([1]).asymmetricMatch('queen'); }).toThrow(); +}); - jestExpect(() => { - stringContaining('en*').asymmetricMatch(1); - }).toThrow(); +test('StringContaining returns false if received value is not string', () => { + jestExpect(stringContaining('en*').asymmetricMatch(1)).toBe(false); }); test('StringNotContaining matches string against string', () => { @@ -214,14 +214,14 @@ test('StringNotContaining matches string against string', () => { jestExpect(stringNotContaining('en').asymmetricMatch('queue')).toBe(true); }); -test('StringNotContaining throws for non-strings', () => { +test('StringNotContaining throws if expected value is not string', () => { jestExpect(() => { stringNotContaining([1]).asymmetricMatch('queen'); }).toThrow(); +}); - jestExpect(() => { - stringNotContaining('en*').asymmetricMatch(1); - }).toThrow(); +test('StringNotContaining returns true if received value is not string', () => { + jestExpect(stringNotContaining('en*').asymmetricMatch(1)).toBe(true); }); test('StringMatching matches string against regexp', () => { @@ -234,16 +234,14 @@ test('StringMatching matches string against string', () => { jestExpect(stringMatching('en').asymmetricMatch('queue')).toBe(false); }); -test('StringMatching throws for non-strings and non-regexps', () => { +test('StringMatching throws if expected value is neither string nor regexp', () => { jestExpect(() => { stringMatching([1]).asymmetricMatch('queen'); }).toThrow(); }); -test('StringMatching throws for non-string actual values', () => { - jestExpect(() => { - stringMatching('en').asymmetricMatch(1); - }).toThrow(); +test('StringMatching returns false if received value is not string', () => { + jestExpect(stringMatching('en').asymmetricMatch(1)).toBe(false); }); test('StringNotMatching matches string against regexp', () => { @@ -256,14 +254,12 @@ test('StringNotMatching matches string against string', () => { jestExpect(stringNotMatching('en').asymmetricMatch('queue')).toBe(true); }); -test('StringNotMatching throws for non-strings and non-regexps', () => { +test('StringNotMatching throws if expected value is neither string nor regexp', () => { jestExpect(() => { stringNotMatching([1]).asymmetricMatch('queen'); }).toThrow(); }); -test('StringNotMatching throws for non-string actual values', () => { - jestExpect(() => { - stringNotMatching('en').asymmetricMatch(1); - }).toThrow(); +test('StringNotMatching returns true if received value is not string', () => { + jestExpect(stringNotMatching('en').asymmetricMatch(1)).toBe(true); }); diff --git a/packages/expect/src/asymmetric_matchers.js b/packages/expect/src/asymmetric_matchers.js index b182fb1f1c44..09bc54aab93b 100644 --- a/packages/expect/src/asymmetric_matchers.js +++ b/packages/expect/src/asymmetric_matchers.js @@ -216,12 +216,8 @@ class StringContaining extends AsymmetricMatcher { this.inverse = inverse; } - asymmetricMatch(other: string) { - if (!isA('String', other)) { - throw new Error('Actual is not a string'); - } - - const result = other.includes(this.sample); + asymmetricMatch(other: any) { + const result = isA('String', other) && other.includes(this.sample); return this.inverse ? !result : result; } @@ -248,11 +244,7 @@ class StringMatching extends AsymmetricMatcher { this.inverse = inverse; } - asymmetricMatch(other: string) { - if (!isA('String', other)) { - throw new Error('Actual is not a string'); - } - + asymmetricMatch(other: any) { const result = this.sample.test(other); return this.inverse ? !result : result; From 17b67af9323c2c621fc0add47ae548e3c2ce9ca3 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Fri, 5 Oct 2018 15:15:10 -0400 Subject: [PATCH 2/4] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddc5f5af354c..5ba37ff18d28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - `[jest-jasmine2`] Fix crash when test return Promise rejected with null ([#7049](https://github.com/facebook/jest/pull/7049)) - `[jest-runtime]` Check `_isMockFunction` is true rather than truthy on potential global mocks ([#7017](https://github.com/facebook/jest/pull/7017)) - `[jest-jasmine]` Show proper error message from async `assert` errors ([#6821](https://github.com/facebook/jest/pull/6821)) +- `[expect]` Return false from asymmetric matchers if received value isn’t string ([#7107](https://github.com/facebook/jest/pull/7107)) ### Chore & Maintenance From 96722cb163a455afeb2df17a9203fcc99257393e Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Fri, 5 Oct 2018 15:32:15 -0400 Subject: [PATCH 3/4] Add failing test and condition to fix error in stringMatching --- packages/expect/src/__tests__/asymmetric_matchers.test.js | 4 ++++ packages/expect/src/asymmetric_matchers.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/expect/src/__tests__/asymmetric_matchers.test.js b/packages/expect/src/__tests__/asymmetric_matchers.test.js index 47feb2b60b77..825397009256 100644 --- a/packages/expect/src/__tests__/asymmetric_matchers.test.js +++ b/packages/expect/src/__tests__/asymmetric_matchers.test.js @@ -244,6 +244,10 @@ test('StringMatching returns false if received value is not string', () => { jestExpect(stringMatching('en').asymmetricMatch(1)).toBe(false); }); +test('StringMatching returns false even if coerced non-string received value matches pattern', () => { + jestExpect(stringMatching('null').asymmetricMatch(null)).toBe(false); +}); + test('StringNotMatching matches string against regexp', () => { jestExpect(stringNotMatching(/en/).asymmetricMatch('queen')).toBe(false); jestExpect(stringNotMatching(/en/).asymmetricMatch('queue')).toBe(true); diff --git a/packages/expect/src/asymmetric_matchers.js b/packages/expect/src/asymmetric_matchers.js index 09bc54aab93b..e34e5eefc6f9 100644 --- a/packages/expect/src/asymmetric_matchers.js +++ b/packages/expect/src/asymmetric_matchers.js @@ -245,7 +245,7 @@ class StringMatching extends AsymmetricMatcher { } asymmetricMatch(other: any) { - const result = this.sample.test(other); + const result = isA('String', other) && this.sample.test(other); return this.inverse ? !result : result; } From cefdf7175107dd857e2dc0727039d7af42d37a29 Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Fri, 21 Dec 2018 17:04:45 -0500 Subject: [PATCH 4/4] Move 7107 into its place in CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d87df01ba714..f0f80b743520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ - `[jest-jasmine2]` Pending calls inside async tests are reported as pending not failed ([#6782](https://github.com/facebook/jest/pull/6782)) - `[jest-circus]` Better error message when a describe block is empty ([#6372](https://github.com/facebook/jest/pull/6372)) - `[jest-jasmine2]` Add missing testLocationResults for `xit` and `fit`([#6482](https://github.com/facebook/jest/pull/6482)) +- `[expect]` Return false from asymmetric matchers if received value isn’t string ([#7107](https://github.com/facebook/jest/pull/7107)) - `[jest-cli]` Fix unhandled error when a bad revision is provided to `changedSince` ([#7115](https://github.com/facebook/jest/pull/7115)) - `[jest-config]` Moved dynamically assigned `cwd` from `jest-cli` to default configuration in `jest-config` ([#7146](https://github.com/facebook/jest/pull/7146)) - `[jest-config]` Fix `getMaxWorkers` on termux ([#7154](https://github.com/facebook/jest/pull/7154)) @@ -88,7 +89,6 @@ - `[jest-haste-map]` Fix `require` detection with trailing commas and ignore `import typeof` modules ([#7385](https://github.com/facebook/jest/pull/7385)) - `[jest-cli]` Fix to set prettierPath via config file ([#7412](https://github.com/facebook/jest/pull/7412)) - `[jest-cli]` Support dashed args ([#7497](https://github.com/facebook/jest/pull/7497)) -- `[expect]` Return false from asymmetric matchers if received value isn’t string ([#7107](https://github.com/facebook/jest/pull/7107)) ### Chore & Maintenance