Exclude test files from coverage report

[rozzilla]

What is the problem this feature will solve?

Hey folks! 👋🏼
I know that as by doc it's currently not possible to exclude specific files or directories from the coverage report.

The problem I face right now is that the coverage includes all of the test files.

For instance, if I have the following file:

// index.js
export function sum(val1, val2) {
  return val1 + val2;
}

and the test:

// index.test.js
import { strictEqual } from "node:assert";
import { it, describe } from "node:test";
import { sum } from "./index.js";

describe("test", () => {
  it("should pass", () => {
    strictEqual(sum(40, 2), 42);
  });
});

Once I run node --experimental-test-coverage --test I get:

> [email protected] no
> node --experimental-test-coverage --test

▶ test
  ✔ should pass (0.155709ms)
▶ test (1.117417ms)

ℹ tests 1
ℹ suites 1
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 47.674834
ℹ start of coverage report
ℹ --------------------------------------------------------------
ℹ file          | line % | branch % | funcs % | uncovered lines
ℹ --------------------------------------------------------------
ℹ index.js      | 100.00 |   100.00 |  100.00 | 
ℹ index.test.js | 100.00 |   100.00 |  100.00 | 
ℹ --------------------------------------------------------------
ℹ all files     | 100.00 |   100.00 |  100.00 |
ℹ --------------------------------------------------------------
ℹ end of coverage report

What is the feature you are proposing to solve the problem?

Ideally, .test.* should be excluded from the coverage report

What alternatives have you considered?

No response

[RedYetiDev]

Seems simple enough. The same logic for --test-skip-pattern can be used.

--coverage-exclude-glob ...
--coverage-include-glob ...

[RedYetiDev]

I'm happy to work on this :-).

[RedYetiDev]

Hey! Sorry for the delay, I've been out-of-town, but I'll begin work on this ASAP.

[JakobJingleheimer]

Thanks @RedYetiDev! Shouldn't the default behaviour exclude test files themselves from the coverage report? I can't think of a reason anyone would ever want them included.

[cjihrig]

Now that we have a way to configure the list of files, I agree that we should exclude the tests from the report. The issue in the past was that people could write tests in their source files.

[RedYetiDev]

Thanks @RedYetiDev! Shouldn't the default behaviour exclude test files themselves from the coverage report? I can't think of a reason anyone would ever want them included.

That shouldn't be too difficult. We can use the same list of file patterns to include with --test as ones to exclude for coverage

[RedYetiDev]

The only "snag" is that glob patterns are used to find test files. Because that feature isn't stable, it'd cause an experimentalWarning ExperimentalWarning: glob is an experimental feature and might change at any time, which users may not want to see when running code coverage (which is also experimental). If we do not care about this, I can go-ahead and submit a PR, or I can find a way to not emit the error when triggered internally from code coverage?

[cjihrig]

We already use glob in the test runner. I think you need to avoid the public API to avoid the warning.

[RedYetiDev]

The matchesGlob API emits a warning, but I can work around it.

I'm just wondering if, speed wise, it's faster to get all the paths matching a glob, and check the path against that array, or check each glob using the matchesGlob API, WDYT?

Oh wait! The test runner will already have done that magic for us, I can reuse data!

[silviuburceadev]

I'm keeping my lib files in src/main and my tests in src/test. I have my tests written in TS and run them with node --import tsx --test --experimental-test-coverage src/test/**.test.ts and I can see the src/test folder included in the coverage report. I've thought that maybe adding --experimental-strip-types might help the test runner/coverage reporter figure out that I'm on TS, but it isn't working.

[Llorx]

@RedYetiDev Just created a PR with the "default exclude" behaviour: #55633

[RedYetiDev]

@pmarchini IMHO this issue is completed, the PR itself should be the discussion for the other exclusion rules. If you disagree, re-open

[pmarchini]

@RedYetiDev, I reopened the issue since there's a related PR that will resolve this. Even though the PR is the right place for the implementation discussion, I think it might be better to keep this issue open until the PR lands.
I've seen many times in the past where PRs get stuck or are no longer under active development, and I found them through the issues.

What do you think?

[RedYetiDev]

SGTM, thanks for your insight :-)