refactor(coverage): remove unnecessary `__vite_ssr_exports__` manipulation by hi-ogawa · Pull Request #7132 · vitest-dev/vitest

hi-ogawa · 2024-12-26T00:27:01Z

Description

Related Missing function coverage when transform prepends code right before the function #7130

Vite SSR doesn't create a mapping for Object.defineProperty(__vite_ssr_exports__, ...) (as they are simply injected by MagicString.appendLeft), so I'm not sure what this replacement on v8 coverage is for.

It looks like test/coverage-test is passing locally. Let me check others on CI.

Sadly, this doesn't seem to entirely help #7130. There's still some odd changes (namely export getter get appears in function coverage) in #7096 after applying the same patch.

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:.

…ation

netlify · 2024-12-26T00:29:47Z

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`cd186af`
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/676ca2d8bf75700008330cc9
😎 Deploy Preview	https://deploy-preview-7132--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

hi-ogawa · 2024-12-26T00:42:43Z

/ecosystem-ci run

vitest-ecosystem-ci · 2024-12-26T00:43:08Z

📝 Ran ecosystem CI: Open

suite	result
aria-live-capture	✅ success
nuxt	❌ failure
nuxt-test-utils	❌ failure
elk	✅ success
effect	❌ failure
lerna-lite	✅ success
zustand	✅ success
vue	✅ success
vite	✅ success
vitest-vscode	❌ failure
vitest-sonar-reporter	❌ failure
vitest-github-actions-reporter	❌ failure
vitest-browser-simple	✅ success
vitest-browser-examples	❌ failure
vitest-coverage-large	✅ success
vitest-reporters-large	✅ success
vitest-benchmark-large	✅ success

AriPerkkio

We should add test case that validates function counts. We used to have one but looks like it got removed during some test setup refactorings.

So maybe something like:

vitest/test/coverage-test/coverage-report-tests/generic.report.test.ts

Lines 132 to 141 in 7fd341c

    
           test('function count is correct', async () => { 
        
             const coverageJson = await readCoverageJson() 
        
             const coverageMap = libCoverage.createCoverageMap(coverageJson as any) 
        
             const fileCoverage = coverageMap.fileCoverageFor('<process-cwd>/src/function-count.ts') 
        
             const { functions } = fileCoverage.toSummary() 
        
             expect(functions.total).toBe(5) 
        
             expect(functions.covered).toBe(3) 
        
           })

vitest/test/coverage-test/src/function-count.ts

Lines 1 to 36 in 7fd341c

    
           /* 
        
            * This file should have: 
        
            * - 5 functions in total 
        
            * - 3 covered functions 
        
            * - 2 uncovered functions 
        
            */ 
        
           /* eslint-disable unused-imports/no-unused-vars */ 
        
           // This function is covered 
        
           function first() { 
        
             return 1 
        
           } 
        
           first() 
        
           // This function is covered 
        
           export function second() { 
        
             fifth() 
        
             return 2 
        
           } 
        
           // This function is NOT covered 
        
           export function third() { 
        
             throw new Error('Do not call this function') 
        
           } 
        
           // This function is NOT covered 
        
           function fourth() { 
        
             return 4 
        
           } 
        
           // This function is covered 
        
           function fifth() { 
        
             return 5 
        
           }

hi-ogawa · 2024-12-26T08:24:53Z

We should add test case that validates function counts.

It looks like there are several tests related to function count like this snapshot

vitest/test/coverage-test/test/__snapshots__/results-v8.snapshot.json

Lines 108 to 110 in f9a6284

    
           "fnMap": { 
        
             "0": { 
        
               "name": "isEven",

In #7096, I'm seeing get gets included in the snapshot, so there's something wrong, but in this PR, that's not happening.

hi-ogawa · 2025-01-13T10:06:38Z

It's probably easier to do it together with #7192, so moving the change to there.

refactor(coverage): remove unnecessary __vite_ssr_exports__ manipul…

cd186af

…ation

hi-ogawa marked this pull request as ready for review December 26, 2024 00:48

AriPerkkio reviewed Dec 26, 2024

View reviewed changes

chore: cleanup

f27ac1b

hi-ogawa added the p2-to-be-discussed Enhancement under consideration (priority) label Jan 6, 2025

hi-ogawa mentioned this pull request Jan 13, 2025

fix(coverage): exclude injected functions without mapping #7192

Closed

10 tasks

hi-ogawa marked this pull request as draft January 13, 2025 08:49

hi-ogawa closed this Jan 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

refactor(coverage): remove unnecessary `__vite_ssr_exports__` manipulation#7132

refactor(coverage): remove unnecessary `__vite_ssr_exports__` manipulation#7132
hi-ogawa wants to merge 2 commits intovitest-dev:mainfrom
hi-ogawa:fix-coverage-vite-exports-already-no-map

hi-ogawa commented Dec 26, 2024 •

edited

Loading

Uh oh!

netlify bot commented Dec 26, 2024

Uh oh!

hi-ogawa commented Dec 26, 2024

Uh oh!

vitest-ecosystem-ci bot commented Dec 26, 2024 •

edited

Loading

Uh oh!

AriPerkkio left a comment

Uh oh!

hi-ogawa commented Dec 26, 2024

Uh oh!

hi-ogawa commented Jan 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	test('function count is correct', async () => {
	const coverageJson = await readCoverageJson()
	const coverageMap = libCoverage.createCoverageMap(coverageJson as any)
	const fileCoverage = coverageMap.fileCoverageFor('<process-cwd>/src/function-count.ts')

	const { functions } = fileCoverage.toSummary()

	expect(functions.total).toBe(5)
	expect(functions.covered).toBe(3)
	})

	/*
	* This file should have:
	* - 5 functions in total
	* - 3 covered functions
	* - 2 uncovered functions
	*/

	/* eslint-disable unused-imports/no-unused-vars */

	// This function is covered
	function first() {
	return 1
	}

	first()

	// This function is covered
	export function second() {
	fifth()
	return 2
	}

	// This function is NOT covered
	export function third() {
	throw new Error('Do not call this function')
	}

	// This function is NOT covered
	function fourth() {
	return 4
	}

	// This function is covered
	function fifth() {
	return 5
	}

Uh oh!

Comments

Conversation

hi-ogawa commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

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

Tests

Documentation

Changesets

Uh oh!

netlify bot commented Dec 26, 2024

✅ Deploy Preview for vitest-dev ready!

Uh oh!

hi-ogawa commented Dec 26, 2024

Uh oh!

vitest-ecosystem-ci bot commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AriPerkkio left a comment

Choose a reason for hiding this comment

Uh oh!

hi-ogawa commented Dec 26, 2024

Uh oh!

hi-ogawa commented Jan 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hi-ogawa commented Dec 26, 2024 •

edited

Loading

vitest-ecosystem-ci bot commented Dec 26, 2024 •

edited

Loading