vitest icon indicating copy to clipboard operation
vitest copied to clipboard

/* istanbul ignore if */ does not change coverage reporting for if condition

Open smtchahal opened this issue 3 years ago • 5 comments

Describe the bug

/* istanbul ignore if */ should ignore an if path from being considered for code coverage, but it doesn't with vitest. See the stackblitz for a reproducable example.

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-ogxiag?file=util.ts

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.1 LTS 22.04.1 LTS (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Core(TM) i5-9300H CPU @ 2.40GHz
    Memory: 11.06 GB / 15.47 GB
    Container: Yes
    Shell: 5.8.1 - /usr/bin/zsh
  Binaries:
    Node: 16.15.1 - ~/.nvm/versions/node/v16.15.1/bin/node
    npm: 8.11.0 - ~/.nvm/versions/node/v16.15.1/bin/npm
  Browsers:
    Chrome: 105.0.5195.102
    Firefox: 104.0.2
  npmPackages:
    @vitejs/plugin-react: ^2.1.0 => 2.1.0 
    vite: ^3.1.0 => 3.1.0 
    vitest: ^0.23.2 => 0.23.2

Used Package Manager

npm

Validations

smtchahal avatar Sep 10 '22 16:09 smtchahal

As a work-around for now you can use @preserve in the comment, e.g.

-/* istanbul ignore if */
+/* istanbul ignore if -- @preserve */
if (condition) {

The root cause seems to be terser or esbuild removing the comments. This @preserve is related to their options: https://terser.org/docs/api-reference#format-options, https://esbuild.github.io/api/#legal-comments. At this point we don't have comments available in srcCode:

https://github.com/vitest-dev/vitest/blob/f1cdfb6960a1eb39345f973529bc1e72ab4090b4/packages/vitest/src/node/plugins/coverageTransform.ts#L8-L10

AriPerkkio avatar Sep 11 '22 07:09 AriPerkkio

@AriPerkkio That works for now, thanks! Will this need to be fixed in esbuild/terser?

smtchahal avatar Sep 11 '22 16:09 smtchahal

If this is indeed coming from esbuild there is no way to fix this as they have stated not to support this. I think this is coming from esbuild since /*! comments are preserved in the transpiled code - this does not seem to be supported by terser.

But I'm still not sure, as I thought Vite was supposed to use esbuild only for dependencies, not the sources. Maybe vitest team has better insight here?

I've tried to change the Vite's minifier but none of these have any effect:

build: {
  minify: 'terser',
  terserOptions: { format: { comments: 'all' } },
},

AriPerkkio avatar Sep 11 '22 17:09 AriPerkkio

But I'm still not sure, as I thought Vite was supposed to use esbuild only for dependencies, not the sources. Maybe vitest team has better insight here?

TypeScript is processed by esbuild. JavaScript is left untouched.

I've tried to change the Vite's minifier but none of these have any effect:

These are placed under build option, as you can see. Vitest doesn't "build" code, this options are used only for vite build. There is top level esbuild option that is used by Vitest, when transpiling TypeScript. Code is not minified, it's only processed.

sheremet-va avatar Sep 12 '22 05:09 sheremet-va

Thanks for the clarification @sheremet-va, that makes sense.

TypeScript is processed by esbuild. JavaScript is left untouched.

Yes, it seems that in Javascript sources the comments are preserved. For Typescript I don't think there is anything we can do here on vitest's side. https://github.com/evanw/esbuild/issues/516#issuecomment-725093126

The only work-around is to use the @preserved in the comment.

AriPerkkio avatar Sep 12 '22 05:09 AriPerkkio

Related PR from istanbul-lib-instrument, adds support for /*! istanbul-ignore */ comments. https://github.com/istanbuljs/istanbuljs/pull/693

This won't solve everything but at least it extends the supported comment formats.

AriPerkkio avatar Sep 24 '22 14:09 AriPerkkio