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 Report Path in Browser Mode Does Not Consider "reportsDirectory" #5013

Closed
6 tasks done
oscarmarina opened this issue Jan 19, 2024 · 10 comments · Fixed by #5056
Closed
6 tasks done

Coverage Report Path in Browser Mode Does Not Consider "reportsDirectory" #5013

oscarmarina opened this issue Jan 19, 2024 · 10 comments · Fixed by #5056
Labels
feat: browser Issues and PRs related to the browser runner

Comments

@oscarmarina
Copy link

oscarmarina commented Jan 19, 2024

Describe the bug

Hello,

I am experiencing an issue with the coverage report in Vitest when using Browser Mode with a web component built with Lit. When I run the tests and try to view the coverage, the page is blank. It seems that the coverage report is looking for the folder at the root of the project, not taking into account the reportsDirectory setting in my vite.config.js file.

reportsDirectory:./test/coverage/,

reportsDirectory

==

I would like to suggest an improvement for the LCOV reporter. Currently, the LCOV reporter generates the same files as the HTML reporter. However, I believe the LCOV reporter could be improved by creating a separate folder for its output. This would keep everything more organized, instead of leaving all the files in the root of the /coverage folder. Furthermore, when the Vitest UI is activated, the LCOV reporter is "equated" to the HTML reporter for generating the report view.

Reproduction

You can reproduce this issue by downloading the repository from my GitHub or viewing it on Stackblitz.

https://github.com/oscarmarina/lit-vitest-testing-comparison

https://stackblitz.com/github/oscarmarina/lit-vitest-testing-comparison

npm i && npm run test

System Info

System:
    OS: macOS 12.7.2
    CPU: (4) x64 Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz
    Memory: 17.82 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v20.10.0/bin/yarn
    npm: 10.3.0 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: 8.6.5 - ~/.nvm/versions/node/v20.10.0/bin/pnpm
  Browsers:
    Chrome Canary: 122.0.6257.0
    Safari: 17.2.1

Used Package Manager

npm

Validations

@oscarmarina oscarmarina changed the title Coverage Report Path in Browser Mode" Does Not Consider "reportsDirectory" Coverage Report Path in Browser Mode Does Not Consider "reportsDirectory" Jan 19, 2024
@AriPerkkio
Copy link
Member

Has this been working with earlier versions of Vitest?

I think this part of the UI is the root cause. For some reason the reportsDirectory is sliced from the last /, so your ./test/coverage becomes just /coverage.

// TODO
// For html report preview, "coverage.reportsDirectory" must be explicitly set as a subdirectory of html report.
// Handling other cases seems difficult, so this limitation is mentioned in the documentation for now.
export const coverageUrl = computed(() => {
if (coverageEnabled.value) {
const idx = coverage.value!.reportsDirectory.lastIndexOf('/')
const htmlReporter = coverage.value!.reporter.find((reporter) => {
if (reporter[0] !== 'html')
return undefined
return reporter
})
return htmlReporter && 'subdir' in htmlReporter[1]
? `/${coverage.value!.reportsDirectory.slice(idx + 1)}/${htmlReporter[1].subdir}/index.html`
: `/${coverage.value!.reportsDirectory.slice(idx + 1)}/index.html`
}

I would like to suggest an improvement for the LCOV reporter. Currently, the LCOV reporter generates the same files as the HTML reporter.

These built-in reporters are provided by Istanbul. Changes for them should be requested at https://github.com/istanbuljs/istanbuljs.

But it sounds like you should rather use lcovonly reporter if you have html reporter enabled:

lcov

Same as lcovonly, but also generates a HTML report you can view in your browser

https://istanbul.js.org/docs/advanced/alternative-reporters/#lcov

@oscarmarina
Copy link
Author

Hi @AriPerkkio, this is the first time I use vitest, I don't know if it worked before, sorry.

I explained poorly about lcov, what I was suggesting is to be able to use lcov to activate the coverage tab

lcov-active

@hi-ogawa
Copy link
Contributor

I think currently there's a difference between "vitest:browser" middleware plugin and "vitest:ui" plugin. "vitest:browser" only serves its own html/js assets but "vitest:ui" also serves generated coverage html:

async configureServer(server) {
server.middlewares.use(
base,
sirv(resolve(distRoot, 'client'), {

coverageFolder && server.middlewares.use(coveragePath!, sirv(coverageFolder[0], {

This "server side" code might also need to be adjusted for coverage preview on (headed) browser mode.

@AriPerkkio AriPerkkio added feat: browser Issues and PRs related to the browser runner and removed feat: ui Vitest UI labels Jan 20, 2024
@oscarmarina
Copy link
Author

Hi, I've set up a branch using provider: 'playwright', just as a helpful reference.

@userquin
Copy link
Member

userquin commented Feb 2, 2024

On my Windows laptop I can see this weird error when running the reproduction:

imagen

@userquin
Copy link
Member

userquin commented Feb 2, 2024

The problem seems to be here: https://github.com/oscarmarina/lit-vitest-testing-comparison/blob/main/index.html#L8

If I change previous line using absolute path /demo/index.html, the coverage folder showing the example:

imagen

@userquin
Copy link
Member

userquin commented Feb 2, 2024

Vitest getting this error:
imagen

@userquin
Copy link
Member

userquin commented Feb 2, 2024

@vitest/ui plugin not being included since ui is false and browser package not including coverage logic, so #5056 seems to be fine.

@oscarmarina
Copy link
Author

hi @userquin
the istanbul HTML report looks like this:

image

====
When you remove this line the "coverage" folder is created in the root of the project that's when it works fine

image

@userquin
Copy link
Member

userquin commented Feb 2, 2024

It works (when removing the directory option) because it is on the root, once you include the report directory the client will request /coverage/index.html and since there is no middleware to serve /coverage/ requests from the test/coverage directory, it fails, there is no coverage folder in root.

IIRC I did the original PR to include coverage in the ui 😅

@github-actions github-actions bot locked and limited conversation to collaborators Feb 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: browser Issues and PRs related to the browser runner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants