Expose expect.extend and alias jasmine.addMatchers to it #1933

blainekasten · 2016-10-14T21:19:41Z

Summary

This solves #1835.

It exposes expect.extend as the jest API for adding matchers and coerces jasmines addMatchers API.

Test plan

I've added jest-jasmine2/jasmine-uses-jests-extend-api-test.js which proves a few things:

jasmine.addMatchers calls expect.extend
jasmine.addMatchers properly gets the API coerced into expect.extend

You can test this by running:
npm run build && npm run jest jasmine-uses-jest

blainekasten

I still need to add proper flow tags to the code. I just wanted to get this up early to see if the approach was good. Looking for feedback!

blainekasten · 2016-10-14T21:20:21Z

packages/jest-jasmine2/src/__tests__/jasmine-uses-jests-extend-api-test.js

+  it('properly alias to jests api', () => {
+    expect(1)._toBeValue(1);
+    expect(1).not._toBeValue(2);
+  });


I should add a test that expect.extend properly works similar to testing jasmine.addMatchers

blainekasten · 2016-10-14T21:20:37Z

packages/jest-jasmine2/src/index.js

@@ -15,6 +15,7 @@ import type {TestResult} from 'types/TestResult';
 import type Runtime from 'jest-runtime';

 const JasmineReporter = require('./reporter');
+const AddMatchersAdapter = require('./jasmineAddMatcherAdapter');


need to remove this

blainekasten · 2016-10-14T21:20:55Z

packages/jest-jasmine2/src/jasmineAddMatcherAdapter.js

