feat: implement istanbul coverage support for browser testing

[sheremet-va]

This PR adds coverage support for istanbul provider. It is currently not possible to use c8 as a coverage provider when running in the browser.

Also closes #3086

[AriPerkkio]

I'm really excited about these browser testing features!

I'll need to check how WebDriverIO can be set up on MacOS before looking at the PR's code:

> vitest run --browser chrome

Error: Failed to find a Chrome installation via:
$ which chrome: not found: chrome
$ which chromium: not found: chromium
$ which google-chrome: not found: google-chrome

I don't think it's possible to make c8 work [...]

In #2786 we removed most of the Node related APIs from c8 provider. Same V8 profiler commands work on Chromium based browsers when they are spawned with certain exec args. But let's first focus on the istanbul provider and see if c8 for Chromium browsers is worth the work later.

[sheremet-va]

I'll need to check how WebDriverIO can be set up on MacOS before looking at the PR's code:

I would recommend running pnpm coverage in test/browser and setup browser of your choice in the config. The browser itself should not matter.

I managed to make coverage work with --isolate flag, but if I run all tests it seems there is some kind of a race condition that empties coverage before the function is actually running. I wonder why don't we have the same issue in a worker? Maybe because we empty the module cache?

[AriPerkkio]

some kind of a race condition that empties coverage before the function is actually running

This might be causing that:

vitest/packages/coverage-istanbul/src/index.ts

Lines 12 to 14 in be34451

	// Reset coverage map to prevent duplicate results if this is called twice in row
	// @ts-expect-error -- untyped global
	globalThis[COVERAGE_STORE_KEY] = {}

I got the test/browser running fine now. I'll look into this tomorrow.

[AriPerkkio]

Looks like the fix: colec and run only a single test at a time fixed the race condition - great!

I think --browser --coverage support should be tested under test/coverage instead as we have good set of test cases there already. Below are some suggestions ready for copy-pasting:

// test/coverage-test/package.json
{
  "name": "@vitest/test-coverage",
  "private": true,
  "scripts": {
    "test": "pnpm test:c8 && pnpm test:istanbul && pnpm test:custom && pnpm test:browser && pnpm test:types",
    "test:c8": "node ./testing.mjs --provider c8",
    "test:custom": "node ./testing.mjs --provider custom",
    "test:istanbul": "node ./testing.mjs --provider istanbul",
    "test:browser": "node ./testing.mjs --browser --provider istanbul",
    "test:types": "vitest typecheck --run --reporter verbose"
  },
  "devDependencies": {
    "@vitejs/plugin-vue": "latest",
    "@vitest/browser": "workspace:*",
    "@vue/test-utils": "latest",
    "happy-dom": "latest",
    "vite": "latest",
    "vitest": "workspace:*",
    "vue": "latest",
    "webdriverio": "^8.5.5"
  }
}

// test/coverage-test/testing.mjs
import { startVitest } from 'vitest/node'

// Set this to true when intentionally updating the snapshots
const UPDATE_SNAPSHOTS = false

const provider = process.argv[1 + process.argv.indexOf('--provider')]
const isBrowser = process.argv.includes('--browser')

const configs = [
  // Run test cases. Generates coverage report.
  ['test/', {
    include: ['test/*.test.*'],
    exclude: [
      'coverage-report-tests/**/*',
      // TODO: Enable once mocking is supported in browser
      isBrowser && '**/no-esbuild-transform.test.js',
      // TODO: Include this and fix the error
      isBrowser && '**/coverage.test.ts',
    ].filter(Boolean),
    coverage: { enabled: true },
    browser: { enabled: isBrowser, name: 'chrome', headless: true },
  }],

  // Run tests for checking coverage report contents.
  ['coverage-report-tests', {
    include: [
      ['c8', 'istanbul'].includes(provider) && './coverage-report-tests/generic.report.test.ts',
      `./coverage-report-tests/${provider}.report.test.ts`,
    ].filter(Boolean),
    coverage: { enabled: false, clean: false },
  }],
]

