vitest-dev / vitest

Next generation testing framework powered by Vite.
https://vitest.dev
MIT License
13.05k stars 1.17k forks source link

v8 coverage provider report 100% statement coverage for uncovered v-if inside template #4993

Open index23 opened 9 months ago

index23 commented 9 months ago

Describe the bug

v8 coverage provider report 100% statement coverage for uncovered v-if inside template

HelloWorld.vue

<script setup lang="ts">
import { computed } from 'vue'

const props = defineProps<{
  msg: string
}>()

const hasMessage = computed(() => {
  return Boolean(props.msg)
})

</script>

<template>
  <div class="greetings">
    <template v-if="hasMessage">
      <h1 class="green">{{ msg }}</h1>
    </template>
    <h3>
      You’ve successfully created a project with
      <a href="https://vitejs.dev/" target="_blank" rel="noopener">Vite</a> +
      <a href="https://vuejs.org/" target="_blank" rel="noopener">Vue 3</a>.
    </h3>
  </div>
</template>

HelloWorld.spec.ts

import { describe, it, expect } from 'vitest'

import { mount } from '@vue/test-utils'
import HelloWorld from '../HelloWorld.vue'

describe('HelloWorld', () => {
  it('renders properly', () => {
    const wrapper = mount(HelloWorld)
    expect(wrapper.find('.greetings').exists()).toBe(true)
  })
})

In this example we test component without props so template with h1 tag will not render but provider report that coverage for that lines are 100% covered, as well as for entire file.

Screenshot 2024-01-17 at 16 32 11

Reproduction

Link to repo where you can reproduce issue, https://github.com/index23/vue-test-template-coverage

Steps to reproduce:

  1. Clone repo
  2. Run unit tests with npm run test:unit
  3. Coverage will generate in coverage directory
  4. Open coverage/lcov-report/index.html

Screenshot 2024-01-17 at 16 32 11

System Info

System:
    OS: macOS 14.2.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 41.92 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
    pnpm: 8.14.1 - ~/Library/pnpm/pnpm
  Browsers:
    Brave Browser: 119.1.60.125
    Chrome: 120.0.6099.216
    Edge: 120.0.2210.133
    Safari: 17.2.1
  npmPackages:
    @vitejs/plugin-vue: ^4.5.2 => 4.6.2 
    @vitest/coverage-v8: ^1.2.0 => 1.2.0 
    vite: ^5.0.10 => 5.0.11 
    vitest: ^1.0.4 => 1.2.0

Used Package Manager

npm

Validations

AriPerkkio commented 9 months ago

This usually happens when source maps are missing. v8-to-istanbul simply marks whole file as covered then. I haven't yet looked into the reproduction case, but that will likely reveal in issue with source maps.

Does this happen with latest version of @vitejs/plugin-vue?

index23 commented 9 months ago

@AriPerkkio Yes, it is same with latest version "@vitejs/plugin-vue": "^5.0.3". I not sure whether this happens, because I've tested some other cases where coverage for template works fine. For example, I have v-if and v-else in template and have test for v-if, v-else part of template is correct marked as uncovered.

<template v-if="hasMessage">
  <h1 class="green">{{ msg }}</h1>
</template>
<template v-else>
  <h1 class="yellow">Default message</h1>
</template>

and tests...

describe('HelloWorld', () => {
  it('renders properly', () => {
    const wrapper = mount(HelloWorld, {props:{msg:'Yes did it!'}})
    expect(wrapper.find('.greetings').exists()).toBe(true)
  })
})

coverage...

Screenshot 2024-01-18 at 10 04 26

sheremet-va commented 9 months ago

Vue parser was updated in the latest minor version - maybe it's a regression on Vue's part?

AriPerkkio commented 9 months ago

Source maps are present in this case. If you add some unreached code into <script> tags, it will be shown correctly.

The reason why line 17's <h1>...</h1> is shown covered, is because Vue compiler will generate the class="green" part on another line again on the compiled code - and it's included in source maps.

The line 24 of generated code is executed by Node as it's in top level scope. It's source maps points it to line 17 in sources -> it's covered. You can see the line mapped here by clicking line 24 on "Generated" side: https://ariperkkio.github.io/source-map-debugger/?s=N4Ig5gpg... image

In the V8 report this line is marked as covered as it's executed. If Vue excluded that line from source maps, or added /* v8 ignore next */ (and Istanbul equivalent one) on top of it, the uncovered line would show correctly. There's another part in the compiled code which is uncovered in V8 report that would make it show uncovered in final report:

