feat: support istanbul coverage provider

[AriPerkkio]

Closes #1252

Documentation of the istanbul packages is not perfect. I had to do some analyzing of jest and nyc to get things working.

• https://github.com/facebook/jest/blob/main/packages/jest-reporters/src/CoverageReporter.ts
• https://github.com/istanbuljs/nyc/tree/master/lib

Creates common interface for coverage providers to implement and adds new coverage provider:

• onFileTransform:

  • c8: No file transforms needed
  • istanbul: Instrument files under testing with istanbul-lib-instrument. Coverage objects are added to globalThis.__VITEST_COVERAGE__

• onBeforeFilesRun:

  • c8: Instruct v8 to enable coverage collection by setting up process.env.NODE_V8_COVERAGE
  • istanbul: No actions needed

• onAfterSuiteRun:

  • c8: Write v8 coverage reports to file system using v8.takeCoverage()
  • istanbul: Collect the globalThis.__VITEST_COVERAGE__ from workers through birpc into main thread

• onAfterAllFilesRun:

  • c8: Create report using c8/lib/report
  • istanbul: Process and merge coverage objects with istanbul-lib-coverage and create report with istanbul-reports

Example

vitest/test/coverage-test/src/utils.ts: Istanbul on the left with 57% coverage, v8 on the right with 70% coverage:

[AriPerkkio]

This is still a work-in-progress but it might be worth to create a draft PR at this point to prevent duplicate work.

I think the refactor: create interface for coverage logic part is complete, but istanbul integration still requires work. I can continue this later on, but if anyone is eager to finish this PR let me know.

I think it's also good time for Vitest team to take a look at this overall and call for a stop if this doesn't look good.

[Demivan]

We can create @vitest/istanbul package that just reexports what is needed.

@vitest-dev/team What do you think?

[antfu]

Yes, it sounds great to me
#1676 (comment)

[wight554]

Would be cool to see this configurable to use custom instrumenters like https://github.com/kwonoj/swc-coverage-instrument/

[AriPerkkio]

This seems like a cool idea, thanks for improvement idea @wight554. However I think it might be best if we leave this customization out of this PR and introduce it in a separate feature afterwards.
Do you think there is anything important we need to take into consideration at this point? Leave some abstractions in place, etc?
My Rust knowledge is a bit rusty and to me it's not that clear what exactly import('swc-plugin-coverage-instrument') would return here. Does it match the API of istanbul-lib-instrument?

[wight554]

This seems like a cool idea, thanks for improvement idea @wight554. However I think it might be best if we leave this customization out of this PR and introduce it in a separate feature afterwards. Do you think there is anything important we need to take into consideration at this point? Leave some abstractions in place, etc? My Rust knowledge is a bit rusty and to me it's not that clear what exactly import('swc-plugin-coverage-instrument') would return here. Does it match the API of istanbul-lib-instrument?

I've checked how jest handles this, see https://github.com/facebook/jest/blob/24e0472ac41ea03cdd89ffaea8c5f410ecf6054f/packages/jest-transform/src/ScriptTransformer.ts#L410
In this case all that needs to be added is config value to skip instrumenter part and return unmodified map and code in onFileTransform

[wight554]

looks good overall, noticed few issues:

1. exclude option seems to be ignored when using instanbul
2. when setting provider to c8 explicitly vitest run --coverage skips coverage
3. vitest doesn't suggest installing missing istanbul packages and skips coverage instead
4. c8 randomly ignores coverage for 1 file in 100% covered code when using c8 with these changes

codebase I used for testing: https://github.com/wight554/blog-template
You might notice schemas are added to coverage report when using istanbul [1] if you try

Upd. I use typescript instead of esbuild for compiling tests but ts-jest does the same and uses default jest instrument, so shouldn't be an issue
Upd2. if matters:

  System:
    OS: Windows 10 10.0.22622
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
    Memory: 17.38 GB / 31.93 GB
  Binaries:
    Node: 16.14.0 - c:\program files\nodejs\node.EXE
    Yarn: 3.1.1 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.3.1 - c:\program files\nodejs\npm.CMD
  Browsers:
    Chrome: 103.0.5060.134
    Edge: Spartan (44.22621.436.0), Chromium (103.0.1264.71)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    vite: ^3.0.3 => 3.0.3
    vitest: 0.19.1 => 0.19.1

[antfu]

The overall direction looks great to me. Please ping me when you think the work on your side is done, and I can help to do the refactoring and make the packages more general as @vitest/coverage-istanbul and even @vitest/coverage-c8 later.

[AriPerkkio]

From my side all that is left to do is unit+manual testing and add documentation. I think everything else (besides refactoring into separate packages) is done. I'm happy to leave these tasks to other people as well. 😄

I'm currently travelling and won't be doing anything for the next ~10 days. @antfu if you'd like to start working on this now, feel free to take over the PR. 👍

[antfu]

Great, thanks! Have a good time traveling!

[AriPerkkio]

There's now some initial documentation and unit tests in place. I think the next logical step is to refactor these coverage providers into their own packages - #1676 (comment).

[sheremet-va]

There's now some initial documentation and unit tests in place. I think the next logical step is to refactor these coverage providers into their own packages - #1676 (comment).

I think we should have these packages:

• @vitest/coverage-v8
• @vitest/coverage-istanbul

Then Vitest checks for coverage field, if package is installed. We might also expose this as CoverageProvider for extending with custom provider.

[Demivan]

@antfu coverage-c8 and coverage-istanbul build is failing locally.

src/provider.ts(6,10): error TS2305: Module '"vitest/config"' has no exported member 'configDefaults'.

[Demivan]

https://github.com/vitest-dev/vitest/runs/7835819196?check_suite_focus=true#step:6:107

Need to add node to tsconfig types

[JounQin]

It should be noticed provider: 'istanbul' is required if the user wants to use istanbul, ideally, if @vitest/coverage-istanbul is detected, istanbul should be used by default IMO.

[antfu]

I thought about it as once, but it could be complex to handle, like if you have both c8 and Istanbul packages installed, or if we have more providers in the future. Also, it might be dangerous to have dependencies change the behavior implicitly.

[JounQin]

@antfu

But the current behavior is very strange.

When provide: 'Istanbul' is not enabled but @vitest/coverage-istanbul installed

 MISSING DEP  Can not find dependency '@vitest/coverage-c8'

✖ Do you want to install @vitest/coverage-c8? … no

There is no choice or notice to add provide: 'Istanbul'.

When provide: 'Istanbul' enabled with @vitest/coverage-istanbul installed

Coverage enabled with istanbul

This message is very redundant to appear every time.

if you have both c8 and Istanbul packages installed, or if we have more providers in the future

IMO, something like Coverage enabled with istanbul is just for this case, for the default fallback or conflict providers installed message.