[Bug] The sourceFilter option seems to not be honored

[ericmorand]

Describe the bug

The sourceFilter option seems to not be honored - if I understand well its meaning that is "the pattern used to identify the files that will be part of the coverage report".

To Reproduce

I created a repository to easily test and reproduce the issue: https://github.com/ericmorand/c8-with-bundle

Just execute npm mcr and wait for the report to be displayed: it includes every file that is referenced in the V8 coverage data (that is node_modules files), plus src/main/lib/foo.ts but excluding all the other files present in src/main that also matches the passed sourceFilter pattern src/main/**/*.ts (like src/main/lib/bar.ts).

Expected behavior

The report only contains all the files that match the passed sourceFilter pattern. In this case, all the files matching src/main/**/*.ts

[cenfun]

I can't pass the npm run build:main, could you please help to check it:

> build:main
> node build.mjs src/main/index.ts dist --treeshake

\c8-with-bundle\node_modules\typescript\lib\typescript.js:113963
    Debug.assert(contains(commandLine.fileNames, inputFileName), `Expected fileName to be present in command line`);
          ^

Error: Debug Failure. False expression: Expected fileName to be present in command line
    at Object.getOutputFileNames (\c8-with-bundle\node_modules\typescript\lib\typescript.js:113963:11)

[ericmorand]

Sorry, you need to build the test suite with npm run build:test. I assmue my main build script is flawed but we dont need it here. :)

[cenfun]

same errors

> build:test
> node build.mjs src/test/index.ts dist --source-map

\c8-with-bundle\node_modules\typescript\lib\typescript.js:113963
    Debug.assert(contains(commandLine.fileNames, inputFileName), `Expected fileName to be present in command line`);
          ^

Error: Debug Failure. False expression: Expected fileName to be present in command line

Im using Node.js v20, Windows 10

[ericmorand]

Very interesting. Windows 10 being the difference between our setups, I assume my build tool doesn't support it properly. Let me fix this.

[cenfun]

So back to your issue. I think your sourceFilter pattern should be **/src/**/*.ts, it used the tool minimatch see https://github.com/isaacs/minimatch

[ericmorand]

It doesn't remove node_modules from the coverage but adds src/test/index.ts, which seems to demonstrate that the pattern is honored.

By the way, I pushed a new version of the project that comes with the dist folder already generated. You can now execute npm run mcr, without having to build the test suite before.

[cenfun]

Thanks, I got it. The entry file is dist/index.mjs not src/**, so we need both entryFilter and sourceFilter
and It's recommended to use a config file mcr.config.js

// mcr.config.js
module.exports = {
    // logging: "debug",
    name: 'My Coverage Report',
    reports: [
        'v8',
        'console-details'
    ],
    outputDir: './coverage-reports',
    entryFilter: {
        "**/node_modules/**": false,
        "**/dist/**": true
    },
    sourceFilter: {
        "**/src/test/**": false,
        "**/src/**": true
    }
};

Then run npx mcr npm t again. Let me know if it works.

[MCR] My Coverage Report
┌──────────────┬─────────┬────────────┬──────────┬───────────┬─────────┬─────────────────┐
│ Name         │   Bytes │ Statements │ Branches │ Functions │   Lines │ Uncovered Lines │
├──────────────┼─────────┼────────────┼──────────┼───────────┼─────────┼─────────────────┤
│ src/main/lib │         │            │          │           │         │                 │
│ └ foo.ts     │ 74.38 % │    80.00 % │          │   50.00 % │ 57.14 % │ 5-7             │
├──────────────┼─────────┼────────────┼──────────┼───────────┼─────────┼─────────────────┤
│ Summary      │ 74.38 % │    80.00 % │          │   50.00 % │ 57.14 % │                 │
└──────────────┴─────────┴────────────┴──────────┴───────────┴─────────┴─────────────────┘

[cenfun]

BTW, the --sourceFilter dose not support multiple patterns for now, and I will fix it in next version:

--entryFilter "{'**/node_modules/**': false, '**/src/*.js': true}"

[ericmorand]

I'm not sure I understand the point of having an entry filter, but I probably misunderstand the meaning of the source filter. My asumption was that it filtered the files that are considered as "sources" - that is, the files that we want to get the coverage. With this asumption, there is no need for an entry filter because none of the files that are hit during the execution of the command should be emitted in the report, except if they have been added explicitely to the source filter.

Typically, in my case, using sourceFilter "src/main/**/*.ts", I would only get those files in the report, regardless of the files that were hit when executing command. Would I want node_modules files to be included in the report, I would just have to add them to the sourceFilter pattern.

Also, another question: how do I include every files matched by sourceFilter into the report - typically here src/main/index.ts.

[cenfun]

1, Due to the characteristics of v8 coverage data, we need to use two filters entryFilter and sourceFilter see here for more infomation
2, src/main/index.ts should be a untest file, which is not included in the bundle and coverage data, we can try all option see here

module.exports = {
     logging: "debug",
    name: 'My Coverage Report',
    reports: [
        'v8',
        'console-details'
    ],
    outputDir: './coverage-reports',
    entryFilter: {
        "**/node_modules/**": false,
        "**/src/test/**": false,
        "**/dist/**": true,
        "**/src/**": true
    },
    sourceFilter: {
        "**/src/test/**": false,
        "**/src/**": true
    },
    all: {
        dir: ['./src']
    }
};

and report

[MCR] My Coverage Report
┌─────────────────┬─────────┬────────────┬──────────┬───────────┬─────────┬─────────────────┐
│ Name            │   Bytes │ Statements │ Branches │ Functions │   Lines │ Uncovered Lines │
├─────────────────┼─────────┼────────────┼──────────┼───────────┼─────────┼─────────────────┤
│ src/main        │         │            │          │           │         │                 │
│ ├ index.ts      │  0.00 % │     0.00 % │          │           │  0.00 % │ 1-2             │
│ ├ tsconfig.json │  0.00 % │            │          │           │  0.00 % │ 1-3             │
│ └ lib           │         │            │          │           │         │                 │
│   ├ bar.ts      │  0.00 % │     0.00 % │          │    0.00 % │  0.00 % │ 1-3             │
│   ├ empty.ts    │         │            │          │           │         │                 │
│   └ foo.ts      │ 74.38 % │    80.00 % │          │   50.00 % │ 57.14 % │ 5-7             │
├─────────────────┼─────────┼────────────┼──────────┼───────────┼─────────┼─────────────────┤
│ Summary         │ 21.95 % │    23.53 % │          │   20.00 % │ 18.18 % │                 │
└─────────────────┴─────────┴────────────┴──────────┴───────────┴─────────┴─────────────────┘

[ericmorand]

Oh, I didn't see the all option in the help of the CLI, I assumed it didn't exist. It works when using a configuration file. Thanks.

[cenfun]

Yes, the CLI arguments only can pass the string and number, so some of the options not supported by CLI, please use config file for CLI.

[ericmorand]

About source and entry filter, I have the feeling that the second one is not needed and that every file (present in the V8 report or found in the source map) can be filtered against only the source filter - which is ultimately the declaration of the files that we want to cover.

I'll try locally to remove the option, and will create a PR only for discussion and proof of concept, if you agree.

[cenfun]

No, I don't think so.
The entryFilter is used to filter all entry files from the V8 coverage data. like huge node_modules, otherwise, performance would suffer due to the parsing too many meaningless files.
The sourceFilter is only used to filter source files from the sourcemap of the entry file.
In other words, if an entry file does not have a sourcemap, then the sourceFilter will not be executed.

[cenfun]

I've thought about it, and perhaps it's feasible to merge the entryFilter and sourceFilter into one filter. I will continue to think about it.