@@ -0,0 +1,36 @@
+/**


file should be renamed to jasmineAddMatchersAdapter.js

blainekasten · 2016-10-14T21:22:28Z

packages/jest-matchers/src/index.js


 module.exports = {
-  addMatchers,
+  addMatchers: expect.extend, // TODO: remove!


TODO: My goal is to remove this export. Does that sound good? I just need to go throw and update any code that depends on it.

yep, sounds good.

cpojer · 2016-10-20T07:27:58Z

packages/jest-matchers/src/__tests__/addMatchers-test.js

@@ -10,9 +10,7 @@

 'use strict';


can you rename this file as well? "extend-test.js"?

cpojer · 2016-10-20T07:28:13Z

...tests/toThrowErrorMatchingSnapshot/__tests__/throws-if-tested-function-did-not-throw-test.js

+test('throws the error if tested function did not throw error', () => {
+      expect(() => {}).toThrowErrorMatchingSnapshot();
+    });
+


I'm not really show where this test is coming from and why it is here?

cpojer · 2016-10-20T07:29:10Z

packages/jest-jasmine2/src/__tests__/jasmine-uses-jests-extend-api-test.js

+
+    jasmine.addMatchers({});
+
+    expect(expect.extend).toBeCalled();


if this test fails by throwing, we won't reset expect.extend and we'll be in an odd state. Can you do afterAll(() => expect.extend = originalExtend)?

cpojer · 2016-10-20T07:29:17Z

packages/jest-jasmine2/src/__tests__/jasmine-uses-jests-extend-api-test.js

+    expect.extend = originalExtend;
+  });
+
+  it('properly alias to jests api', () => {


cpojer · 2016-10-20T07:30:47Z

packages/jest-jasmine2/src/extendJasmineExpect.js

@@ -18,11 +17,15 @@ const jasmineExpect = global.expect;

 // extend jasmine matchers with `jest-matchers`
 module.exports = () => {
-  addMatchers({toMatchSnapshot, toThrowErrorMatchingSnapshot});
+  jestExpect.extend({toMatchSnapshot, toThrowErrorMatchingSnapshot});
+
  global.expect = actual => {
    const jasmineMatchers = jasmineExpect(actual);
    const jestMatchers = jestExpect(actual);
    const not = Object.assign(jasmineMatchers.not, jestMatchers.not);
    return Object.assign(jasmineMatchers, jestMatchers, {not});
  };


I think with your change we should be able to kill this and replace it with:

global.expect = jestExpect;

because we won't need Jasmine's expect at all any longer.

cpojer · 2016-10-20T07:31:05Z

packages/jest-jasmine2/src/extendJasmineExpect.js

  global.expect = actual => {
    const jasmineMatchers = jasmineExpect(actual);
    const jestMatchers = jestExpect(actual);
    const not = Object.assign(jasmineMatchers.not, jestMatchers.not);
    return Object.assign(jasmineMatchers, jestMatchers, {not});
  };
+
+  // Expose addMatchers API
+  global.expect.extend = jestExpect.extend;


with the change mentioned above, we won't need to do this either :)

cpojer · 2016-10-20T07:35:52Z

This does look good. Would you mind polishing it up so we can ship it? :)

cc @DmitriiAbramov

aaronabramov · 2016-10-21T04:13:42Z

this looks good! There is a few things we'll need to clean up in www before we can land it (custom equality) but i remember that it wasn't that complicated.
(sorry for the long response. long vacation :/)

cpojer · 2016-10-21T06:54:15Z

We can land it before cleaning up our internal code. We'll just update everything at FB the next time we release a version of Jest.

We should also find a way to expose matcherHint etc. so that messages can be pretty-printed, but that can be done in a separate PR.

blainekasten · 2016-10-22T14:36:49Z

packages/jest-jasmine2/src/__tests__/jasmine-uses-jests-extend-api-test.js

+
+    jasmine.addMatchers({});
+
+    expect(expect.extend).toBeCalled();


blainekasten · 2016-10-22T14:36:58Z

integration_tests/snapshot/__tests__/__snapshots__/second-snapshot-test.js.snap

+  "c": "three\`",
+  "d": "vier",
+}
+`;


I'm not sure where this came from, should I remove it?

blainekasten · 2016-10-22T14:37:38Z

packages/jest-jasmine2/src/expose-jest-expect.js

+// globaly expost `jestExpect` and overrides jasmines
+module.exports = () : void => {
+  global.expect = jestExpect;
+};


This seems overly simple for a file, but do you think there is a different/better way to globally expose it? I figured it should happen in this package as it's the one that handles all the runtime resolutions.

blainekasten · 2016-10-22T14:38:05Z

packages/jest-jasmine2/src/index.js

@@ -86,11 +86,17 @@ function jasmine2(

  env.addReporter(reporter);

+  // Adapter for coercing jasmine `addMatcher` api into using
+  // jest's `expect.extend` API.


should this be Jest's?

blainekasten · 2016-10-22T14:40:37Z

packages/jest-matchers/src/matchers.js

@@ -398,6 +402,8 @@ const matchers: MatchersObject = {

    return {message, pass};
  },
+
+  toMatchSnapshot,


Is there a reason these matchers were added in jest-jasmine2 and not in jest-matchers ?

yeah, we didn't want jest-matchers to depend on the snapshot feature yet.

cpojer · 2016-10-24T05:42:51Z

thanks @blainekasten. The last iteration I think you might not have run locally yourself ( :P ) but I spend some time polishing it and will merge in a moment.

blainekasten · 2016-10-24T16:01:23Z

Awesome. Thanks for carrying it across the line for me. I wasn't sure what you guys were ultimately going to want. So thanks!

* Expose expect.extend and alias jasmine.addMatchers to it * Clean up code for PR * rename files * Cleanup + updates.

github-actions · 2021-05-14T07:10:42Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the CLA Signed ✔️ label Oct 14, 2016

blainekasten commented Oct 14, 2016

View reviewed changes

blainekasten mentioned this pull request Oct 14, 2016

Implement jest expect.extend API enzymejs/enzyme-matchers#44

Closed

cpojer reviewed Oct 20, 2016

View reviewed changes

facebook-github-bot added the CLA Signed ✔️ label Oct 21, 2016

cpojer mentioned this pull request Oct 21, 2016

toChange matcher for mutable/stateful interfaces diffing #1940

Closed

blainekasten commented Oct 22, 2016

View reviewed changes

blainekasten added 3 commits October 24, 2016 13:41

Expose expect.extend and alias jasmine.addMatchers to it

f2d400d

Clean up code for PR

b39d524

rename files

214d285

Cleanup + updates.

bb6b2ca

cpojer force-pushed the jest-add-matcher-api branch from 0f71b9a to bb6b2ca Compare October 24, 2016 05:44

cpojer merged commit 472af8a into jestjs:master Oct 24, 2016

okonet pushed a commit to okonet/jest that referenced this pull request Nov 11, 2016

Expose expect.extend and alias jasmine.addMatchers to it (jestjs#1933)

8bfa732

* Expose expect.extend and alias jasmine.addMatchers to it * Clean up code for PR * rename files * Cleanup + updates.

nickpresta pushed a commit to nickpresta/jest that referenced this pull request Nov 15, 2016

Expose expect.extend and alias jasmine.addMatchers to it (jestjs#1933)

926c929

* Expose expect.extend and alias jasmine.addMatchers to it * Clean up code for PR * rename files * Cleanup + updates.

tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017

Expose expect.extend and alias jasmine.addMatchers to it (jestjs#1933)

36aa12d

* Expose expect.extend and alias jasmine.addMatchers to it * Clean up code for PR * rename files * Cleanup + updates.

SimenB mentioned this pull request Oct 12, 2017

Add ability to add own matchers #118

Closed

SimenB mentioned this pull request May 3, 2020

feat: create no-deprecated-functions jest-community/eslint-plugin-jest#560

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose expect.extend and alias jasmine.addMatchers to it #1933

Expose expect.extend and alias jasmine.addMatchers to it #1933

blainekasten commented Oct 14, 2016

blainekasten left a comment

blainekasten Oct 14, 2016

blainekasten Oct 14, 2016

blainekasten Oct 14, 2016

blainekasten Oct 14, 2016

cpojer Oct 20, 2016

cpojer Oct 20, 2016

cpojer Oct 20, 2016

cpojer Oct 20, 2016

blainekasten Oct 22, 2016

cpojer Oct 20, 2016

cpojer Oct 20, 2016

cpojer Oct 20, 2016

cpojer commented Oct 20, 2016

aaronabramov commented Oct 21, 2016

cpojer commented Oct 21, 2016

blainekasten Oct 22, 2016

blainekasten Oct 22, 2016

blainekasten Oct 22, 2016

blainekasten Oct 22, 2016

blainekasten Oct 22, 2016

cpojer Oct 24, 2016

cpojer commented Oct 24, 2016

blainekasten commented Oct 24, 2016

github-actions bot commented May 14, 2021

Expose expect.extend and alias jasmine.addMatchers to it #1933

Expose expect.extend and alias jasmine.addMatchers to it #1933

Conversation

blainekasten commented Oct 14, 2016

blainekasten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Oct 20, 2016

aaronabramov commented Oct 21, 2016

cpojer commented Oct 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Oct 24, 2016

blainekasten commented Oct 24, 2016

github-actions bot commented May 14, 2021