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

Support Jest's coverage options #2825

Closed
nickserv opened this issue Jul 20, 2017 · 9 comments
Closed

Support Jest's coverage options #2825

nickserv opened this issue Jul 20, 2017 · 9 comments

Comments

@nickserv
Copy link
Contributor

nickserv commented Jul 20, 2017

I was surprised when I got a warning from npm test stating that Jest's coverage options aren't supported. As a prior Jest user I expected them to be supported since Jest's --coverage option works fine.

While I know I could just change my test script or eject, I think it would reduce confusion to support this out of the box. Is seems like there's a whitelist for options, how easy would it be to implement support for all the coverage* options?

Output

> react-scripts test --env=jsdom

Out of the box, Create React App only supports overriding these Jest options:

  • collectCoverageFrom
  • coverageReporters
  • coverageThreshold
  • snapshotSerializers.

These options in your package.json Jest configuration are not currently supported by Create React App:

  • collectCoverage
  • coveragePathIgnorePatterns

If you wish to override other Jest options, you need to eject from the default setup. You can do so by running npm ru
n eject but remember that this is a one-way operation. You may also file an issue with Create React App to discuss su
pporting more options out of the box.                                                                               

npm ERR! Test failed.  See above for more details.
@gaearon
Copy link
Contributor

gaearon commented Jul 20, 2017

I don’t think you need to put collectCoverage into package.json. Just run npm test -- --coverage as described here to get the coverage report.

I am also not sure why you need coveragePathIgnorePatterns. Isn’t collectCoverageFrom enough to specify where you want it to be collected?

@gaearon
Copy link
Contributor

gaearon commented Jul 20, 2017

(To clarify, I’m not sure I understand the purpose of putting them in package.json. Coverage makes test runs slow so I don’t recommend to always enable it. Therefore it makes more sense to me to teach people to supply it as a flag when they need it, rather than support it being always on.)

@nickserv
Copy link
Contributor Author

nickserv commented Jul 20, 2017

Thanks for the quick response.

I know I could use --coverage instead without having to change my package.json, I'd just rather have the same syntax supported by Jest unless there are technical reasons why this shouldn't be supported in create-react-app.

I understand that most people may not want to enable coverage options in package.json (the user guide even recommends against this). I personally don't have an issue using it on my projects, and in my opinion it would be more helpful for create-react-app to support as many of Jest's options as possible to ease migration and flexibility. I think it's fair to leave the docs as is either way, as the performance consequences of --coverage depend on the project and this may confuse users that are next to Jest's coverage feature.

@gaearon
Copy link
Contributor

gaearon commented Jul 20, 2017

What happens if you add it as option to package.json and then run npm test? Does it run watcher with coverage? Watcher with coverage produces very confusing output after changes.

@nickserv
Copy link
Contributor Author

nickserv commented Jul 20, 2017

Both plain Jest and react-script test display coverage results with the watcher, but you can hide it by setting --coverageReporters without text or text-summary. Personally I find it useful, though it is kind of confusing the way it's displayed when a change causes no tests to run (I think this should be fixed upstream).

By the way it may be worth noting that I occasionally upload my Jest coverage results to cloud services (like Coveralls). As a result I like having coverage enabled in all environments (for projects where it isn't too slow) so I can preview my coverage locally.

@nickserv
Copy link
Contributor Author

nickserv commented Jul 20, 2017

Whoops, I just double checked and react-scripts test --coverage actually doesn't run the watcher, it just runs tests once with coverage as if you ran jest --coverage. My mistake, I'm not sure if that functionality is intentional though.

@gaearon
Copy link
Contributor

gaearon commented Jul 20, 2017

It’s intentional because enabling coverage together with watcher produces a very weird output. After changing files, you’ll see 0% reported for everything that test re-run hasn’t touched.

@nickserv
Copy link
Contributor Author

Ah, I think I noticed that once but I didn't realize the cause. I think it's fair to discourage that but I still found it surprising that create-react-app disables this without warning. Could we propose UI improves to Jest upstream and have create-react-app modify less of Jest's configuration automatically?

@gaearon
Copy link
Contributor

gaearon commented Jan 8, 2018

Sure, it would be nice if Jest showed something more meaningful or ignored coverage altogether in watch mode. I'll close this issue but feel free to reach out to Jest.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants