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

coverage includes all lines (comments, import statements, etc..) #1715

Closed
sijakret opened this issue Oct 9, 2021 · 6 comments
Closed

coverage includes all lines (comments, import statements, etc..) #1715

sijakret opened this issue Oct 9, 2021 · 6 comments

Comments

@sijakret
Copy link

sijakret commented Oct 9, 2021

Hi there,

First of all: I love the modern web tooling it is so fast and straight forward!

When using the coverage reporting in web-test-runner the results I am getting seem to include lines without relevant statements (i.e. imports, comments, etc..).

Below are results from a typical karma stack for reference and the coverage results from web-test-runner are of comparatively low value here unfortunately. Do I have another problem here or is this behavior intended?

(please excuse the cropped views but i think it get's the point across.)

karma + babel + istanbul reporter
(ignores imports, comments, exported function definitions.. which is very common and desirable)
image
web test runner + chromium v8 + v8-to-istanbul
image

any tips on how to ignore the irrelevant lines ?

edit: v8 report does not seem to distinguish between statements and lines.
edit2: maybe this is just a reporter misconfiguration?
image

initially raised this here: istanbuljs/v8-to-istanbul#56 not sure where it really belongs

@sijakret sijakret changed the title coverage coverage includes all lines (comments, import statements, etc..) Oct 9, 2021
@web-padawan
Copy link
Contributor

Thanks for the issue. I think the reason for it is that v8-to-istanbul reports all the lines executed in the browser.

@sijakret
Copy link
Author

sijakret commented Oct 10, 2021

Ok, imho the istanbul-instrumented coverage is significantly more useful than the v8 coverage.

This is how I am doing it now using the standard js-space istanbul instrumentation:

// ..
import { fromRollup } from '@web/dev-server-rollup';
import rollupIstanbulPlugin from 'rollup-plugin-istanbul';

/**
 * unit test config
 */
export default {
  // ..
  nodeResolve: true,
  coverage: true,
  plugins: [
    fromRollup(rollupIstanbulPlugin)({
      // be sure to add your sources here!
      include: ['**/src/**']
    }),
    // ..
  ],
  coverageConfig: {
    report: true,
    reportDir: 'coverage-report',
    // nativeInstrumentation: false, // <- if uncomment this line, report is empty!
  }
}

Note: If I set nativeInstrumentation: false the generated report is empty (it is still generated though..)

Overall this setup works very well.
In my case, the instrumented code is not too heavy so I much prefer the more meaningful report here.

Since I assume most people are using this tooling in conjunction with es module stacks, shouldn't what I am doing using rollup-plugin-istanbul be the documented alternativen to the v8 coverage?
If you agree I can create a PR to update the corresponding section in the docs

@web-padawan
Copy link
Contributor

I'm not sure what is the performance impact of rollup-plugin-istanbul, does it slow down running tests? I'm going to try it in our repo with >5000 unit tests.

@sijakret
Copy link
Author

In my case I did not see a significant slowdown. But there is not much too much code that is being instrumented. it is likely not faster than the native(?) v8 coverage tracking.

My guess is the babel plugin from the current example in the Docs and the rollup plugin both use https://www.npmjs.com/package/istanbul-lib-instrument to instrument the code so likely there's not a big difference between them.

The other day I couldn't get the babel plugin to work though and overall the rollup plugin seems more focused and thus adequate for the task.

@sijakret
Copy link
Author

sijakret commented Nov 9, 2021

Happy with rollup-plugin-istanbul at this point. So from my perspective this can be closed.
It is not as fast as the v8 coverage but has much more meaningful results for us.

@sijakret sijakret closed this as completed Nov 9, 2021
@ChristianUlbrich
Copy link
Contributor

In our project the native V8 coverage instrumentations seems to struggle; whatever crazy combination of include/exclude patterns I try, our 800+ TS transpiled integration tests with sourcemaps will let the tests run 10x times longer. Hm, maybe using pnpm in a monorepo and transpile code before testing with tsc and sourcemaps is not sth. that is so common.

Anyhow, thanks to you @sijakret, the following works for us™:

// ...
coverage: true,
coverageConfig: {
  include: ['src/**/*.js'],
  exclude: ['src/**/*.scss', 'src/**/*.spec.js'],
  reportDir: './reports/coverage/integration',
  reporters: ['lcov', 'cobertura'],
},
plugins: [
   fromRollup(rollupIstanbulPlugin)({
      include: ['src/**/*.js'],
 })]

N.B. I have to set both include, otherwise I end up with stuff from the complete monorepo (node_modules, ...). And I can second your observation, setting nativeInstrumentation: false won't generate the html report. :(

Oddly enough, this is way faster than the native instrumentation. (50s vs. 700s+)

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