Relative paths in test.alias cause uncovered files in c8 reports

[AriPerkkio]

Describe the bug

There has been couple of reports that c8 coverage reports can be missing files. I finally received a reproduction repository and was able to debug this issue.

If user configures test.alias using relative paths, the coverage report from c8 can be missing files that are imported using the aliased path. Using absolute paths in test.alias works always. Does this look like a bug in rollup-plugin-alias or something that could be handled on Vitest's or Vite's side?

Relative path

├── src // This is aliased as @some-alias
|  ├── animals.test.ts // imports '@some-alias/animals'
|  ├── animals.ts
|  └── math.ts
├── test
|  └── nested
|     └── math.test.ts // imports '@some-alias/math'
└── vite.config.ts

export default defineConfig({
  test: {
    alias: {
      "@some-alias": "./src"
...

Tests run OK. Coverage report is empty.

% Coverage report from c8
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |
----------|---------|----------|---------|---------|-------------------

The rollup-plugin-alias used by Vite is here returning { id: './src/math' } for resolveId('@some-alias/math').

The final transformed test case contains following import:

const __vite_ssr_import_1__ = await __vite_ssr_import__("/@id/src/math");

The V8 coverage reports contain paths which are not File URLs. c8 does not pick these up.

{
  "scriptId": "761",
  "url": "src/math", // <-- Should be File URL with absolute path + extension
  "functions": [...]

Absolute path

import path from "path";

export default defineConfig({
  test: {
    alias: {
      "@some-alias": path.resolve("./src")
...

Coverage report is complete.

% Coverage report from c8
------------|---------|----------|---------|---------|-------------------
File        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
------------|---------|----------|---------|---------|-------------------
All files   |   68.18 |    57.14 |     100 |   68.18 |
 animals.ts |   72.72 |       50 |     100 |   72.72 | 3-5
 math.ts    |   63.63 |    66.66 |     100 |   63.63 | 7-10
------------|---------|----------|---------|---------|-------------------

Here rollup-plugin-alias is returning full path, including extension, { id: '/Users/x/y/vitest-c8-test-alias/src/math.ts' } for resolveId('@some-alias/math').

The final transformed test case contains following import:

const __vite_ssr_import_1__ = await __vite_ssr_import__("/src/math.ts");

The V8 coverage reports contain File URLs now. c8 can process these successfully:

{
  "scriptId": "761",
  "url": "file:///Users/x/y/vitest-c8-test-alias/src/math.ts",
  "functions": [...]
  ...

Reproduction

Make sure you download this as c8 does not work on Stackblitz: https://stackblitz.com/edit/vitest-dev-vitest-n4nrrs

System Info

System:
    OS: macOS 12.5
    CPU: (8) arm64 Apple M2
    Memory: 677.78 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 19.1.0 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.19.3 - /opt/homebrew/bin/npm
  Browsers:
    Brave Browser: 107.1.45.133
    Safari: 15.6

Used Package Manager

pnpm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[sheremet-va]

Does it matter what file is in __vite_ssr_import__? We are passing down __filename to vm.runScriptInThisContext, doesn't it use this for coverage? The method itself is defined here:

vitest/packages/vite-node/src/client.ts

Line 323 in d424c1f

__vite_ssr_import__: request,

[AriPerkkio]

Changing the context.__filename doesn't seem to do anything.

But the filename that is passed to vm.runInThisContext seems to be used by Node in the V8 report's url:

vitest/packages/vite-node/src/client.ts

Lines 347 to 351 in d424c1f

	const fn = vm.runInThisContext(code, {
	filename: fsPath,
	lineOffset: 0,
	columnOffset: -codeDefinition.length,
	})

The fsPath in these two cases is "src/math" and "/Users/x/y/vitest-c8-test-alias/src/math.ts":

Relative Path

{ "test": { "alias": "@some-alias": "./src" } }

const __vite_ssr_import_1__ = await __vite_ssr_import__("/@id/src/math");

vm.runInThisContext(code, {
  filename: "src/math",
  lineOffset: 0,
  columnOffset: -codeDefinition.length,
})

{
  "scriptId": "761",
  "url": "src/math", // Not OK
  "functions": [...]

Absolute path

{ test: { alias: "@some-alias": path.resolve("./src") } }

const __vite_ssr_import_1__ = await __vite_ssr_import__("/src/math.ts");

vm.runInThisContext(code, {
  filename: "/Users/x/y/vitest-c8-test-alias/src/math.ts",
  lineOffset: 0,
  columnOffset: -codeDefinition.length,
})

{
  "scriptId": "761",
  "url": "file:///Users/x/y/vitest-c8-test-alias/src/math.ts", // Works OK
  "functions": [

[sheremet-va]

And can we do something about it? I am not sure it's Vitest responsibility, since we fully rely on Vite 🤔 But it's maybe also out of scope for Vite, because everything works fine there 😄 Maybe we need to ask Vite people how they figure out what to do with /@id/ routes?

[sheremet-va]

It seems they unwrap it:

https://github.com/vitejs/vite/blob/47a78db49740daee873525de90d0e44b19883771/packages/vite/src/node/utils.ts#L71

Maybe we also just need to remove /@id/ part and call resolveId on it?

if (id.startsWith('/@id/')
  fsPath = await resolveId(fsPath) // I am using fsPath, because it's normalised already

[AriPerkkio]

Move resolveId a bit and apply changes below, coverage works:

vitest/packages/vite-node/src/client.ts

Line 196 in d424c1f

const request = async (dep: string) => {

const request = async (dep: string) => {
+    if (dep.startsWith('/@id/')) {
+      console.log('dep before', dep)
+      dep = await resolveId(dep.replace('/@id', ''))
+      console.log('dep now', dep)
+    }
    const depFsPath = toFilePath(normalizeRequestId(dep, this.options.base), this.root)

RUN  v0.25.3 /Users/x/v/vitest-c8-test-alias
Coverage enabled with c8

stdout | unknown test
dep before /@id/src/math
dep now /@fs/Users/x/v/vitest-c8-test-alias/src/math.ts

stdout | unknown test
dep before /@id/src/animals
dep now /@fs/Users/x/v/vitest-c8-test-alias/src/animals.ts

✓ src/animals.test.ts (1)
✓ test/nested/math.test.ts (1)

Test Files  2 passed (2)
Tests  2 passed (2)
Start at  11:14:05
Duration  1.06s (transform 680ms, setup 0ms, collect 22ms, tests 6ms)

% Coverage report from c8
------------|---------|----------|---------|---------|-------------------
File        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
------------|---------|----------|---------|---------|-------------------
All files   |   68.18 |    57.14 |     100 |   68.18 |                   
animals.ts  |   72.72 |       50 |     100 |   72.72 | 3-5               
math.ts     |   63.63 |    66.66 |     100 |   63.63 | 7-10              
------------|---------|----------|---------|---------|-------------------

Does this look like a good solution? Some tests do not pass.

[sheremet-va]

Maybe we can even use moduleGraph.ensureEntryFromUrl 🤔 It has all needed information 🤔

[sheremet-va]

Move resolveId a bit and apply changes below, coverage works:

We don't need to change ID, issue is only with filename? We should standardise fsPath, and don't touch ID. I will try to implement moduleGraph.ensureEntryFromUrl here.

[sheremet-va]

@AriPerkkio can you check #2439? I didn't unwrap ID, but it still seems to work.