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

Including/excluding files from coverage based on source path #2637

Open
dgp1130 opened this issue Feb 8, 2024 · 2 comments
Open

Including/excluding files from coverage based on source path #2637

dgp1130 opened this issue Feb 8, 2024 · 2 comments

Comments

@dgp1130
Copy link
Contributor

dgp1130 commented Feb 8, 2024

Description

I'm working on integrating Web Test Runner with Angular CLI and noticed some limitations with respect to code coverage. We're trying to pre-build a project's tests and run then WTR on the output directory, however this means the project is bundled together. Consider the following source tree:

src/foo.ts
src/foo_test.ts
src/bar.ts
src/bar_test.ts

When this is bundled, src/**_test.ts would be treated as "entry points" since those contain the tests. Depending on the import structure of these files, the output might look like:

dist/foo_test.js
dist/bar_test.js
dist/chunk-abc123.js

foo.ts and bar.ts contents may be spread arbitrarily between all three files.

Today with WTR, coverage does have understanding of sourcemaps and it will properly display coverage for source files (which is awesome BTW). However determining which files to show/hide seems to be based on the JavaScript files, not their TypeScript sources. This leads to three problems:

Missing source files

First, some files are missing. In my example use case, I have a src/foo.ts file which is missing altogether!

Screenshot from 2024-02-07 17-05-42

This happens because foo.ts is only used by foo_test.ts. So when the files are bundled, foo.ts is completely inlined in foo_test.ts. That shouldn't be a problem, but WTR automatically hides test files from code coverage, presumably to reduce noise in the output.

By contrast, bar.ts happens to be used in both foo_test.ts and bar_test.ts, so it gets bundled into the chunk-abc123.js file which is processed for code coverage.

I would expect explicitly setting coverageConfig: { include: ['dist/**/*.js'] } to override the test file filter, but that doesn't seem to work and has no effect. Even if it did, it would be useful to exclude the actual test source files (foo_test.ts and bar_test.ts) while including content which happened to get bundled into the test output files (foo_test.js and bar_test.js).

node_modules

Second, node_modules is not excluded correctly. One of my tests has a dependency on lit-html, but this dependency gets bundled into the output. Therefore there is no dist/node_modules/... directory, and this seems to circumvent the automatic exclusion of node_modules/.

Explicitly setting coverageConfig: { exclude: ['**/node_modules/**'] } has no effect either.

Screenshot from 2024-02-07 17-15-33

Excluding files without sourcemaps

Third, files without sourcemaps are included in the coverage output. In this example I see a dist/tests/chunk-*.js file.

Screenshot from 2024-02-07 17-17-32

This file just includes a __export helper function generated by ESBuild. It has no source in my application code and should be hidden from coverage output entirely.

In general, I think it's reasonable for WTR to include files without sourcemaps like this (might be nice to specify a base to translate dist/test/ -> src/, but that's not that important). However, I can't easily exclude this file. I want to do:

coverageConfig: {
  exclude: ["dist/chunk-*.js"],
}

But this will drop all chunks from coverage. Ironically, since WTR automatically excludes test files, the only thing it will display is code from shared chunks. So if I exclude chunks, I'm dropping all my covered files and get no coverage information whatsoever. Since the file hash is not deterministic, I can't easily exclude just this file. ESBuild helpers might also end up in multiple files, or none at all, so there's no easy workaround here.

Ultimately the root cause of all three problems is that coverage includes/excludes are based on the JavaScript files actually being tested, not the original sourcemaps we actually want coverage information for.

Reproduction

This repo has a trivial pre-bundle + coverage setup and demonstrates the above issues.

  1. src/foo.ts is only tested in src/foo_test.ts, so it is missing from coverage. Meanwhile src/bar.ts is used in src/foo_test.ts and src/bar_test.ts, so it gets bundled into a chunk-*.js file and included in coverage.
  2. node_modules/lit-html/... is included in the results despite being a third party dependency.
  3. A dist/test/chunk-*.js is included in coverage even though it only contains ESBuild helpers.

Proposed Solution

As much as I'd like to say that coverageConfig.exclude should refer to sourcemapped file paths, I expect that's probably too extreme a fix. I think we'd likely need a new coverageConfig.sourcemapExclude option which is filtered based on source locations rather than on disk location of the tested file. This should default to **/node_modules/** and would allow devs to configure it to src/**_test.ts without including a src/foo.ts file which happened to get inlined into a test file. This would immediately fix the first and second problems.

A coverageConfig.sourcemapInclude option might make sense for symmetry as well.

The third one is a little more complicated, since I'd need to exclude unsourcemapped content in dist/chunk-*.js but include any source files which happened to be bundled into them. The obvious solution is to make coverageConfig.sourcemapInclude override coverageConfig.exclude, but I'm not sure if that makes a whole lot of sense when multiple source files and generated code are bundled into the same file, or if other use cases might want different behavior. sourcemapInclude: ['.'] might be reasonable as default behavior here, though that might necessitate sourcemapExclude: ['**/*_test.ts'] which would be hard to infer from exclude, since WTR doesn't have knowledge of what the tests were built from.

With these utilities I think the proper configuration which would solve this problem would be:

export default {
  // Does not hide source files bundled into test files (implicitly fixes problem 1).
  coverageConfig: {
    // Hide any unsourcemapped content in chunk files (fix problem 3).
    exclude: ['dist/tests/chunk-*.js'],

    sourcemapInclude: [
      // Include all source files, even if they are excluded above
      // (avoid unintentional side-effect of fixing problem 3).
      // Arguably `sourcemapInclude: ['.']` should be the default behavior?
      "src/**.ts",
    ],
    sourcemapExclude: [
      // Hide dependencies (fix problem 2).
      // Should probably be the default behavior.
      '**/node_modules/**,
    ],
  },
};

I'm happy to help contribute a fix if that would be helpful. Though I want to at least get alignment on exactly what the options should be and how they should work. Also I'd likely need some pointers in the source code to understand where I should be looking to add this behavior.

@eight04
Copy link

eight04 commented Feb 15, 2024

I'm also facing the node_modules issue.

Fortunately, we host the coverage on codecov which allows us to exclude node_modules via codecov config:
https://docs.codecov.com/docs/ignoring-paths

@RobbyRabbitman
Copy link

I had the same issue with excluding the node_modules. In my case setting the include property worked. Maybe that solves part of your problem. Here is my config:

import { vitePlugin } from '@remcovaes/web-test-runner-vite-plugin';
import { defaultReporter, summaryReporter } from '@web/test-runner';
import { playwrightLauncher } from '@web/test-runner-playwright';

/** @type {import("@web/test-runner").TestRunnerConfig} */
export default {
  files: ['src/**/*.spec.ts'],
  plugins: [vitePlugin()],
  nodeResolve: {
    exportConditions: ['browser', 'development'],
  },
  coverage: true,
  coverageConfig: {
    include: ['src/**/*.ts'],
  },
  reporters: [
    summaryReporter(),
    defaultReporter(),
  ],
  browsers: [playwrightLauncher({ product: 'chromium' })],
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants