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

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

Open
6 tasks done
smtchahal opened this issue Sep 10, 2022 · 6 comments
Open
6 tasks done
Labels

Comments

@smtchahal
Copy link

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

@AriPerkkio
Copy link
Member

AriPerkkio commented Sep 11, 2022

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:

transform(srcCode, id) {
return ctx.coverageProvider?.onFileTransform?.(srcCode, id, this)
},

@smtchahal
Copy link
Author

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

@AriPerkkio
Copy link
Member

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' } },
},

@sheremet-va
Copy link
Member

sheremet-va commented Sep 12, 2022

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.

@AriPerkkio
Copy link
Member

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. evanw/esbuild#516 (comment)

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

@AriPerkkio
Copy link
Member

Related PR from istanbul-lib-instrument, adds support for /*! istanbul-ignore */ comments. istanbuljs/istanbuljs#693

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

PeachScript added a commit to umijs/father that referenced this issue Dec 20, 2022
* refactor: react jsx runtime respect tsconfig.json

* chore: update pnpm-lock.yaml

* style: update istanbul comment

* test: workaround use istanbul ignore with esbuild

ref: vitest-dev/vitest#2021 (comment)

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

No branches or pull requests

3 participants