Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import assert from 'node:assert';
import type { TestRunner } from '../api';
import { DependencyChecker } from '../dependency-checker';
import { normalizeBrowserName } from './browser-provider';
import { getVitestBuildOptions } from './build-options';
import { VitestExecutor } from './executor';

Expand Down Expand Up @@ -50,7 +51,10 @@ const VitestTestRunner: TestRunner = {
}

if (options.coverage.enabled) {
checker.check('@vitest/coverage-v8');
checker.checkAny(
['@vitest/coverage-v8', '@vitest/coverage-istanbul'],
'Code coverage requires either "@vitest/coverage-v8" or "@vitest/coverage-istanbul" to be installed.',
);
}

checker.report();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,74 @@ async function findTestEnvironment(
}
}

function determineCoverageProvider(
browser: BrowserConfigOptions | undefined,
testConfig: InlineConfig | undefined,
optionsCoverageEnabled: boolean | undefined,
projectSourceRoot: string,
): 'istanbul' | 'v8' | 'custom' | undefined {
let determinedProvider = testConfig?.coverage?.provider;
if (!determinedProvider && (optionsCoverageEnabled || testConfig?.coverage?.enabled)) {
const browsersToCheck = getBrowsersToCheck(browser, testConfig?.browser);

const hasNonChromium =
browsersToCheck
.map((b) => normalizeBrowserName(b).browser)
.filter((b) => !['chrome', 'chromium', 'edge'].includes(b)).length > 0;
Comment on lines +82 to +85
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using '.some()' is more efficient and readable than '.filter().length > 0' as it short-circuits as soon as a match is found. Additionally, performing the normalization inside the predicate avoids creating an intermediate array.

Suggested change
const hasNonChromium =
browsersToCheck
.map((b) => normalizeBrowserName(b).browser)
.filter((b) => !['chrome', 'chromium', 'edge'].includes(b)).length > 0;
const hasNonChromium = browsersToCheck.some((b) => {
const normalized = normalizeBrowserName(b).browser;
return !['chrome', 'chromium', 'edge'].includes(normalized);
});


if (hasNonChromium) {
determinedProvider = 'istanbul';
} else {
const projectRequire = createRequire(projectSourceRoot + '/');
const checkInstalled = (pkg: string) => {
try {
projectRequire.resolve(pkg);

return true;
} catch {
return false;
}
};
const hasIstanbul = checkInstalled('@vitest/coverage-istanbul');
const hasV8 = checkInstalled('@vitest/coverage-v8');

if (hasIstanbul && !hasV8) {
determinedProvider = 'istanbul';
} else {
determinedProvider = 'v8';
}
}
}

return determinedProvider;
}

function getBrowsersToCheck(
browser: BrowserConfigOptions | undefined,
testConfigBrowser: BrowserConfigOptions | undefined,
): string[] {
const browsersToCheck: string[] = [];

// 1. Check browsers passed by the Angular CLI options
const cliBrowser = browser as CustomBrowserConfigOptions | undefined;
if (cliBrowser?.instances) {
browsersToCheck.push(...cliBrowser.instances.map((i) => i.browser));
}

// 2. Check browsers defined in the user's vitest.config.ts
const userBrowser = testConfigBrowser as CustomBrowserConfigOptions | undefined;
if (userBrowser) {
if (userBrowser.instances) {
browsersToCheck.push(...userBrowser.instances.map((i) => i.browser));
}
if (userBrowser.name) {
browsersToCheck.push(userBrowser.name);
}
}

return browsersToCheck;
}
Comment on lines +114 to +138
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for collecting browsers to check has several issues:

  1. It appends browsers from both the CLI options and the Vitest configuration, even though the CLI options override the configuration. This can lead to incorrect provider detection (e.g., selecting Istanbul when only Chromium browsers are actually being used via CLI).
  2. It doesn't check the 'enabled' property of the configuration-based browser settings.
  3. It misses the 'name' property for CLI-provided browser options.

The suggested implementation correctly prioritizes the CLI override and respects the 'enabled' flag while maintaining type safety for configuration properties.

function getBrowsersToCheck(
  browser: BrowserConfigOptions | undefined,
  testConfigBrowser: BrowserConfigOptions | undefined,
): string[] {
  const activeConfig = (browser ?? (testConfigBrowser?.enabled ? testConfigBrowser : undefined)) as
    | CustomBrowserConfigOptions
    | undefined;

  if (!activeConfig) {
    return [];
  }

  const browsersToCheck: string[] = [];
  if (activeConfig.instances) {
    browsersToCheck.push(...activeConfig.instances.map((i) => i.browser));
  }
  if (activeConfig.name) {
    browsersToCheck.push(activeConfig.name);
  }

  return browsersToCheck;
}
References
  1. When refactoring code that handles configuration properties, ensure that any implicit type validation previously performed is still maintained, especially if the configuration is guaranteed to be type-safe by an upstream process.


export async function createVitestConfigPlugin(
options: VitestConfigPluginOptions,
): Promise<VitestPlugins[0]> {
Expand All @@ -89,6 +157,13 @@ export async function createVitestConfigPlugin(
async config(config) {
const testConfig = config.test;

const determinedProvider = determineCoverageProvider(
browser,
testConfig,
options.coverage.enabled,
projectSourceRoot,
);

if (reporters !== undefined) {
delete testConfig?.reporters;
}
Expand Down Expand Up @@ -155,8 +230,8 @@ export async function createVitestConfigPlugin(
(browser || testConfig?.browser?.enabled) &&
(options.coverage.enabled || testConfig?.coverage?.enabled)
) {
// Validate that enabled browsers support V8 coverage
validateBrowserCoverage(browser, testConfig?.browser);
// Validate that enabled browsers support the selected coverage provider
validateBrowserCoverage(browser, testConfig?.browser, determinedProvider);

projectPlugins.unshift(createSourcemapSupportPlugin());
setupFiles.unshift('virtual:source-map-support');
Expand Down Expand Up @@ -208,6 +283,7 @@ export async function createVitestConfigPlugin(
options.coverage,
testConfig?.coverage,
projectName,
determinedProvider,
),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...(reporters ? ({ reporters } as any) : {}),
Expand Down Expand Up @@ -434,25 +510,12 @@ interface CustomBrowserConfigOptions {
function validateBrowserCoverage(
browser: BrowserConfigOptions | undefined,
testConfigBrowser: BrowserConfigOptions | undefined,
provider?: string,
): void {
const browsersToCheck: string[] = [];

// 1. Check browsers passed by the Angular CLI options
const cliBrowser = browser as CustomBrowserConfigOptions | undefined;
if (cliBrowser?.instances) {
browsersToCheck.push(...cliBrowser.instances.map((i) => i.browser));
}

// 2. Check browsers defined in the user's vitest.config.ts
const userBrowser = testConfigBrowser as CustomBrowserConfigOptions | undefined;
if (userBrowser) {
if (userBrowser.instances) {
browsersToCheck.push(...userBrowser.instances.map((i) => i.browser));
}
if (userBrowser.name) {
browsersToCheck.push(userBrowser.name);
}
if (provider === 'istanbul') {
return;
}
const browsersToCheck = getBrowsersToCheck(browser, testConfigBrowser);

// Normalize and filter unsupported browsers
const unsupportedBrowsers = browsersToCheck
Expand All @@ -473,6 +536,7 @@ async function generateCoverageOption(
optionsCoverage: NormalizedUnitTestBuilderOptions['coverage'],
configCoverage: VitestCoverageOption | undefined,
projectName: string,
provider?: 'istanbul' | 'v8' | 'custom',
): Promise<VitestCoverageOption> {
let defaultExcludes: string[] = [];
// When a coverage exclude option is provided, Vitest's default coverage excludes
Expand All @@ -486,6 +550,7 @@ async function generateCoverageOption(
}

return {
provider,
excludeAfterRemap: true,
reportsDirectory:
configCoverage?.reportsDirectory ?? toPosixPath(path.join('coverage', projectName)),
Expand Down
36 changes: 36 additions & 0 deletions tests/e2e/tests/vitest/browser-coverage-istanbul.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { ng } from '../../utils/process';
import { applyVitestBuilder } from '../../utils/vitest';
import assert from 'node:assert';
import { installPackage } from '../../utils/packages';
import { expectFileToExist, readFile } from '../../utils/fs';
import { updateJsonFile } from '../../utils/project';

export default async function (): Promise<void> {
await applyVitestBuilder();

// Install ONLY Istanbul coverage package.
// This will trigger the auto-detection logic to use Istanbul even for Node tests.
await installPackage('@vitest/coverage-istanbul@4');

// Use the 'json' reporter to get a machine-readable output for assertions.
await updateJsonFile('angular.json', (json) => {
const project = Object.values(json['projects'])[0] as any;
const test = project['architect']['test'];
test.options = {
coverageReporters: ['json', 'text'],
};
});

// Run tests with coverage (defaults to Node/jsdom environment)
const { stdout } = await ng('test', '--no-watch', '--coverage');

// Verify that tests passed
assert.match(stdout, /1 passed/, 'Expected tests to run successfully.');

// Verify that coverage files are generated
const coverageJsonPath = 'coverage/test-project/coverage-final.json';
await expectFileToExist(coverageJsonPath);

const coverageSummary = JSON.parse(await readFile(coverageJsonPath));
assert.ok(Object.keys(coverageSummary).length > 0, 'Expected coverage report to not be empty.');
}
11 changes: 7 additions & 4 deletions tests/e2e/tests/vitest/browser-coverage-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import { unlink } from 'node:fs/promises';
export default async function (): Promise<void> {
await applyVitestBuilder();

// Install necessary packages to pass the provider check
// Install necessary packages to pass the browser provider check
await installPackage('playwright@1');
await installPackage('@vitest/browser-playwright@4');
await installPackage('@vitest/coverage-v8@4');

// === Case 1: Browser configured via CLI option ===
const error1 = await execAndCaptureError('ng', [
Expand All @@ -26,10 +25,12 @@ export default async function (): Promise<void> {
const output1 = stripVTControlCharacters(error1.message);
assert.match(
output1,
/Code coverage is enabled, but the following configured browsers do not support the V8 coverage provider: firefox/,
'Expected validation error for unsupported browser with coverage (CLI option).',
/Code coverage requires either "@vitest\/coverage-v8" or "@vitest\/coverage-istanbul" to be installed./,
'Expected validation error for missing coverage packages.',
);

await installPackage('@vitest/coverage-v8@4');

const configPath = 'vitest.config.ts';
const absoluteConfigPath = path.resolve(configPath);

Expand All @@ -41,6 +42,7 @@ export default async function (): Promise<void> {
import { defineConfig } from 'vitest/config';
export default defineConfig({
test: {
coverage: { provider: 'v8' },
browser: {
enabled: true,
name: 'firefox',
Expand Down Expand Up @@ -71,6 +73,7 @@ export default async function (): Promise<void> {
import { defineConfig } from 'vitest/config';
export default defineConfig({
test: {
coverage: { provider: 'v8' },
browser: {
enabled: true,
provider: 'playwright',
Expand Down
Loading