Skip to content
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.

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

Use the "expect" npm package #1679

Closed
mjackson opened this issue Sep 12, 2016 · 51 comments · Fixed by #4345
Closed

Use the "expect" npm package #1679

mjackson opened this issue Sep 12, 2016 · 51 comments · Fixed by #4345

Comments

@mjackson
Copy link

I'd like to donate use of the expect npm package to the Jest project. We're both working on very similar expect-style APIs, except you guys are killing it with awesome error messages and I'm not. So I wanna let you run with it :)

AFAICT there are only a few places where our APIs differ. Please correct me if I'm wrong. They are:

  • expect uses e.g. .toNotEqual instead of .not.toEqual. TBH I like the .not notation better. Otherwise you end up creating a negated form of every single assertion. So this is 👍 . I'm fine w breaking backwards compat. Maybe we can provide a codemod for people who want to automate the upgrade process. Should be a simple s/toNot/not.to/g.
  • expect includes support for mocha's error diffing when using toEqual (see here). I don't expect full mocha support, but I'd like to keep at least this one aspect for people currently using expect with mocha to help make the transition to Jest easier.
  • expect exposes its basic assert function for those who just need to expect.assert(something). I don't see this in Jest. Would it be easy to expose this?
  • expect includes an expect.extend method for extending the core assertions. We use it e.g. in expect-element to generate assertions specific to DOM nodes and expect-jsx for JSX. In expect we just add properties to Expectation.prototype. Does Jest have a method for doing something similar?
  • expect has a expect.createSpy API where you use jest.fn. expect.createSpy could just be a simple alias for jest.fn.
  • expect has an expect.spyOn function. I don't see this in Jest, but it should be pretty easy to keep it. It's just a wrapper for createSpy.
  • Spies in expect have a calls property that is an array of { context, arguments } objects. Jest uses mock.calls that is an array of arguments. I'm fine with deprecating and using Jest's API here. Just something to note for people upgrading.

Overall I think this should be fairly easy from my side. Not sure what this looks like from your side though. Anything I missed?

/cc @cpojer

@aaronabramov
Copy link
Contributor

@mjackson i think the biggest issue will be migrating expect(x).toMatchSnapshot()
and we haven't quite figured it out ourselves yet :)

the issue here is that it depends on some jest global state (like current running test, filename, whether it's running in --update mode or not, etc). and that means that there has to be some pretty complicated integration with the test runner.
and also
whenever something doesn't match snapshot, we don't really throw an error right away. Instead, we log the failure and decide what to do with it after the test finishes.

@mjackson
Copy link
Author

@DmitriiAbramov Help me understand. Right now, expect doesn't have any concept of snapshotting. So it's essentially just a new feature for expect users. What is there to migrate?

@cpojer cpojer self-assigned this Sep 14, 2016
@cpojer cpojer added this to the next milestone Sep 14, 2016
@phated
Copy link

phated commented Sep 16, 2016

Coupling this to the global state of a specific runner for snapshots would make this a non-starter. I'm also confused why nothing would be thrown immediately (also seems like that would break usage in other test runners).

@mjackson
Copy link
Author

@phated It's my understanding that expect(x).toMatchSnapshot depends on global state, but the other methods do not and will continue to throw immediately. Since toMatchSnapshot isn't actually something that currently exists in expect, it shouldn't adversely affect existing users.

@cpojer
Copy link
Member

cpojer commented Sep 16, 2016

We won't be adding the snapshot matcher to expect from the beginning. For now it will just be jest-matchers. Snapshotting is done in its own package, jest-snapshot. We will explore ways to bring snapshotting to other runners through expect (cc @suchipi) but it isn't our immediate goal. @DmitriiAbramov didn't know this was coming because I failed to communicate with him.

@suchipi
Copy link
Contributor

suchipi commented Sep 16, 2016

I don't know how expect is architected, but in #1413 I was trying to make the snapshot matcher pretty agnostic; with the proposed changes, this is how it would be used:

import {createSnapshotState, processSnapshot} from 'jest-snapshot';

const fileName = "/home/whoever/whatever.snap.js";
const snapshotName = "renders correctly 1";
const actual = {/* ... */};
const options = {updateSnapshot: true}; // or false
const snapshotState = createSnapshotState(fileName);
const {pass, expected} = processSnapshot(snapshotState, snapshotName, actual, options);

So as long as you can bring your own actual, path to the snapshot file, and whether it should update, you could implement it anywhere.

@cpojer
Copy link
Member

cpojer commented Nov 14, 2016

Small update: we just added expect.extend :) We are working on making this happen!

