fix!(coverage): remove .all, reworked include/exclude#7837
fix!(coverage): remove .all, reworked include/exclude#7837AriPerkkio merged 1 commit intovitest-dev:mainfrom
.all, reworked include/exclude#7837Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
3c1d9e7 to
0ddd0a1
Compare
0ddd0a1 to
1d7055c
Compare
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
@vitest/browser
@vitest/coverage-istanbul
@vitest/coverage-v8
@vitest/expect
@vitest/mocker
@vitest/pretty-format
@vitest/runner
@vitest/snapshot
@vitest/spy
@vitest/ui
@vitest/utils
vite-node
vitest
@vitest/web-worker
@vitest/ws-client
commit: |
1d7055c to
e1ac12d
Compare
| // Run again through picomatch as tinyglobby's exclude pattern is different ({ "exclude": ["math"] } should ignore "src/math.ts") | ||
| includedFiles = includedFiles.filter(file => this.isIncluded(file)) |
There was a problem hiding this comment.
We use picomatch when checking covered files, and tinyglobby when checking uncovered files from file system. To make coverage.include and coverage.exclude behave similarly, we need to run tinyglobby's results through picomatch again here. This feels like duplicate work.
Minimal repro below. @SuperchupuDev any suggestions how to make tinyglobby behave identical to picomatch here?
import { writeFileSync, rmSync, readdirSync, mkdirSync } from "node:fs";
import { resolve } from "node:path";
import { glob } from "tinyglobby";
import pm from "picomatch";
{
// Setup
rmSync("./src", { force: true, recursive: true });
mkdirSync("./src");
writeFileSync("./src/first.ts", "export const a = 1;", "utf8");
writeFileSync("./src/second.ts", "export const a = 2;", "utf8");
}
// Include everything inside src, but exclude anything that contains word "second"
const options = {
include: ["src"],
exclude: ["second"],
};
// ['/<root>/repros/tinyglobby/src/first.ts', '/<root>/repros/tinyglobby/src/second.ts']
const files = readdirSync(options.include[0]).map((file) => resolve(options.include[0], file));
const picomatchResult = files.filter((file) =>
pm.isMatch(file, options.include, {
ignore: options.exclude,
contains: true,
})
);
const tinyglobbyResult = await glob(options.include, {
ignore: options.exclude,
absolute: true,
});
console.log({ picomatchResult, tinyglobbyResult });
// {
// picomatchResult: [
// '/<root>/repros/tinyglobby/src/first.ts'
// ],
//
// tinyglobbyResult: [
// '/<root>/repros/tinyglobby/src/first.ts',
// '/<root>/repros/tinyglobby/src/second.ts'
// ]
// }There was a problem hiding this comment.
hi! i just woke up. will investigate in some hours. off the top of my head you might want to use expandDirectories: false as i believe vitest uses that everywhere else? if it doesn't match after that then it's likely a picomatch issue, as iirc behavior between picomatch.isMatch and picomatch(pattern)(file) can differ (or at least i've seen behavior differences at least once in the past). still have to test and confirm though
There was a problem hiding this comment.
We'll merge this for Vitest v4 as is, but in the future it would be nice to have part working just by using tinyglobby, without needing picomatch.
There was a problem hiding this comment.
@AriPerkkio sorry for the wait. after testing locally it looks like the reason why your example gives different results between tinyglobby and picomatch is because you are using the contains picomatch option, which tinyglobby does not expose as an option and neither does fast-glob. i suppose you can achieve the same functionality by wrapping the patterns with **/*<pattern>*/**. or you could move off contains and its behavior altogether
e1ac12d to
b243a72
Compare
Description
coverage.excludepatterns, removecoverage.all#6956test-excludewithmicromatchandtinyglobbytest-excludeis 3.6MB and depends onglobandminimatchvitestalready depends onpicomatchandtinyglobbyso we can get those free in coverage packagesBreaking changes:
coverage.alloption is removedcoverage.extensionsoption is removedcoverage.includewith a pattern.coverage.extensionsis removed, it is recommended to define extensions inincludepattern, e.g.include: ["src/**.{js,ts,tsx}"].Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.