Istanbul: Coverage ignore doesn't work for default in switch

akellbl4 commented 3 months ago

Describe the bug

If switch...case is used and default statement is not covered by tests and /* istanbul ignore next -- @preserve */ used for ignoring the branch, it still shows up as uncovered.

I tried different combinations with double slash comment and wrapping default into a block, which produced the same result.

Reproduction

https://stackblitz.com/edit/github-rzcfqv

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @vitest/coverage-istanbul: ^2.0.3 => 2.0.3 
    vitest: ^2.0.3 => 2.0.3

Used Package Manager

npm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

AriPerkkio commented 3 months ago

Looks like esbuild doesn't respect comments inside switch:

https://esbuild.egoist.dev/#W1siaW5kZXgudH...:

switch (option) {
  ...
  /* This is removed -- @preserve */
  /* istanbul ignore next -- @preserve */
  default: {
    /* This is preserved -- @preserve */
    /* istanbul ignore next -- @preserve */
    return 'Not a foo, not a bar';
   }
}

Becomes:

switch (option) {
  ...
  default: {
    /* This is preserved -- @preserve */
    /* istanbul ignore next -- @preserve */
    return "Not a foo, not a bar";
  }
}

I don't think there are any work-arounds for this unfortunately.

akellbl4 commented 3 months ago

@AriPerkkio thanks for looking into it! I'll file an issue to esbuild. Should this issue be closed or left for tracking purposes?

AriPerkkio commented 3 months ago

We can leave this as open as it's now marked as upstream issue. It's good for others who run into same issue. And maybe in future once Vite moves from esbuild to Rolldown this issue could get fixed.

hyrious commented 2 months ago

I haven't use vitest's coverage tool before so maybe I'm wrong. Does vitest support sourcemaps in coverage report? At least I know c8 supports it which is working perfectly on my tiny projects (like this one). In addition integrating esbuild in tests like this is quite simple. Here's the source code of my tiny ts loader.

Sourcemap support is obviously useful here because the report can find the original comments from the source instead of from the transpiled js.

AriPerkkio commented 2 months ago

@hyrious sure it does support. Without source maps code coverage in Vitest would be useless as most users are using code transforms.

@vitest/coverage-v8 works just fine with the reproduction as it does exclusion when comparing to the source code. Note that the issue title mentions Istanbul coverage.

Istanbul's instrumentation is done to the transpiled code, not the source codes.

https://github.com/vitest-dev/vitest/blob/073a50c9740f75ba25ec1fabba73b6dae73e66f4/packages/coverage-istanbul/src/provider.ts#L167-L171

vitest-dev / vitest