Coverage toggling

[smith]

It would be nice to be able to toggle coverage checking (with --coverage/--no-coverage.)

Toggling the overlay is useful, but the slowest part of my test run is generating the coverage report. I'd like to be able to toggle this quickly to where I could normally leave it off but turn it back on when wanting to do coverage checking.

[seanpoulter]

Brilliant. We'd love your help getting that implemented @smith. I'm happy to help point out the code that we'll want to change. It'd be great to have a second set of eyes on the coverage and process restarting code as well -- both have some changes on the go.

[stephtr]

In my opinion adding a new vscode setting (like jest.collectCoverage) which can be flipped using a command would be the best way to achieve that. That way one also doesn't have to manually add jest.pathToJest if its default value works.

[smith]

facebook/jest#5836 adds what I think is needed to implement this.

[connectdotz]

I can see 2 coverage usage patterns:

• on-demand: like @smith mentioned, an occasional jest coverage run is sufficient, a command to toggle on/off both display and an 'one-off' jest run with coverage flag will be good enough.
• continuous: as users add more tests, they might want to see the coverage updated as each jest run. In this case, users probably should update their regular jest command line to append the --coverage flag, but we should still have a command to toggle the display in case the coverage is somewhat impacting development visual (my personal experience with code coverage display)...

In any case, I think the following logic should be able to cover both:

• have a command to toggle coverage: mainly to turn on/off coverage UI display. Optionally: it could be enhanced with keyboard shortcut or on-screen control for ease of use.
• if the display coverage command is "on" but we don't have the coverage report to show for, i.e. coverage report is not in the jest output: start a jest run with coverage flag with facebook/jest#5836 coverage option. If users already have the --coverage in their rgular jest command line as for contineous coverage reporting, i.e. we can find coverage report: we can omit this step all together.

@stephtr do you see any problem with this logic? If users can run regular jest without modifying jest.pathToJest, the runner (facebook/jest#5836) should be able to just append the --coverage flag without any new setting, wouldn't it?

[stephtr]

Assumed --coverage is not specified and one turns on the coverage overlay, shouldn't the overlay update continuously while it is showing up? If one enables the coverage overlay, he probably wants to increase coverage, in which case continuous updates would be useful.

I'm a bit dissatisfied with the idea that for viewing coverage (including the CodeLens) one just needs to execute the Toggle Coverage Overlay command, but for enabling the coverage CodeLens on startup, one has to edit the path configuration. Another possible solution would be to modify the setting showCoverageOnLoad to have the options none, CodeLens and CodeLens+Overlay. During runtime one can switch between those states through commands (we currently have the Toggle Coverage Overlay, which can be also assigned a keyboard shortcut, we could probably also add a small button into the status bar). Switching from none to CodeLens or CodeLens+Overlay would restart the runner with--coverage set, in the reverse direction it would remove the flag and when switching between CodeLens and CodeLens+Overlay it wouldn't change any parameters.

By the way, my implementation of the setting actually just forwarded it to facebook/jest#5836.

[seanpoulter]

Here's my two cents ...

@smith, nice work on the PR to alter jest-editor-support! 👍

@connectdotz, I'll second your two use cases. I think a quick pick menu would help us with the user interface to show the code coverage. We could bring it up from a keyboard command and/or a status bar item. That'd solve Nathan's use case pretty well.

I don't understand what you're dissatisfied with @stephtr. What path configuration are we changing?

I'm a bit dissatisfied with the idea that for viewing coverage (including the CodeLens) one just needs to execute the Toggle Coverage Overlay command, but for enabling the coverage CodeLens on startup, one has to edit the path configuration.

I understand that an extension setting to collect coverage could make it easier on the user initially but it's mixing concerns. Keep the Jest configuration where Jest wants it. If you're looking for where Jest is configured you've already go to check package.json, jest.config.js, and if there's a non-standard file in a --config argument. It's already a confusing mess, adding another place to look isn't going to help that user. What's going to help the user is a link to a paragraph in our docs that explains how to set it up and what the code coverage we're showing even means.

My final concern, if we're going to get smart about checking the Jest configuration to see if coverage has been configured we're going to have to check standard and non-standard config files, and probably not support react-scripts very well.

[stephtr]

Should we pause the discussion until the more urgent issues are fixed?

My dissatisfaction is with the idea that the user controls the coverage option as part of the pathToJest setting, which in my opinion should just contain the path or maximally flags which don't get changed regularly by the user.

We are already injecting quite a few settings into Jest, so the Runner isn't a new place for that. My plan would be that we aren't getting smart and don't check if coverage has been enabled but just inject it with Runner#L69 when necessary (e.g. when the current state of showCoverage(OnLoad) is true). The result would be that switching between collect-coverage/no-coverage is possible with a single command without having to edit the settings.