@mjackson
Copy link
Author

@cpojer woo! That was definitely one of the largest differences. Is there a branch I can try out? Or did you put it in a release already? I'll make some time soon to check out jest-matchers and see what's still needed to make this happen so we can have a clear roadmap.

@cpojer
Copy link
Member

cpojer commented Nov 15, 2016

Hey @mjackson,

we are tracking this internally right now. The JS Tools team is pretty busy until the end of year with our three projects (Yarn, Jest and react-native-packager) so we may not make a ton of progress on this immediately but it's definitely one of our goals to make this work well. I'll share something more soon :)

@jeffbski
Copy link

@mjackson FWIW after using both the chai style .not.equal and expect .toNotEqual, I always preferred the mjackson/expect style better. I know the code is probably simpler the other way, but I prefer less dots. Here is an even worse example from chai expect('foo').to.have.length.of.at.least(2);. Painful to type (for me). I don't even like how it looks/reads either. Just my opinions of course, I'm sure there are others who like the dot style.

@mjackson
Copy link
Author

I agree, Jeff. I don't like the chai style either. We're moving to the
Jasmine style. So .not.toEqual instead of .not.to.equal.

See the difference?
On Tue, Nov 15, 2016 at 11:21 PM Jeff Barczewski notifications@github.com
wrote:

@mjackson https://github.com/mjackson FWIW after using both the chai
style .not.equal and expect .toNotEqual, I always preferred the
mjackson/expect style better. I know the code is probably simpler the other
way, but I prefer less dots. Here is an even worse example from chai
expect('foo').to.have.length.of.at.least(2);. Painful to type (for me). I
don't even like how it looks/reads either. Just my opinions of course, I'm
sure there are others who like the dot style.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1679 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAFqp4Xzrs2nOqdO0hFBdFlj2rsMSOo-ks5q-oTOgaJpZM4J63Xy
.

Michael Jackson
@mjackson

@cpojer
Copy link
Member

cpojer commented Nov 16, 2016

I believe there is value in the not negator but any more dots to chain matchers is not really useful imho. I understand this is subjective but I believe the current style in Jest is clean and straightforward. We can also build simple modules that will bring back things like toNotEqual because it's implementation is basically just expect(foo).not.toEqual().

@jeffbski
Copy link

jeffbski commented Nov 16, 2016

@mjackson yes, I agree that wouldn't be too bad if we are just adding one dot clause.
@cpojer I like that idea too, I'd certainly load up that extra module to keep the old style :-)
Thanks for your consideration. I know it isn't life or death decision, but it does influence the developer experience. :-)

@mjackson
Copy link
Author

Absolutely, it affects the UX. Thanks for chiming in here and caring, Jeff :)

FWIW, I think that .not is the best UX for both plugin authors and users.

For plugins, authors only have to write one assertion instead of two. So, e.g. if I want to make a toFlyLikeABird assertion, I don't have to make a toNotFlyLikeABird version as well. .not gives me the negated version for free.

For users, I only have to remember one form of the assertion instead of 2.

@niksajanjic
Copy link

I would also like to add that I use jasmine API sometimes, like spyOn that will be covered with jest once this comes out, but there's one more example where I use jasmine:

expect(someModule.someFunction).toHaveBeenCalledWith(arg1, arg2, jasmine.any(Function));

Because we are also passing anonymous callback function as 3rd parameter we have to expect it because .toHaveBeenCalledWith function will fail if we try to expect only first 2 arguments without selecting last one. I don't mind using jasmine, just giving an example for you guys to decide if it's worth covering this with jest.

@cpojer
Copy link
Member

cpojer commented Dec 20, 2016

@vjeux is helping out to make this a reality.

Here is what was done so far, among adding a ton of features to jest-matchers:

TODO

  • A codemod that does ~80% of the conversion.
  • expect.assert doesn't really make much sense because node comes with assert. I'd rather not add it and include it in the codemod.
  • Add spyOn to Jest: we are currently unsure whether it should be a separate module (jest-mock) or whether these should be on expect. I would prefer to make this a breaking change and have the codemod take care of it. Is this ok @mjackson?

Assuming we'll figure out these things soon, are you happy to transfer ownership of the project to us?

@phated
Copy link

