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

Simplify coverage.exclude patterns, remove coverage.all #6956

Open
4 tasks done
AriPerkkio opened this issue Nov 23, 2024 · 1 comment
Open
4 tasks done

Simplify coverage.exclude patterns, remove coverage.all #6956

AriPerkkio opened this issue Nov 23, 2024 · 1 comment
Labels
breaking change discussion p2-to-be-discussed Enhancement under consideration (priority)
Milestone

Comments

@AriPerkkio
Copy link
Member

AriPerkkio commented Nov 23, 2024

Clear and concise description of the problem

Setting good defaults for coverage.exclude is difficult. The coverage.include is even more difficult. By default coverage.include is ['*'] as we don't know where users' source code are.

Currently we have listed complex coverage.exclude pattern that includes all kinds of framework and tool specific patterns that aren't really Vitest related. We constantly get new requests to add there more tool specific files, like MSW, postcss, tailwind, etc. This is not maintainable in long run.

What (most) users don't realize is that they shouldn't be using coverage.exclude in the first place. Instead of defining every single *.config.* file of your project's root, you should be defining coverage.include.

├── vitest.config.js
├── tailwind.config.js
├── postcss.config.js
|
├── public
|  └── mockServiceWorker.js
|
├── src
|  ├── math.js
|  └── utils
|     └── geometry.js
└── test
   └── math.test.js

Here users shouldn't do coverage.exclude: ['tailwind.config.js', 'postcss.config.js, 'public/mockServiceWorker.js']. They should do just coverage.include: ['src']

Suggested solution

  • Make coverage.exclude really simple - just Vitest config, Vitest workspace files, test files, node_modules. Only Vitest related files that we control.
  • Keep coverage.include: ['*'] as default value. Vite/JS community lacks good conventions here.
  • When user defines coverage.exclude, throw an error if coverage.include is not defined. At this point most users should realize that they don't need complex patterns in coverage.exclude at all. Defining coverage.include is enough.
    • In the error message we can say that If you want Vitest V2 like functionality, set 'coverage.includes: ["*"]'. Hopefully at this point users see that they could narrow down source files here.

Alternative

Alternatively we could do similar option as Jest has: https://jestjs.io/docs/configuration#collectcoveragefrom-array

Their option forces users to define coverage.include, as by default they have coverage.all: false. By default Jest shows only files that were imported by test files. Users have to define uncovered files in collectCoverageFrom.

💡 We could remove coverage.all and by default only show files that were imported by tests. Then if coverage.includes is defined, show all covered+uncovered files that match those.

Additional context

No response

Validations

@AriPerkkio
Copy link
Member Author

Alternative

Alternatively we could do similar option as Jest has: https://jestjs.io/docs/configuration#collectcoveragefrom-array

Their option forces users to define coverage.include, as by default they have coverage.all: false. By default Jest shows only files that were imported by test files. Users have to define uncovered files in collectCoverageFrom.

💡 We could remove coverage.all and by default only show files that were imported by tests. Then if coverage.includes is defined, show all covered+uncovered files that match those.

This issue was discussed with the team and we think following approach should be best:

  • Remove coverage.all from public API
  • By default uncovered files are not shown, show only files that were imported by tests
  • When user defines coverage.include, show all files that match that
  • Remove all non-Vitest/Vite related values from coverage.exclude

This is pretty much same experience that Jest's collectCoverageFrom provides.

@AriPerkkio AriPerkkio changed the title Make coverage.include required if coverage.exclude is overriden Simplify coverage.exclude patterns, remove coverage.all Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change discussion p2-to-be-discussed Enhancement under consideration (priority)
Projects
Status: Approved
Development

No branches or pull requests

1 participant