Include collectCoverage=true by default for testing

[connorjs]

Current Behavior

tsdx test does not collect coverage by default

Desired Behavior

tsdx test should collect coverage by default (I think)

Suggested Solution

Include collectCoverage: true in createJestConfig.ts (permalink)

Who does this impact? Who is this for?

I expect this to be for all users. I find code coverage to help understand parts of a library/application which does not have test coverage. It does not present a perfect metric, but all teams on which I've worked value it.

Describe alternatives you've considered

I currently just include collectCoverage: true in my own jest.config.js file as described in customization.

Additional context

Using my own jest.config.js totally works for me, but I wanted to understand why this was not the default. Especially given the TSDX Jest config includes the collectCoverageFrom option. Am I missing something maybe? The Jest docs suggest this value defaults to false.

Because this retrofits all executed files with coverage collection statements, it may significantly slow down your tests.

Jest does call out the performance impact, but I'd like to understand if TSDX has an explicit opinion on whether coverage falls into "best practice" category.

[agilgur5]

Am I missing something maybe?

I think you're misremembering or misinterpreting how coverage is normally used.

During a development session, one may run tests anywhere from a few times to hundreds of times. Sometimes with --watch mode on where coverage reporting is only partial and is obtrusive. Coverage isn't going to be useful for a large portion of those runs, but it may be useful in the middle or toward the end. It's also best practice to run coverage in CI.

You generally don't want to run coverage for every test run, but every once in a while or toward the end of a session and in CI.

The Jest docs suggest this value defaults to false.

Yes, Jest defaults coverage reporting to false as do most, if not all, test frameworks. In JS world (Python too), many test runners don't even have coverage built-in and require separate tools like nyc. CRA also does not have coverage turned on by default.

Jest does call out the performance impact

Jest runs babel-plugin-istanbul over your code to instrument it, which can indeed be incredibly CPU and memory intensive, taking potentially several times longer than a plain test run (something I've personally experienced in https://github.com/facebook/jest/issues/9233). So it actually could be a quite detrimental DX to turn it on by default.

Especially given the TSDX Jest config includes the collectCoverageFrom option.

Yes, this is specified to make it easier to just run --coverage without needing to configure anything else for the default file structure. Per the above link, CRA similarly sets this option.

I'd like to understand if TSDX has an explicit opinion on whether coverage falls into "best practice" category.

To summarize, you should definitely use code coverage, but there's no need to run it for every single test run, and doing so could be detrimental to DX for several reasons mentioned above (perf, watch mode, etc).

You should also run coverage in CI. I've been looking to add historical coverage to the templates' GitHub Actions but need to look at what the latest providers/bots/Actions are. I've used Codecov in my TS/JS and Python libraries but that was before Actions were a thing and as a GitHub App it can't be added to a default template, that has to be manually authorized.

[connorjs]

Thanks so much for the in-depth reply/explanation. I'll dive deeper into understanding how to run this only in our CI environment. Thanks!

[agilgur5]

@connorjs I would suggest not setting collectCoverage in jest.config.js and instead running with --coverage in CI (--coverage is an alias for --collectCoverage). Similarly can do so in local development occasionally.

For GitHub Actions you can see this in the templates. For Travis CI, here's a line from one of my libraries.