fix(workspace): throw error when browser mode and @vitest/coverage-v8 are used

[AriPerkkio]

Description

This error is not thrown when browser mode is used with @vitest/coverage-v8. Browser tests start and show errors related node:inspector usage outside Node.

vitest/packages/vitest/src/node/config.ts

Lines 142 to 143 in 4ff7159

	if (resolved.coverage.provider === 'v8' && resolved.coverage.enabled && isBrowserEnabled(resolved))
	throw new Error('@vitest/coverage-v8 does not work with --browser. Use @vitest/coverage-istanbul instead')

The reason is that resolved configuration is for root configuration, not the workspace configuration. Coverage options are in root config, browser config in workspace config. At this scope we don't have access to those both I think.

// vitest.config.ts
import { defineConfig } from "vitest/config";

export default defineConfig({
  test: {
    coverage: {
      enabled: true,
    },
  },
});

// vitest.workspace.ts
import { defineWorkspace } from "vitest/config";

export default defineWorkspace([
  {
    test: {
      name: "Browser project",
      browser: {
        headless: true,
        enabled: true,
        name: "chrome",
      },
    },
  },
]);

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

[netlify]

✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.

Name	Link
🔨 Latest commit	e256152
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/65dc218fa06ca300082975ae

[hi-ogawa]

Thanks for investigation. I see why resolveConfig doesn't throw for workspace browser and your approach looks good to me.

As an alternative, I'm wondering if we can move up getSerializableConfig's root config merging:

vitest/packages/vitest/src/node/workspace.ts

Lines 330 to 332 in 366a78b

	return deepMerge({
	...this.config,
	coverage: this.ctx.config.coverage,

to before resolveConfig:

vitest/packages/vitest/src/node/workspace.ts

Lines 299 to 300 in 366a78b

	async setServer(options: UserConfig, server: ViteDevServer) {
	this.config = resolveConfig(this.ctx.mode, options, server.config)

Something like this?

this.config = resolveConfig(
  this.ctx.mode,
  {
    ...options,
    coverage: this.ctx.config.coverage,
  },
  server.config
);

I feel this is somewhat awkward, so I like doing checks during resolveWorkspace though.

[AriPerkkio]

Merging coverage options as early as in setServer's resolveConfig seems like the best option. Now we don't need to duplicate the logic of "browser with v8-coverage" checks.

The other configuration modifications done in getSerializableConfig are quite tricky as they may depend on configuration resolved in resolveConfig. Ideally the poolOptions should also be moved to setServer()'s scope but that doesn't seem possible now. I'll leave those as they were.

[hi-ogawa]

I see, throwing during WorkspaceProject.setServer propagates the error mostly same way, then that's good 👍

I didn't think about this approach when I was fixing workspace snapshot option #5199, but probably I could've done something similar.