phated commented Dec 20, 2016

I forget if this has been brought up but can this remain es5-compatible?

@cpojer
Copy link
Member

cpojer commented Dec 20, 2016

(edit): We'll likely support the syntax that is supported in node 4 and up. You can add an additional transformation step for the environment you are using.

Also, the current expect library is great and works well. If you've been happy about using it in the past and you don't want to upgrade on existing projects, I think that's fine too.

@vjeux
Copy link
Contributor

vjeux commented Dec 20, 2016

Here's an example of the before/after

jest-matchers:
image

current expect:
image

@suchipi
Copy link
Contributor

suchipi commented Dec 20, 2016

It doesn't make much difference, but my vote for spyOn would be towards jest-mock, just because "expect.spyOn" sounds kinda weird

@mjackson
Copy link
Author

Awesome, thanks @vjeux! :D

expect.assert doesn't really make much sense because node comes with assert. I'd rather not add it and include it in the codemod.

expect currently works in non-node environments. Is jest-matchers currently node-only? I can understand Jest only running in node, but I don't see any reason why an assertion lib should depend on it.

@cpojer
Copy link
Member

cpojer commented Dec 20, 2016

Can you provide us with an integration test? How does expect work in the browser? Do you ship a single build file of it or do you let webpack/browserify take care of the bundling? There is nothing that should prevent jest-matchers to work in the browser (assuming chalk is shimmed properly) but we haven't tested it yet.

cc @zertosh who knows how to create bundles for webpack. Can you help us get a build file of the "jest-matchers" package?

@thymikee
Copy link
Collaborator

@mjackson Jest is also running in jsdom

@cpojer
Copy link
Member

cpojer commented Feb 27, 2017

Hey @skovhus, you are a codemod pro. Do you think you could help us out here by taking https://github.com/cpojer/js-codemod/blob/master/transforms/expect.js and putting it into jest-codemods and adding a few of the fixes that @kentor just mentioned?

@skovhus
Copy link
Contributor

skovhus commented Feb 28, 2017

@jpojer thanks, would love to add that to Jest Codemods. On vacation this week, but will look into it next week. ; )

@AndersDJohnson
Copy link

FYI spyOn seems to have been added with #2537.

@skovhus
Copy link
Contributor

skovhus commented Apr 24, 2017

Happy to say that the transformer for this in Jest-Codemods is almost there (see progress in skovhus/jest-codemods#39). Think it will get most projects 95% of the way when transitioning from expect@1 to Jest/Jest-matchers.

Trying it out on a few ReactTraining repos, shows that we got some ES2015 in jest-matchers. This breaks the tests as jest-matchers is used in older browser. See #3360

@skovhus
Copy link
Contributor

skovhus commented Jun 28, 2017

@cpojer @SimenB any recommendation how expect.restoreSpies() should be code modded?

(related to #2965)

@cpojer
Copy link
Member

cpojer commented Jun 28, 2017

@skovhus if it isn't supported right now, we should add that API to jest-mock. Happy to accept PRs.

@SimenB
Copy link
Member

SimenB commented Jun 28, 2017

I think resetAllMocks is what you're after. It restores stuff that's been called with jest.spyOn. Or do you think it's too broad (since it takes all spies/mocks that ever were)?

@SimenB
Copy link
Member

SimenB commented Aug 24, 2017

PR for it: #4345

@ghost
Copy link

ghost commented Jan 23, 2018

@gaearon You can load the new Expect library (as maintained by facebook/jest) using the awesome service unpkg. Just use the URL https://unpkg.com/expect@latest/build-es5/index.js, which resolves to the latest version 22.1.0 (as of 23.01.2018). Hope that helps! 😄

@SimenB
Copy link
Member

SimenB commented Jan 23, 2018

jest-mock is also available in a build-es5 directory if you need that

@ghost
Copy link

ghost commented Jan 23, 2018

@gaearon Proof of concept of running jest/expect inside JS Bin: jsbin/nucimeceve.

@valera-rozuvan
Copy link

valera-rozuvan commented Jan 25, 2018

I decided to wrap the expect package from facebook/jest as a standalone UMD module. It is located at jest-expect-standalone.

Include jest-expect-standalone as a script tag, and you will have the library available via window.expect.

<script src="https://unpkg.com/jest-expect-standalone@latest/dist/expect.min.js"></script>

See sample JS Bin jsbin/wapokahaxe.

@gaearon Heads up!

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.