image

This is similar issue as Svelte has: https://github.com/sveltejs/svelte/issues/7824. If Vue and Svelte compilers had something like Babel's auxiliaryCommentBefore we could use it like Jest does: https://github.com/jestjs/jest/blob/8c78a08c6e75c1a0c4447aa8a0c61ad9e395e16f/e2e/coverage-transform-instrumented/preprocessor.js#L25

So to summarize: Issue is in Vue compiler. It's generated code does not work with code coverage tools.

index23 commented 9 months ago

@AriPerkkio Thank you for explanation. Just summarize to be more clear to me. Coverage tool marks those lines as covered because it is hoisted in top level scope after compilation. Am I right?

And I have another question. Do you find any solution? I think that it is not minor issue. For example, process on my project is based on code coverage, among other tools.

AriPerkkio commented 9 months ago

Just summarize to be more clear to me. Coverage tool marks those lines as covered because it is hoisted in top level scope after compilation. Am I right?

Yep, but it looks like there is also another issue. Even if the hoisted variable was dropped out of the source maps, this line would still mark the v-if line as covered. Both sides of the $setup.hasMessage ternary are pointing to that line -> it's always marked as covered.

image

Link https://evanw.github.io/source-map-visualization/#MjQ5....

And I have another question. Do you find any solution?

There are no good work-arounds for this. Vue compiler needs to adjust their source maps.

index23 commented 9 months ago

It's clear to me. I am ready to go further. We have two issues here:

  1. Template is hoisted to top level scope. Which repo is responsible for this, vitejs/vite-plugin-vue?
  2. Both sides of ternary operator, including comment node, pointing to same line in source code, so in both cases, with or without prop, line will be marked as covered. Which repo is responsible for this, vitejs/vite?

I would start with resolving second issue...

@AriPerkkio Is it ok to link your noted links, if I open issue to another repo?

AriPerkkio commented 9 months ago

I think this is an issue of Vue compiler, not the vite-plugin-vue itself. While their source maps are correct right now, they are not compatible with code coverage tools. It's similar issue as Svelte has: https://github.com/sveltejs/svelte/issues/7824. Feel free to link this issue in other ones.

boboldehampsink commented 9 months ago

Downgrading from Vue 3.4 to 3.3 fixes these issues.

index23 commented 9 months ago

Downgrading from Vue 3.4 to 3.3 fixes these issues.

Thanks for answer, but unfortunately, doesn't work for me. @boboldehampsink Can you send me your lock file? Maybe it solves issue with correct version from another dependency.

boboldehampsink commented 9 months ago

I used

  "resolutions": {
    "vue": "~3.3.0"
  },

and ran yarn upgrade

index23 commented 9 months ago

Nope, same issue...

cenfun commented 9 months ago

The reasons could be: 1, The soucemap is inaccurate, especially for Vue which map JS code to HTML code. 2, The conversion of the soucemap is inaccurate. For example, when the target position is between two mappings, v8-to-istanbul is unable to find the precise position, see here

We found this issue previously but have been unable to solve it until we implemented our own converter. image

You could try it simply using vitest customProviderModule 1, npm install vitest-monocart-coverage 2, vitest.config.ts

import { fileURLToPath } from 'node:url'
import { mergeConfig, defineConfig, configDefaults } from 'vitest/config'
import viteConfig from './vite.config'
export default mergeConfig(
  viteConfig,
  // @ts-ignore
  defineConfig({
    test: {
      environment: 'jsdom',
      exclude: [...configDefaults.exclude, 'e2e/*'],
      root: fileURLToPath(new URL('./', import.meta.url)),
      coverage: {
        // @ts-ignore
        reporter: ['v8', 'lcov'],
        provider: 'custom',
        customProviderModule: 'vitest-monocart-coverage'
      }
    }
  })
)
AriPerkkio commented 9 months ago

1, The soucemap is inaccurate, especially for Vue which map JS code to HTML code.

There's link to the source maps above https://github.com/vitest-dev/vitest/issues/4993#issuecomment-1904344364. It is accurate.

2, The conversion of the soucemap is inaccurate.

What do you mean?

The root cause is that this line is actually covered. In the transpiled code it's in two positions: once on top level and once inside the conditional call. The top level code is always executed. I'm not sure what of rule could be used to exclude the top level code from coverage report.

cenfun commented 9 months ago

image

Please see my screenshot, there are two mappings: start mapping: gm1(generated mapping 1) -> om1(original mapping 1) end mapping: gm2(generated mapping 2) -> om2(original mapping 2)

