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

Add Jest. #250

Merged
merged 1 commit into from
Aug 1, 2016
Merged

Add Jest. #250

merged 1 commit into from
Aug 1, 2016

Conversation

cpojer
Copy link
Contributor

@cpojer cpojer commented Jul 28, 2016

This adds Jest as a default test runner with an example snapshot test as announced in the Jest 14 blog post. The generated snapshot for the example looks like this:

exports[`App renders a welcome view 1`] = `
<div
  className="App">
  <div
    className="App-header">
    <img
      alt="logo"
      className="App-logo"
      src="test-file-stub" />
    <h2>
      Welcome to React
    </h2>
  </div>
  <p
    className="App-intro">
    To get started, edit 
    <code>
      src/App.js
    </code>
     and save to reload.
  </p>
</div>
`;

This integration is the simplest test runner integration I can think of and requires no configuration in the project itself. The repo internal configuration mirrors some of the configuration from the webpack integration tutorial.

I understand there are many opinions in this space and many people have aspirational goals as to what a test runner and framework should be. In reality, what we have found at Facebook is that when it is hard to set up a test runner or hard to write tests, people simply don't do it. This is why Jest's philosophy is to provide an integrated experience that is well set-up and performant by default.

Further, I believe that snapshot testing and the React test renderer will greatly influence how people write React tests and I'm excited for the possibility of other test runners integrating better with this in the future. DOM testing should be left to webdriver or other end-to-end tests. In a test environment, regardless of the real rendering target, there is little value in rendering to a fake DOM. This is especially true for non-web rendering targets.

@ghost ghost added the CLA Signed label Jul 28, 2016
@cpojer cpojer mentioned this pull request Jul 28, 2016

'use strict';

module.exports = new Proxy({}, {
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes CSS modules but we currently don’t support them, so empty exports are fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohhh! I had no idea, I just noticed the CSS import.

Copy link
Contributor

Choose a reason for hiding this comment

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

Importing them just means the corresponding styles get attached, but there are no actual exports (at least not yet).

case 'eject':
case 'start':
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add a test command to main package.json that would work with --debug-template? Please see how we switch paths in config/paths.js. This lets us iterate super quickly on the template without running create-react-app every time we want to see if the commands still work.

You can rename the existing test command to e2e.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

5652ad5

is this what you had in mind? Because test doesn't have a script associated with it I wrote a quick script to add a package.json that links to the preset, runs the test and ensures a snapshot file was created.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I'm a bit confused by this.

We only use shell scripts for our own infrastructure (end to end test and release script). We would like all user-ran scripts to be written in JavaScript.

Ideally I'd like to mirror what we do with start and build. Create a JS file in scripts directory called "test.js". It's fine if all it does is call Jest.

Then set that script as "test" command in package.json, identical to how we do build and start. This lets us work on Create React App locally without creating apps all the time. We just run those commands in the root folder, and they work (try npm start).

Finally, npm test needs to be a part of end to end test. To be clear e2e is our test that ensures Create React App works. It runs npm start and npm run build in root folder to test our development environment, then creates an app and runs them again to make sure generated app works, then ejects and runs them yet again. So this script is where you want to add "npm test" calls so that we know that testing infrastructure works and won't be broken by future PRs.

@gaearon
Copy link
Contributor

gaearon commented Jul 28, 2016

End-to-end testing script in tasks/e2e.sh should include npm test in all places after npm run build.

@cpojer cpojer force-pushed the master branch 2 times, most recently from 4ba3013 to 5652ad5 Compare July 28, 2016 11:25
@lacker
Copy link
Contributor

lacker commented Jul 28, 2016

One big difference between this and the sample PR adding Mocha ( #216 ) is that this just uses babel without webpack to do the transform and the Mocha example is using webpack. So @cpojer I am curious on what you think are the pros and cons of using webpack+babel versus just using babel?

In the discussion here - #218 (comment) - folks were in favor of using webpack, and I implemented run that way in #245 , but I am not totally happy with it because Webpack is not really designed for it. It does not have the caching that e.g. babel-node does, and it doesn't handle node_modules sanely by default, so it feels fragile.

It seems like there are a few ways that we will need a "React runtime" that operates on top of node - for testing, for run scripts, and perhaps for running custom middleware. I would rather be constructing it in just one way, which means that if we are going to use babel without webpack for testing, I'd want to redo the script stuff to not use webpack too.

@@ -23,9 +23,14 @@ module.exports = function(appPath, appName, verbose, originalDirectory) {
appPackage.dependencies[key] = ownPackage.devDependencies[key];
});

// Test config
appPackage.jest = {
preset: '<rootDir>/node_modules/react-scripts/config/jest',
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is writing one line of configuration into the package.json. Kind of aesthetic but we manage to avoid it for webpack, babel, & eslint. Is there a way around it here? also curious for @gaearon thoughts if there is a way around this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We had to do it for ESLint too for the sake of editor integration. I guess it's unavoidable on some level, but I'd like these to stay "pointers" and no more than a couple of lines per tool max.

Copy link
Contributor

Choose a reason for hiding this comment

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

However I would of course prefer there be a way to pass config as a command line option or Node API argument. @cpojer can we make it so?

@cpojer
Copy link
Contributor Author

cpojer commented Jul 29, 2016

Yeah, I definitely agree that jest-runtime is the way to got for running scripts. This way you don't have to bundle anything and everything (transforms etc.) just happen during runtime.

I'll polish this PR a bit.

@cpojer cpojer force-pushed the master branch 7 times, most recently from cc7078f to 941a55d Compare July 29, 2016 10:02
var isInCreateReactAppSource = (
process.argv.some(arg => arg.indexOf('--debug-template') > -1)
);
var isInCreateReactAppSource = process.argv.indexOf('--debug-template') !== -1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a small simplification.

Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work, actually, there's a reason it was with some beforehand!

process.argv might be ['something', '--debug-template --smoke-test'], which process.argv.indexOf doesn't return true for. indexOf only checks if an entire string matches, not if part of a string matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh, that's weird. I reverted it but don't quite understand how to repro process.argv to be in the state you described.

Copy link
Contributor

Choose a reason for hiding this comment

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

We use --smoke-test in the e2e test, but I'm not 100% sure if that case can actually ever happen. (better safe than sorry though) @gaearon and @vjeux have a better idea of that part of the codebase!

@cpojer cpojer force-pushed the master branch 2 times, most recently from e27d57f to 6c39ed4 Compare July 29, 2016 10:16
@statianzo
Copy link

Can you please clarify what should be considered there given that the feature is disabled?

Given that mocking is a prominent feature of Jest, documenting the variance from the default behavior should be considered.

@marcysutton
Copy link

DOM testing should be left to webdriver or other end-to-end tests.

Note that Phantom/headless still has issues with testing focus, as far as I know, so that sounds like a good assumption. ariya/phantomjs#10427

@trevordmiller
Copy link

As someone who was a big proponent of AVA, I should say that I've since decided that AVA's got some issues that make it difficult to work with and I wouldn't recommend beginners use it. I intend to write a blogpost sometime about things that AVA could do to improve. But for myself I'm switching to Mocha and that's what I'd recommend for CRA.

That's good feedback @kentcdodds - I'll do the same :) I'll update the original post to reflect.

@cpojer
Copy link
Contributor Author

cpojer commented Aug 3, 2016

Thank you for bringing up these concerns. First I want to say that we (Facebook) completely dropped the ball on Jest. We open sourced it two years ago and stopped maintaining and working on it. For the entire past year, besides other JavaScript infrastructure work, I have worked full time on Jest to make it actually really good and we now have two people committed to work on it full-time (Hi @DmitriiAbramov!). I think these graphs are telling and we are committed to work on Jest long-term. However, I completely understand that we are fighting an uphill battle and negating the negativity around the project from the first year of its existence is really really hard. We thought about rebranding the project but I believe that the only way we can change the perception is by actually putting our hearts into it and making it really good, which is nothing but hard work over a long period of time.

Let me answer your questions:

Few use Jest outside of Facebook and CRA wont have browser stuff in it, not yet anyway; @trevordmiller suggests using Mocha with Node only (no browser stuff) instead of Jest

Jest is definitely less popular than other frameworks, for the reasons I mentioned above. However, more and more people are adopting it recently and Jest now integrates perfectly with react-native. The framework is used in many large apps that you don't often hear about because they aren't open source (I'm often surprised when I find out myself). I would consider it as recently launched, to be honest, and ask you to throw away any preconception you might have built up previously. If popularity is an actual concern, I think we should have never released React or any other one of our open source projects in the first place. What matters are the features and ease-of-use.

Jest provides these things:

  • Test isolation and sandboxing: no two tests will ever conflict with each other, nor will there ever be any global or module local state that is going to cause trouble.
  • Performance: Jest is built for scale to support both huge and small projects. It will always be fast. We cache babel's output and always only do the minimal amount of work necessary to always provide fast iteration speed and Jest runs tests in parallel. It is simply not useful to wait for a bundle to be built before tests can be run and not scalable for bigger projects.
  • Full code coverage support. Simply run Jest using --coverage. No additional tooling is required. We are currently rewriting it and working on making it awesome.
  • Snapshot testing with great UX for React and react-native integration. We have more ideas for improving the test-renderer and I fully expect snapshot testing without the DOM to become the standard for React and react-native applications.
  • Easy setup: We made an effort to make it easy to use and set-up and we are working on making our configuration actually really good.

On top of some great features it is really a regular test runner like all the others that use Jasmine. Libraries like chai work with Jest out of the box and there aren't any unexpected surprises. We split up Jest into packages and are happy to integrate other test runners apart from Jasmine.

Please check out everything that is listed on the website and take a look at what we are working on.

Browser + Webpack runner may still be needed for accessibility and browser issues; Chrome headless browser might be a good option

I see testing as a pyramid. The foundation is to have tons of unit tests with a small layer of integration tests on top and then one or two end-to-end (webdriver etc.) tests per product that ensure everything works well together. No one single testing solution is either enough nor useful on its own. However, unit tests should be at the core as unit test runners tend to be fast and give you immediate feedback. End-to-end tests often tend to be flaky, take a long time to run and often aren't very helpful for debugging. We described this in our blog post of why we built snapshot testing. I think it is reasonable for create-react-app to provide a solution for the lowest testing layer and not be opinionated about what people would like to use on top of it.

Mocking is not beginner friendly or isolated; CRA will not use mocking but still something to consider

Please note that there is a significant difference between mocking and automocking. CRA will not use automocking but Jest's module-boundary mocking system is incredibly valuable. I will have more to share on this soon but we are planning to disable automocking by default in the next (or next next) major release of Jest. It's a radical change and we have to be careful and incremental about it, which is why we are starting this here and with the react-native preset first. There are places where it shines, like adding tests to a large codebase that has no test coverage but it loses its value over time and people get confused and frustrated with it. However, we will retain the feature and Jest's module-boundary mocking system is still incredibly valuable to easily stub out any module that would go over the network or does something you want to stub in a test. For example, to achieve something similar to shallow rendering with the test renderer, any React component can simply be mocked using jest.mock('path/to/Button', () => 'Button');.

Final thoughts

Jest is an integral part of all of Facebook's JavaScript code bases: we have many thousands of tests across our web and react-native platforms and use it in all our open source projects. It's heavily integrated with our own continuous integration platform. This means we are committed to make it work well for hundreds of engineers that use it every day as well as making it scalable and performant. The philosophy behind Jest is to provide an integrated unit test experience to make it easy and efficient to write and run tests - especially for React and react-native applications. Please read our performance blog post which highlights some constraints and unique design decisions and work that we have been doing.

Unfortunately we don't get to simply start over with something completely new and we need to incrementally evolve our systems. This also means we focus on bringing the entire community with us to the latest version because open source is not just code. Jest doesn't matter to me. What matters to me are the constraints we put on ourselves, the concepts we want to popularize like snapshot testing and we want to enable people to easily write effective tests in a stable and performant environment. Currently I think Jest has a bunch of major advantages and I'm sure other test frameworks will catch up and adopt similar constraints and concepts over time. I understand that not everyone agrees and testing is an area people are particularly opinionated about but I hope that this post helps build empathy and understanding.

I want to conclude by saying that in the last six months people's reaction to Jest has been predominantly positive as compared to the time before I worked on it. It is still far from perfect but we have a plan for an entire year worth of work sketched out and are excited to tackle the challenge with you (this isn't a hiring pitch but you should come work with me anyway, you are all awesome!).

I encourage you to try out Jest, especially with the defaults we set in create-react-app, and give it a chance.

cc @ryanflorence, @kentcdodds, @trevordmiller

@kentcdodds
Copy link
Contributor

I fully expect snapshot testing without the DOM to become the standard for React and react-native applications.

I hope you're right, that'd be awesome. I can't wait to use that feature.

[We] are happy to integrate other test runners apart from Jasmine.

That's good to hear!

I will have more to share on this soon but we are planning to disable automocking by default in the next (or next next) major release of Jest.

Also great news. Eager to hear more about it.

we have a plan for an entire year worth of work sketched out and are excited to tackle the challenge with you

Great news as well.

I'll give Jest a try. That snapshot testing sounds amazing. Thanks for this comment.


As for including it by default in CRA, I think let's see how people like it. If people don't or find it confusing, try to fix it. If it can't be done, then pull it out. Seems kind of low risk and heck:

its-free-software

Thanks for the work you all put into this stuff!

@iammerrick
Copy link

Hey @cpojer, thank you for your explanation. To be honest I realized that my experience with Jest was when it was first announced and I was very frustrated by it then and gave up adopting it. I think think running browser code in a browser is a good idea but maybe not worth the performance hit/flakiness.

Thank you for taking the time to help me understand how you are thinking about this and why CRA is doing this. To be honest your pitch for Jest was so good, it made me think, "Hey maybe we should move from mocha + karma to Jest" hehe.

Again thank you, not that I am entitled to this information or that my feelings about this matter but I am grateful and feel a lot better about this decision now.

@iammerrick
Copy link

Also, snapshot testing seems very cool reminds me of https://github.com/facebookarchive/huxley which was super cool but didn't take for some reason. :-)

@kentcdodds
Copy link
Contributor

I was thinking the same thing. Huxley was awesome because you could see the changes. This will be much faster though 👍

@iammerrick
Copy link

Agreed @kentcdodds, snapshot tests should be more reliable and less brittle too since rendering engine overhead/glitches won't fire false errors, etc. This is lower on the pyramid and less brittle which is nice!

@cpojer
Copy link
Contributor Author

cpojer commented Aug 3, 2016

Thanks for the kind words. I want to stress that Jest isn't perfect yet and if you plan to try it out on a bigger project you are undoubtedly going to run into an issue. I'm happy to help personally to get you started and I want to learn what we can do better for you. Our configuration system is in dire need of a rewrite (@DmitriiAbramov is thinking about it) and if you'd like to be part of the discussion to redesign it, please let me know. We are happy to do it out in the open. At Facebook, we have JS Infra teams that own the configuration and setup of their projects so naturally no one ever has to fight with it internally and it is something we traditionally aren't doing as well on as we should. Ironically we are discussing this on a project's issue tracker that was created exactly because of this situation.

I want to point out that we will go where you go and our goal is to make testing easier, not harder. Let's figure this out together.

@trevordmiller
Copy link

@cpojer Thank you for the open communication and thoughtful responses. I am definitely willing to give Jest another shot as it sounds like it has improved and is headed in a nice direction. I'll look forward to trying it out. Is the issue tracker at https://github.com/facebook/jest the best place to post feedback?

@cpojer
Copy link
Contributor Author

cpojer commented Aug 3, 2016

Yes. I recommend joining our discord channel and talking to me directly. Live discussions are usually much more useful and have better turnaround times.

@trevordmiller
Copy link

@cpojer Awesome :) Thank you. Which Discord channel?

@cpojer
Copy link
Contributor Author

cpojer commented Aug 3, 2016

I'm sure you will #jest be fine looking for it yourself.

@irvinebroque
Copy link

We use jest at Intercom and are extremely happy with it. I remember trying to get it working 2 years ago, and I can't stress enough how much it's changed for the better since then. Highly recommend giving it another go if you're skeptical.

@carcer
Copy link

carcer commented Aug 3, 2016

Just to balance @irvinebroque

We use Jest in a medium sized codebase, and it's ok.

Automocking is an anti-pattern (it was a fad in 07-08 in the .Net ecosystem and we quickly learnt it caused problem), however we inherited the codebase, and they had very much gone down that path. The lack of commitment to improvement from Facebook up to now has been frustrating. It can be dog slow (we have 1266 tests, and I can heat my flat with the hot air the fans spit out if the suite runs a few times in a row). It used to take over 200s to run, which has came down to 69s with Jest upgrades over the past 18months. We aren't using the new snapshot feature yet, this may reduce the run time further.

Having Jest as the default testing framework for this makes sense, as its a FB product. However, might I suggest that it should be possible to eject only the testing configuration, to allow for swapping that out, while keeping the rest of the app managed.

@gaearon
Copy link
Contributor

gaearon commented Aug 3, 2016

You don't technically need to "eject" testing configuration because it would be equivalent to just using your own test runner. Since testing is fairly independent all you need to do is to change test to run something else.

It can be dog slow (we have 1266 tests, and I can heat my flat with the hot air the fans spit out if the suite runs a few times in a row).

Do you use watching or do you mean consecutive full runs?

@Phoenixmatrix
Copy link

👍 on Jest.

The goal of create-react-app, as I see it at least, is to have the "batteries included", and Jest is really good at this. I've had some experience introducing people who had never done unit testing before (not even in Java, etc), pointed them at Jest with a few tips to set it up, and they were up and running in no time.

For the longest time I was a Mocha guy and dismissed Jest, wanting browser support, complained about slowness, blah blah. In the last couple of months I had a change of heart.

With recent tooling, node-only test debugging is now pretty easy (eg: iron-node). Also, headless browsers are not close enough to "real" browsers to really be meaningful. And real browsers are poor choices for CI of unit tests (actual unit tests, not e2e or integration tests). So having a suite of "pure" unit tests is easier and better, and you can couple it with a suite of e2e tests running in real browsers with karma or whatsnot.

The test isolation in Jest solves a very common problem where tests leak into each other that plagues begginers. The coverage report of only things you meant to test is also a pretty clever way to get relevent metrics. And now we have snapshot tests.

Even in the original version of Jest, performance would not be a big deal for 99% of users (I like to test everything and end up with several thousand tests, but in the age of micro apps and people only testing a subset of functionalities, the vast majority of users will never hit a threshold where performance is an issue). Even if it was, Jest is decently fast now.

The automocks may be seen by an anti-pattern by some, but its easy to disable, and the implementation in Jest is actually pretty good and flexible. Working with newcomers, I've had a lot of success with it.

I'd dare say, karma+browser for unit tests is the anti-pattern, even if using Mocha or whatsnot, for unit tests you'd want the tests running in node.

The one gotcha, and I mentioned it in a few channels before: they have to commit to keeping Jest working well on Windows. Even though I don't use Windows to develop, a very large segment of the community, especially create-react-app's target demographic, is on Windows, and there's been a lot of hiccups on that side before. Mocha + Chai + Webpack work perfectly on WIndows, so it won't take too many hiccups before people say this was a mistakes.

@gaearon
Copy link
Contributor

gaearon commented Aug 3, 2016

they have to commit to keeping Jest working well on Windows. Even though I don't use Windows to develop, a very large segment of the community, especially create-react-app's target demographic, is on Windows, and there's been a lot of hiccups on that side before.

We definitely want to keep an eye on this.
Create React App is committed to full Windows support for all its features.

@Daniel15
Copy link
Member

Daniel15 commented Aug 3, 2016

they have to commit to keeping Jest working well on Windows

There's not many Windows users at Facebook. I'm one of them, I actively use Nuclide, Jest, etc. on Windows and submit bug fixes 😄 . Some projects use AppVeyor for continuous integration in a Windows environment to ensure that the Windows builds don't break. Not sure if Jest is using AppVeyor yet, but it's probably worth configuring.

@Phoenixmatrix
Copy link

We definitely want to keep an eye on this.
Create React App is committed to full Windows support for all its features.

Yeah, so if it goes the Jest route, that forces the Jest team to have the same commitment (and Linux subsystem for Windows that was released officially yesterday is an escape hatch, but doesn't cover all use cases, so it can't be 100% relied upon).

If they are though, Jest is really the best solution for a project like this ❤️

@cpojer
Copy link
Contributor Author

cpojer commented Aug 4, 2016

@carcer thanks for your feedback. Are you talking about 1200 tests or 1200 test suites (individual files)? Generally, any large company will always run into trouble with any test framework if they don't actively work on the test infra themselves, that's just natural. I am planning to eventually switch FB off of automocking entirely and there will likely be codemods that we can share. I recommend for new tests to start off with jest.disableAutomock() and slowly convert old ones when you touch them. We have found this to work well at FB if we provide a few default mocks in our setup files. There are also a bunch of knobs you can turn to get your test suite to be faster, and jest -o should only run affected test files during development to run the minimal amount of tests necessary. Generally, Jest has a lot of room to optimize performance once you get to a bigger scale. I'm also happy to visit your office in person and talk about this and maybe give a talk if you think this is impactful for you and your colleagues.

Replacing Jest

We aren't adding any configuration to the apps created by CRA so swapping Jest for something else only requires to update the scripts.test field in your package.json, there is no overhead or clutter that sticks around.

Windows Support

We are of course committed to supporting windows users well. @Daniel15 has been helping out a lot. If something isn't working well, please bring it up to us and we'll fix it. If someone wants to help fix up Jest's integration tests for a Windows CI and get them running there properly, that would be appreciated as well and makes us more proactive instead of reactive to issues.

@gaearon
Copy link
Contributor

gaearon commented Aug 31, 2016

Thanks a lot to everyone in this thread for your feedback.

Automocking is now off by default in Jest 15.
The output has been vastly improved too.

Interactive watch mode:

Object diffs and failure output (before and after):

Console output (before and after):

Read more about Jest 15.

@kentcdodds
Copy link
Contributor

I'm SO excited to try this out!

@trevordmiller
Copy link

@gaearon @cpojer Much better :) Would be cool if instructions for testing with CRA could be added to https://github.com/facebookincubator/create-react-app/tree/master/template. I would submit a PR, but I'm swamped right now.

@gaearon
Copy link
Contributor

gaearon commented Aug 31, 2016

Testing with CRA is not out in an official release, that’s why it’s undocumented.
I’m waiting for a couple more small things before cutting it, and I’ll document it when I do.

@gaearon gaearon mentioned this pull request Sep 1, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.