Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(coverage): use project specific vitenode for uncovered files #6044

Merged
Merged
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
4 changes: 1 addition & 3 deletions docs/guide/workspace.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,4 @@ All configuration options that are not supported inside a project config have <N

## Coverage

Coverage for workspace projects works out of the box. But if you have [`all`](/config/#coverage-all) option enabled and use non-conventional extensions in some of your projects, you will need to have a plugin that handles this extension in your root configuration file.

For example, if you have a package that uses Vue files and it has its own config file, but some of the files are not imported in your tests, coverage will fail trying to analyze the usage of unused files, because it relies on the root configuration rather than project configuration.
Coverage for workspace projects works out of the box.
4 changes: 3 additions & 1 deletion packages/coverage-istanbul/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,13 +414,15 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
const cacheKey = new Date().getTime()
const coverageMap = libCoverage.createCoverageMap({})

const transform = this.createUncoveredFileTransformer(this.ctx)

// Note that these cannot be run parallel as synchronous instrumenter.lastFileCoverage
// returns the coverage of the last transformed file
for (const [index, filename] of uncoveredFiles.entries()) {
debug('Uncovered file %s %d/%d', filename, index, uncoveredFiles.length)

// Make sure file is not served from cache so that instrumenter loads up requested file coverage
await this.ctx.vitenode.transformRequest(`${filename}?v=${cacheKey}`)
await transform(`${filename}?v=${cacheKey}`)
const lastCoverage = this.instrumenter.lastFileCoverage()
coverageMap.addFileCoverage(lastCoverage)
}
Expand Down
3 changes: 2 additions & 1 deletion packages/coverage-v8/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
const transformResults = normalizeTransformResults(
this.ctx.vitenode.fetchCache,
)
const transform = this.createUncoveredFileTransformer(this.ctx)

const allFiles = await this.testExclude.glob(this.ctx.config.root)
let includedFiles = allFiles.map(file =>
Expand Down Expand Up @@ -401,7 +402,7 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
const { originalSource } = await this.getSources(
filename.href,
transformResults,
file => this.ctx.vitenode.transformRequest(file),
transform,
)

const coverage = {
Expand Down
32 changes: 32 additions & 0 deletions packages/vitest/src/utils/coverage.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { relative } from 'pathe'
import mm from 'micromatch'
import type { CoverageMap } from 'istanbul-lib-coverage'
import type { Vitest } from '../node/core'
AriPerkkio marked this conversation as resolved.
Show resolved Hide resolved
import type { BaseCoverageOptions, ResolvedCoverageOptions } from '../node/types/coverage'
import { resolveCoverageReporters } from '../node/config/resolveConfig'

Expand Down Expand Up @@ -272,6 +273,37 @@ export class BaseCoverageProvider {
return chunks
}, [])
}

createUncoveredFileTransformer(ctx: Vitest) {
const servers = [
...ctx.projects.map(project => ({
root: project.config.root,
vitenode: project.vitenode,
})),
// Check core last as it will match all files anyway
{ root: ctx.config.root, vitenode: ctx.vitenode },
]

return async function transformFile(filename: string) {
let lastError

for (const { root, vitenode } of servers) {
if (!filename.startsWith(root)) {
continue
}

try {
return await vitenode.transformRequest(filename)
sheremet-va marked this conversation as resolved.
Show resolved Hide resolved
}
catch (error) {
lastError = error
}
}

// All vite-node servers failed to transform the file
throw lastError
Copy link
Member

Choose a reason for hiding this comment

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

Hm, what if the file never started with root? Is it possible? Then this will be undefined

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh good point! This might fail with allowExternal files. Ill need to add test case for those.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #6372 as follow-up. I didn't yet look into this a lot, created PR mostly so that I won't forget this issue.

}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { readFileSync } from "node:fs";
import { Plugin, defineWorkspace } from "vitest/config";
import MagicString from "magic-string";

export default defineWorkspace([
// Project that uses its own "root" and custom transform plugin
{
test: {
name: "custom-with-root",
root: "fixtures/workspaces/custom-2",
},
plugins: [customFilePlugin("2")],
},

// Project that cannot transform "*.custom-x" files
{
test: {
name: "normal",
include: ["fixtures/test/math.test.ts"],
},
},

// Project that uses default "root" and has custom transform plugin
{
test: {
name: "custom",
include: ["fixtures/test/custom-1-syntax.test.ts"],
},
plugins: [customFilePlugin("1")],
},
]);

/**
* Plugin for transforming `.custom-1` and/or `.custom-2` files to Javascript
*/
function customFilePlugin(postfix: "1" | "2"): Plugin {
function transform(code: MagicString) {
code.replaceAll(
"<function covered>",
`
function covered() {
return "Custom-${postfix} file loaded!"
}
`.trim()
);

code.replaceAll(
"<function uncovered>",
`
function uncovered() {
return "This should be uncovered!"
}
`.trim()
);

code.replaceAll("<default export covered>", "export default covered()");
code.replaceAll("<default export uncovered>", "export default uncovered()");
}

return {
name: `custom-${postfix}-file-plugin`,
transform(_, id) {
const filename = id.split("?")[0];

if (filename.endsWith(`.custom-${postfix}`)) {
const content = readFileSync(filename, "utf8");

const s = new MagicString(content);
transform(s);

return {
code: s.toString(),
map: s.generateMap({ hires: "boundary" }),
};
}
},
};
}
5 changes: 5 additions & 0 deletions test/coverage-test/fixtures/src/covered.custom-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<function covered>

<function uncovered>

<default export covered>
3 changes: 3 additions & 0 deletions test/coverage-test/fixtures/src/uncovered.custom-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<function uncovered>

<default export uncovered>
8 changes: 8 additions & 0 deletions test/coverage-test/fixtures/test/custom-1-syntax.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { expect, test } from 'vitest'

// @ts-expect-error -- untyped
import output from '../src/covered.custom-1'

test('custom file loads fine', () => {
expect(output).toMatch('Custom-1 file loaded!')
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<function covered>

<function uncovered>

<default export covered>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<function uncovered>

<default export uncovered>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { expect, test } from 'vitest'

// @ts-expect-error -- untyped
import output from '../src/covered.custom-2'

test('custom-2 file loads fine', () => {
expect(output).toMatch('Custom-2 file loaded!')
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
{
"path": "<process-cwd>/fixtures/src/covered.custom-1",
"statementMap": {
"0": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 18
}
},
"1": {
"start": {
"line": 3,
"column": 0
},
"end": {
"line": 3,
"column": 20
}
}
},
"fnMap": {
"0": {
"name": "covered",
"decl": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 18
}
},
"loc": {
"start": {
"line": 1,
"column": 0
},
"end": {
"line": 1,
"column": 18
}
}
},
"1": {
"name": "uncovered",
"decl": {
"start": {
"line": 3,
"column": 0
},
"end": {
"line": 3,
"column": 20
}
},
"loc": {
"start": {
"line": 3,
"column": 0
},
"end": {
"line": 3,
"column": 20
}
}
}
},
"branchMap": {},
"s": {
"0": 1,
"1": 0
},
"f": {
"0": 1,
"1": 0
},
"b": {}
}
Loading
Loading