the coverage start position is gp (generated position), and we need to map it to op (original position), So, where should the position of op be? We can only say that its position is between om1 and om2, because gp is between gm1 and gm2. for this case, it should be om2 or om2+1. For Vue, JS is unable to precisely match HTML.

Same problem for end position: image

AriPerkkio commented 9 months ago

Right, but what about code that's on the top level? That's always covering the line. I guess you would need AST analysis to exclude that part.

image

cenfun commented 9 months ago

Firstly, the case top level you indicated will be ignored, because it is not a block of coverage. It doesn't provide any coverage information for this key/value class: "green".

And, the logic of V8 coverage should be as follows:

So, when <h1> is uncovered, It's impossible for class="green" to be covered. What do you think?

AriPerkkio commented 9 months ago

Here's the V8 report and the contents that Vitest runs for this file:

coverage.json ```json [ { "functionName": "", "ranges": [{ "startOffset": 0, "endOffset": 5760, "count": 1 }], "isBlockCoverage": true }, { "functionName": "", "ranges": [{ "startOffset": 13, "endOffset": 5760, "count": 1 }], "isBlockCoverage": true }, { "functionName": "setup", "ranges": [{ "startOffset": 1280, "endOffset": 1750, "count": 1 }], "isBlockCoverage": true }, { "functionName": "", "ranges": [{ "startOffset": 1416, "endOffset": 1462, "count": 1 }], "isBlockCoverage": true }, { "functionName": "", "ranges": [{ "startOffset": 1518, "endOffset": 1564, "count": 0 }], "isBlockCoverage": false }, { "functionName": "_withScopeId", "ranges": [{ "startOffset": 1777, "endOffset": 1886, "count": 1 }], "isBlockCoverage": true }, { "functionName": "", "ranges": [{ "startOffset": 2030, "endOffset": 2713, "count": 1 }], "isBlockCoverage": true }, { "functionName": "_sfc_render", "ranges": [ { "startOffset": 2716, "endOffset": 3193, "count": 1 }, { "startOffset": 2914, "endOffset": 3112, "count": 0 } ], "isBlockCoverage": true } ] ```
transpiled.js ```js 'use strict';async (__vite_ssr_import__,__vite_ssr_dynamic_import__,__vite_ssr_exports__,__vite_ssr_exportAll__,__vite_ssr_import_meta__,require,exports,module,__filename,__dirname)=>{{const __vite_ssr_import_0__ = await __vite_ssr_import__("/node_modules/.pnpm/vue@3.4.15_typescript@5.3.3/node_modules/vue/index.mjs?v=65178666", {"importedNames":["defineComponent"]}); const __vite_ssr_import_1__ = await __vite_ssr_import__("/node_modules/.pnpm/vue@3.4.15_typescript@5.3.3/node_modules/vue/index.mjs?v=65178666", {"importedNames":["computed"]}); const __vite_ssr_import_2__ = await __vite_ssr_import__("/node_modules/.pnpm/vue@3.4.15_typescript@5.3.3/node_modules/vue/index.mjs?v=65178666", {"importedNames":["toDisplayString","openBlock","createElementBlock","createCommentVNode","createElementVNode","createTextVNode","pushScopeId","popScopeId"]}); const __vite_ssr_import_3__ = await __vite_ssr_import__("/src/components/HelloWorld.vue?vue&type=style&index=0&scoped=e17ea971&lang.css"); const __vite_ssr_import_4__ = await __vite_ssr_import__("/@id/__x00__plugin-vue:export-helper", {"importedNames":["default"]}); const _sfc_main = /* @__PURE__ */ __vite_ssr_import_0__.defineComponent({ __name: "HelloWorld", props: { msg: { type: String, required: true } }, setup(__props, { expose: __expose }) { __expose(); const props = __props; const hasMessage = __vite_ssr_import_1__.computed(() => { return Boolean(props.msg); }); const unreached = __vite_ssr_import_1__.computed(() => { return Boolean(props.msg); }); const __returned__ = { props, hasMessage, unreached }; Object.defineProperty(__returned__, "__isScriptSetup", { enumerable: false, value: true }); return __returned__; } }); const _withScopeId = (n) => (__vite_ssr_import_2__.pushScopeId("data-v-e17ea971"), n = n(), __vite_ssr_import_2__.popScopeId(), n); const _hoisted_1 = { class: "greetings" }; const _hoisted_2 = { key: 0, class: "green" }; const _hoisted_3 = /* @__PURE__ */ _withScopeId(() => /* @__PURE__ */ __vite_ssr_import_2__.createElementVNode( "h3", null, [ /* @__PURE__ */ __vite_ssr_import_2__.createTextVNode(" You\u2019ve successfully created a project with "), /* @__PURE__ */ __vite_ssr_import_2__.createElementVNode("a", { href: "https://vitejs.dev/", target: "_blank", rel: "noopener" }, "Vite"), /* @__PURE__ */ __vite_ssr_import_2__.createTextVNode(" + "), /* @__PURE__ */ __vite_ssr_import_2__.createElementVNode("a", { href: "https://vuejs.org/", target: "_blank", rel: "noopener" }, "Vue 3"), /* @__PURE__ */ __vite_ssr_import_2__.createTextVNode(". ") ], -1 /* HOISTED */ )); function _sfc_render(_ctx, _cache, $props, $setup, $data, $options) { return __vite_ssr_import_2__.openBlock(), __vite_ssr_import_2__.createElementBlock("div", _hoisted_1, [ $setup.hasMessage ? (__vite_ssr_import_2__.openBlock(), __vite_ssr_import_2__.createElementBlock( "h1", _hoisted_2, __vite_ssr_import_2__.toDisplayString($props.msg), 1 /* TEXT */ )) : __vite_ssr_import_2__.createCommentVNode("v-if", true), _hoisted_3 ]); } __vite_ssr_exports__.default = /* @__PURE__ */ __vite_ssr_import_4__.default(_sfc_main, [["render", _sfc_render], ["__scopeId", "data-v-e17ea971"], ["__file", "/Users/abc/efg/vue-test-template-coverage/src/components/HelloWorld.vue"]]); //# sourceMappingSource=vite-node //# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJtYXBwaW5ncyI6Ijs7Ozs7O0FBQ3lCOzs7Ozs7OztBQUV6QixVQUFNLFFBQVE7QUFJZCxVQUFNLGFBQWEsK0JBQVMsTUFBTTtBQUNoQyxhQUFPLFFBQVEsTUFBTSxHQUFHO0FBQUEsSUFDMUIsQ0FBQztBQUVELFVBQU0sWUFBWSwrQkFBUyxNQUFNO0FBQy9CLGFBQU8sUUFBUSxNQUFNLEdBQUc7QUFBQSxJQUMxQixDQUFDOzs7Ozs7OztxQkFJTSxPQUFNLFlBQVc7OztFQUVkLE9BQU07O3NEQUVaO0FBQUEsRUFJSztBQUFBO0FBQUE7QUFBQSwwREFKRCxtREFFRjtBQUFBLDZEQUFxRTtBQUFBLE1BQWxFLE1BQUs7QUFBQSxNQUFzQixRQUFPO0FBQUEsTUFBUyxLQUFJO0FBQUEsT0FBVyxNQUFJO0FBQUEsMERBQUksS0FDckU7QUFBQSw2REFBcUU7QUFBQSxNQUFsRSxNQUFLO0FBQUEsTUFBcUIsUUFBTztBQUFBLE1BQVMsS0FBSTtBQUFBLE9BQVcsT0FBSztBQUFBLDBEQUFJLElBQ3ZFO0FBQUE7Ozs7OzRDQVJGLHlDQVNNLE9BVE4sWUFTTTtBQUFBLElBUlksd0RBQ2Q7QUFBQSxNQUFnQztBQUFBLE1BQWhDO0FBQUEsTUFBZ0Msc0NBQVgsVUFBRztBQUFBO0FBQUE7QUFBQTtJQUUxQjtBQUFBIiwibmFtZXMiOltdLCJzb3VyY2VzIjpbIkhlbGxvV29ybGQudnVlIl0sInNvdXJjZXNDb250ZW50IjpbIjxzY3JpcHQgc2V0dXAgbGFuZz1cInRzXCI+XG5pbXBvcnQgeyBjb21wdXRlZCB9IGZyb20gXCJ2dWVcIjtcblxuY29uc3QgcHJvcHMgPSBkZWZpbmVQcm9wczx7XG4gIG1zZzogc3RyaW5nO1xufT4oKTtcblxuY29uc3QgaGFzTWVzc2FnZSA9IGNvbXB1dGVkKCgpID0+IHtcbiAgcmV0dXJuIEJvb2xlYW4ocHJvcHMubXNnKTtcbn0pO1xuXG5jb25zdCB1bnJlYWNoZWQgPSBjb21wdXRlZCgoKSA9PiB7XG4gIHJldHVybiBCb29sZWFuKHByb3BzLm1zZyk7XG59KTtcbjwvc2NyaXB0PlxuXG48dGVtcGxhdGU+XG4gIDxkaXYgY2xhc3M9XCJncmVldGluZ3NcIj5cbiAgICA8dGVtcGxhdGUgdi1pZj1cImhhc01lc3NhZ2VcIj5cbiAgICAgIDxoMSBjbGFzcz1cImdyZWVuXCI+e3sgbXNnIH19PC9oMT5cbiAgICA8L3RlbXBsYXRlPlxuICAgIDxoMz5cbiAgICAgIFlvdeKAmXZlIHN1Y2Nlc3NmdWxseSBjcmVhdGVkIGEgcHJvamVjdCB3aXRoXG4gICAgICA8YSBocmVmPVwiaHR0cHM6Ly92aXRlanMuZGV2L1wiIHRhcmdldD1cIl9ibGFua1wiIHJlbD1cIm5vb3BlbmVyXCI+Vml0ZTwvYT4gK1xuICAgICAgPGEgaHJlZj1cImh0dHBzOi8vdnVlanMub3JnL1wiIHRhcmdldD1cIl9ibGFua1wiIHJlbD1cIm5vb3BlbmVyXCI+VnVlIDM8L2E+LlxuICAgIDwvaDM+XG4gIDwvZGl2PlxuPC90ZW1wbGF0ZT5cblxuPHN0eWxlIHNjb3BlZD5cbmgxIHtcbiAgZm9udC13ZWlnaHQ6IDUwMDtcbiAgZm9udC1zaXplOiAyLjZyZW07XG4gIHBvc2l0aW9uOiByZWxhdGl2ZTtcbiAgdG9wOiAtMTBweDtcbn1cblxuaDMge1xuICBmb250LXNpemU6IDEuMnJlbTtcbn1cblxuLmdyZWV0aW5ncyBoMSxcbi5ncmVldGluZ3MgaDMge1xuICB0ZXh0LWFsaWduOiBjZW50ZXI7XG59XG5cbkBtZWRpYSAobWluLXdpZHRoOiAxMDI0cHgpIHtcbiAgLmdyZWV0aW5ncyBoMSxcbiAgLmdyZWV0aW5ncyBoMyB7XG4gICAgdGV4dC1hbGlnbjogbGVmdDtcbiAgfVxufVxuPC9zdHlsZT5cbiJdLCJmaWxlIjoiL3NyYy9jb21wb25lbnRzL0hlbGxvV29ybGQudnVlIn0= }} ```

