Skip to content

Conversation

@AriPerkkio
Copy link
Member

Description

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

@netlify
Copy link

netlify bot commented May 8, 2025

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit c5e4c24
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/682de7fed4d24500082da182
😎 Deploy Preview https://deploy-preview-7946--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@AriPerkkio AriPerkkio force-pushed the fix/v8-browser-in-source-test branch from 528b7af to 21a2dad Compare May 15, 2025 19:11
Comment on lines -74 to -77
if (coverage.url.includes('?browserv=') || coverage.url.includes('&browserv=')) {
return false
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check with Storybook folks to see that this doesn't break coverage reports there. This was used to exclude *.stories.* from appearing in coverage report.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*.stories.* are now visible on coverage report 😢

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storybook can now exclude story files inside the configureVitest hook

Copy link
Member Author

@AriPerkkio AriPerkkio May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that works! Using this in fresh Storybook project fixes the issue:

      {
        name: "exclude-stories",
        configureVitest(context) {
          if (context.vitest.config.coverage.enabled) {
            context.vitest.config.coverage.exclude.push("**/*.stories.**");
          }
        },

Related tests in #8009

@pkg-pr-new
Copy link

pkg-pr-new bot commented May 21, 2025

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@7946

@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@7946

@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@7946

@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@7946

@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@7946

@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@7946

@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@7946

@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@7946

@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@7946

@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@7946

@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@7946

vite-node

npm i https://pkg.pr.new/vite-node@7946

vitest

npm i https://pkg.pr.new/vitest@7946

@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@7946

@vitest/ws-client

npm i https://pkg.pr.new/@vitest/ws-client@7946

commit: c5e4c24

@AriPerkkio AriPerkkio force-pushed the fix/v8-browser-in-source-test branch from 21a2dad to c5e4c24 Compare May 21, 2025 14:49
const project = parentServer.vitest.getProjectByName(parentServer.config.name)
if (!project._isCachedTestFile(id) || !code.includes('import.meta.vitest')) {

if (!project._isCachedTestFile(filename) || !code.includes('import.meta.vitest')) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is needed so that /Users/x/vitest/test/coverage-test/fixtures/src/in-source.ts?import&browserv=1747838764408 is recognized as cached test file.

@AriPerkkio AriPerkkio marked this pull request as ready for review May 21, 2025 14:51
@AriPerkkio AriPerkkio requested a review from sheremet-va May 21, 2025 14:51
@sheremet-va sheremet-va merged commit 51cd814 into vitest-dev:main May 23, 2025
13 of 14 checks passed
@AriPerkkio AriPerkkio deleted the fix/v8-browser-in-source-test branch May 24, 2025 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Coverage reports missing when using @vitest/coverage-v8/browser with in-source testing

2 participants