for (const threads of [{ threads: true }, { threads: false }, { singleThread: true }]) {
  for (const isolate of [true, false]) {
    for (const [directory, config] of configs) {
      await startVitest('test', [directory], {
        name: `With settings: ${JSON.stringify({ ...threads, isolate, directory, browser: isBrowser })}`,
        ...config,
        update: UPDATE_SNAPSHOTS,
        ...threads,
        isolate,
      })

      if (process.exitCode)
        process.exit()
    }
  }
}

When the coverage.test.ts is included there is an error coming from vite:import-analysis plugin:

[vite] Internal server error: Failed to resolve import "../src/index.mjs" from "test/coverage.test.ts?v=1679399336218". Does the file exist?
  Plugin: vite:import-analysis
  File: /x/vitest/test/coverage-test/test/coverage.test.ts?v=1679399336218:2:27
  1  |  import { expect, test } from "vitest";
  2  |  import { pythagoras } from "../src/index.mjs";
     |                              ^
  3  |  import { implicitElse } from "../src/implicitElse";
  4  |  import { useImportEnv } from "../src/importEnv";
      at formatError (file:///x/vitest/node_modules/.pnpm/vite@4.0.0_@types+node@18.7.13/node_modules/vite/dist/node/chunks/dep-ed9cb113.js:42301:46)
      at TransformContext.error (file:///x/vitest/node_modules/.pnpm/vite@4.0.0_@types+node@18.7.13/node_modules/vite/dist/node/chunks/dep-ed9cb113.js:42297:19)
      at normalizeUrl (file:///x/vitest/node_modules/.pnpm/vite@4.0.0_@types+node@18.7.13/node_modules/vite/dist/node/chunks/dep-ed9cb113.js:40600:33)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async TransformContext.transform (file:///x/vitest/node_modules/.pnpm/vite@4.0.0_@types+node@18.7.13/node_modules/vite/dist/node/chunks/dep-ed9cb113.js:40753:47)
      at async Object.transform (file:///x/vitest/node_modules/.pnpm/vite@4.0.0_@types+node@18.7.13/node_modules/vite/dist/node/chunks/dep-ed9cb113.js:42572:30)
      at async loadAndTransform (file:///x/vitest/node_modules/.pnpm/vite@4.0.0_@types+node@18.7.13/node_modules/vite/dist/node/chunks/dep-ed9cb113.js:40378:29)

[sheremet-va]

I think --browser --coverage support should be tested under test/coverage instead as we have good set of test cases there already. Below are some suggestions ready for copy-pasting.

Agreed, coverage should be tested there as a separate feature.

When the coverage.test.ts is included there is an error coming from vite:import-analysis plugin

I am not sure why there is an error 🤔

[sheremet-va]

@AriPerkkio seems like test/watch is failing consistently 🤔

[AriPerkkio]

Either the watch mode is broken on Windows or the Windows CI just cannot run the test case.

It fails even when CI runs only test/watch set, so it shouldn't be caused by high CPU load or anything. https://github.com/AriPerkkio/vitest/actions/runs/4540634358/jobs/8001791530

Should we just disable the test on Windows CI?

[sheremet-va]

Should we just disable the test on Windows CI?

We do this for other execa tests, so I don't see the reason not to 🤔

[AriPerkkio]

This removal of named export getProvider caused a breaking change. The @vitest/coverage-istanbul is still exporting it as both named and default. Was this intentional @sheremet-va?

There are some reports where users are using different versions of vitest and @vitest/coverage-c8 and the API breaks:

• Vitest@0.29.8 can not genetate the coverage report #3125
• 0.29.8 seems to have broken C8 unit test coverage reporting #3118

[sheremet-va]

Yeah, not intentional. Should've either not remove it or update peerDependencies. The next version is locked to be 0.30.0 though, so we will have to wait for a fix, but PR is welcome.

[AriPerkkio]

As a fix we can just bump the peerDependency version for 0.30.0 release and it will prevent the usage of mixed versions. I'll set PR soon.

I'm also thinking about a feature that would include the versions of vitest and @vitest/* packages in the logs of unhandled crashes, like seen here on #3118 and here on #3125. It would help users to see that they are not using the latest versions when they think they are.