Should v8-to-istanbul ignore the first two entries of coverage.json here? Currently anything inside that is marked as covered, including the class: "green" part. Isn't logic something like "Mark everything inside a covered block as covered. Mark uncovered blocks inside that block as uncovered"?

cenfun commented 9 months ago

I think it should be:

AriPerkkio commented 9 months ago

Yep, exactly. So the first two entries of the V8 report will mark the class: "green" part as covered.

cenfun commented 9 months ago

Yes, at the beginning, the first two entries indeed mark class: "green" as covered, but if later there is block like <h1> with a count of 0, it will re-mark class: "green" as uncovered. Uncovered has a higher priority than covered. see https://github.com/istanbuljs/v8-to-istanbul/blob/master/lib/v8-to-istanbul.js#L198-L205 default line count is 1: https://github.com/istanbuljs/v8-to-istanbul/blob/master/lib/line.js#L16

AriPerkkio commented 9 months ago

but if later there is block like <h1> with a count of 0

Sure, but there is no such block. That's why it's covered. The class: "green" is between offsets 1931 - 1981 in the transpiled code. From the coverage.json above you can see that there is no block that marks it uncovered.

cenfun commented 9 months ago

this one is <h1> and is uncovered in your coverage.json

{ "startOffset": 2914, "endOffset": 3112, "count": 0 }
AriPerkkio commented 9 months ago

@cenfun could you file a bug report to v8-to-istanbul about this? To me it sounds like you've found a more precise source map tracing than source-map or @jridgewell/trace-mapping.

cenfun commented 9 months ago

There is already a bug here. At that time, we were also using v8-to-istanbul as the converter, but found the above issue. we couldn't wait for it to be solved, so I had to solve it myself, which is to implement my own converter. Until today, I still find some similar issues releated to v8-to-istanbul, so just share some of my experiences with you. For example the comments issue: