Directory Provided by -r Flag Not Used as Coverage Root

[alexlafroscia]

Describe the bug

When running vitest from a directory that is not the one that contains the config file, and using the -r flag to point to another directory that should be treated as the "project root", c8 uses the current working directory as the "root" to gather coverage from instead of the actual project root.

The result of this is that, depending on which directory you run the command from, your coverage report can include a bunch of files that it shouldn't!

This is impacting my team because of the way we're running our tests in CI, due to the lack of "projects" support in Vitest (that could test multiple packages all at once). We're running Vitest from the workspace root using the -r flag, but our coverage reports now include the files from the mono-repo siblings of the package actually under test.

Reproduction

This repository demonstrates the bug, as well as describing one (awkward) fix that I came across

https://github.com/alexlafroscia/__vitest-monorepo-coverage

System Info

System:
    OS: macOS 12.5
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 15.00 GB / 64.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.13.2 - ~/.volta/tools/image/node/16.13.2/bin/node
    Yarn: 1.22.17 - ~/.volta/tools/image/yarn/1.22.17/bin/yarn
    npm: 8.1.2 - ~/.volta/tools/image/node/16.13.2/bin/npm
    Watchman: 4.9.0 - /opt/brew/bin/watchman
  Browsers:
    Chrome: 104.0.5112.101
    Safari: 15.6
    Safari Technology Preview: 16.0

Used Package Manager

npm

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.

[AriPerkkio]

I think support from c8 is required as well: bcoe/c8#415

Istanbul provider is already passing the ctx.config.root to test-exclude.

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

Lines 61 to 67 in 88d5764

	this.testExclude = new _TestExclude({
	cwd: ctx.config.root,
	include: typeof this.options.include === 'undefined' ? undefined : [...this.options.include],
	exclude: [...defaultExclude, ...defaultInclude, ...this.options.exclude],
	excludeNodeModules: true,
	extension: configDefaults.coverage.extension,
	})

[sun0day]

A quick solution for Vitest is that scan ctx.config.root package.json and then add dependencies to ctx.config.coverage.exclude before construct C8CoverageProvider.

[AriPerkkio]

Looks like c8 has an undocumented resolve option which might solve this.

https://github.com/bcoe/c8/blob/7f1069dcce6821a6794557b39d6a13f620c64bad/lib/parse-args.js#L136-L139

@alexlafroscia I've tried to reproduce this issue using the reproduction repository. However I'm not seeing the expected erroneous results in the "Error case". In my tests the files of packages/a are never included in report.

Logs

Base case:

$ pwd
/workspaces/__vitest-monorepo-coverage/packages/b

$ npm test

> [email protected] test
> vitest run --coverage


 RUN  v0.22.1 /workspaces/__vitest-monorepo-coverage/packages/b
      Coverage enabled with c8

 ✓ src/add-and-double.test.js (1)

Test Files  1 passed (1)
     Tests  1 passed (1)
  Start at  06:35:48
  Duration  1.29s (transform 652ms, setup 0ms, collect 20ms, tests 3ms)

 % Coverage report from c8
-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |     100 |      100 |     100 |     100 |                   
 add-and-double.js |     100 |      100 |     100 |     100 |                   
-------------------|---------|----------|---------|---------|-------------------

$ pwd
/workspaces/__vitest-monorepo-coverage

$ npm test

> [email protected] test
> vitest run --coverage -r packages/b


 RUN  v0.22.1 /workspaces/__vitest-monorepo-coverage/packages/b
      Coverage enabled with c8

 ✓ src/add-and-double.test.js (1)

Test Files  1 passed (1)
     Tests  1 passed (1)
  Start at  06:34:55
  Duration  1.28s (transform 644ms, setup 0ms, collect 20ms, tests 2ms)

 % Coverage report from c8
-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |     100 |      100 |     100 |     100 |                   
 add-and-double.js |     100 |      100 |     100 |     100 |                   
-------------------|---------|----------|---------|---------|-------------------

[alexlafroscia]

Hey @AriPerkkio -- sorry about that! It seems I had checked in the "fix" that I found by mistake (which is documented at the end of the repo's README), which certainly makes reproducing this issue confusing!

I've removed that "fix" from the reproduction repo now

alexlafroscia/__vitest-monorepo-coverage@a985394

now running the tests at the workspace root should show the packages/a files in the coverage report

❯ pwd
/Users/alexlafroscia/Code/tmp/__vitest-monorepo-coverage

❯ npm test

> [email protected] test
> vitest run --coverage -r packages/b


 RUN  v0.22.1 /Users/alexlafroscia/Code/tmp/__vitest-monorepo-coverage/packages/b
      Coverage enabled with c8

 ✓ src/add-and-double.test.js (1)

Test Files  1 passed (1)
     Tests  1 passed (1)
  Start at  12:46:22
  Duration  1.52s (transform 444ms, setup 0ms, collect 15ms, tests 2ms)

 % Coverage report from c8
--------------------|---------|----------|---------|---------|-------------------
File                | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------|---------|----------|---------|---------|-------------------
All files           |     100 |      100 |     100 |     100 |
 a/src              |     100 |      100 |     100 |     100 |
  index.js          |     100 |      100 |     100 |     100 |
 b/src              |     100 |      100 |     100 |     100 |
  add-and-double.js |     100 |      100 |     100 |     100 |
--------------------|---------|----------|---------|---------|-------------------

[AriPerkkio]

Thanks @alexlafroscia, I'm able to reproduce the issue now.

We should really wait for support from c8's side for configuring the test-exclude. There is already an on-going PR which is adding a similar flag that could be used. I'm not really eager to add work-arounds like #1902 (comment).

Ideally vitest would be passing { cwd: ctx.config.root } (or projectRoot, depending on the final API name) here on coverage-c8/src/provider.ts#L49 and c8 would be using it here lib/report.js#L39. I tested these changes locally and it works perfectly.

[Jason3S]

@alexlafroscia,

I also have a mono-repo setup. To generate the coverage report, I run coverage in each package folder, not at the root. Once they are finished, they are gathering into a single lcov.info file.

<root>/package.json.scripts

"coverage-collect": "globcat packages/*/coverage/lcov.info > lcov.info",

The aggregate file is sent to Coveralls or CodeCov.

This works fine with Jest, using:

<root>/jest.config.js

    coverageReporters: ['html', 'json', ['lcov', { projectRoot: __dirname }], 'text'],

But with vitest the process doesn't work out of the box. Instead each lcov.info has to be patched.

For example:
All SF lines in <root>/packages/client/coverage/lcov.info like this:

SF:src/index.ts needs to be adjusted to SR:packages/client/src/index.ts

I would rather have the ability to tell instanbul or c8 the coverageRoot so they can correctly generate the lcov.info files without needing to patch them.

[AriPerkkio]

rather have the ability to tell istanbul or c8 the coverageRoot

@Jason3S If you can share a minimal reproduction repository for the istanbul case, I'm quite sure we can fix that. Clear reproduction steps describing what happens and what should happen, would be perfect for debugging this.

    coverageReporters: ['html', 'json', ['lcov', { projectRoot: __dirname }], 'text'],

The projectRoot is already used by Vitest's istanbul provider:

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

Lines 124 to 126 in 1997767

	reports.create(reporter as any, {
	skipFull: this.options.skipFull,
	projectRoot: this.ctx.config.root,

It was added in #2051.

coverage/lcov.info:
Before: SF:packages/b/src/add-and-double.js
After: SF:src/add-and-double.js

[AriPerkkio]

[email protected] will add support for passing reporter options. I think this issue reported by @alexlafroscia can then be fixed in Vitest. The release has not yet been made though.

I think it's also worth to add some kind of "Coverage in monorepositories" section to documentation, https://vitest.dev/guide/coverage.html.

[Jason3S]

@AriPerkkio,

Thank you for the quick response. I'll see if I can setup a sample repo.

But, you don't need a mono repo to test it out.

The generated <root>/package/client/coverage/coverage-final.json already contains full path names.

Here is a package vitest.config.ts file:

import { defineConfig } from 'vitest/config';
import * as path from 'path';

export default defineConfig({
    test: {
        // reporters: 'verbose',
        coverage: {
            // enabled: true,
            provider: 'istanbul',
            clean: true,
            all: false, // should be true.
            reporter: ['html', 'json', 'lcov', 'text'],
            exclude: [
                '**/*.test.*',
            ],
        },
        include: ['src/**/*.test.{ts,mts}'],
        exclude: ['content/**', 'fixtures/**', 'bin.mjs', '_snapshots_'],
        root: path.join(__dirname, '../..'),
        typecheck: {},
    },
});

run vitest run --coverage

It generates the report, but the lcov is relative to the package directory, not the root.

I tried changing root, but that doesn't seem to make a difference.

Note: it is possible to use nyc to generate the report afterwards.

cd package/client
nyc report --temp-dir "$(pwd)/coverage" --reporter lcov --report-dir "$(pwd)/cov" --cwd ../..

[AriPerkkio]

I tried changing root, but that doesn't seem to make a difference.

I see the problem now. Using Vitest's config.root as reporters' projectRoot can be used as default value but there are times when it needs to be overwritten. Exactly as here you would need to use config.root as <root>/packages/client and reporters' projectRoot as <root>/.

I've started working on a feature that will expose reporter configuration options in coverage.reporter. It will have identical API as Jest has.

reporter: [
   ["text", { "projectRoot": "../../" }],
   ["json", { "file": "renamed-report.json" }],
],

This can already be implemented for istanbul provider but for c8 we'll need to wait for the new release.
This will solve @Jason3S's case, but the original problem described in this issue still requires more support from c8.

[Jason3S]

@AriPerkkio,

Thank you.

[AriPerkkio]

@alexlafroscia now that the new @vitest/coverage-v8 is replacing @vitest/coverage-c8 and we are no longer limited by c8, this issue should be resolved. Could you test whether the new package works for you?

[AriPerkkio]

Closing as there has been no activity since last request. Both coverage providers should be able to provide this functionality now.

[Jason3S]

It is now working for me.
Thank you for adding the ability to pass parameters.