Skip to content

refactor: remove duplicate values from coverage.exclude#9348

Open
btea wants to merge 4 commits intovitest-dev:mainfrom
btea:refactor/coverage-exclude-remove-duplicate-value
Open

refactor: remove duplicate values from coverage.exclude#9348
btea wants to merge 4 commits intovitest-dev:mainfrom
btea:refactor/coverage-exclude-remove-duplicate-value

Conversation

@btea
Copy link
Contributor

@btea btea commented Dec 27, 2025

Description

When vitest --coverage is executed, resolveConfig will be executed twice.

Resolves #issue-number

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.
  • Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

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 Dec 27, 2025

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f5a1dab
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/69b764032dcef4000863e571
😎 Deploy Preview https://deploy-preview-9348--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.

@sheremet-va
Copy link
Member

When vitest --coverage is executed, resolveConfig will be executed twice.

What do you mean? It is called once.

@btea
Copy link
Contributor Author

btea commented Dec 27, 2025

As shown in the image below, I set a breakpoint when setting the value of coverage.exclude. The first time, resolved.coverage.exclude was an empty array, and then the default value was set. The second time, since the value had already been set, the same value was set again.

screenshot image image

@sheremet-va
Copy link
Member

As shown in the image below, I set a breakpoint when setting the value of coverage.exclude. The first time, resolved.coverage.exclude was an empty array, and then the default value was set. The second time, since the value had already been set, the same value was set again.

screenshot image image

How does your config look like?

@btea
Copy link
Contributor Author

btea commented Dec 27, 2025

@sheremet-va
Copy link
Member

sheremet-va commented Dec 27, 2025

This seems more like a bug with projects resolution, the coverage.exclude is not the only affected array. Do you really need a projects field? Can't you just put a unit in the root's test.name?

@btea
Copy link
Contributor Author

btea commented Dec 27, 2025

yeah, you are right, it appears that after removing the projects configuration, resolveConfig will only execute once.

By the way, I saw in the documentation that coverage.exclude defaults to an empty array, but the actual code has a default value configured. Perhaps the documentation should be updated accordingly.

@sheremet-va
Copy link
Member

sheremet-va commented Dec 27, 2025

By the way, I saw in the documentation that coverage.exclude defaults to an empty array, but the actual code has a default value configured. Perhaps the documentation should be updated accordingly.

Technically speaking, the default is empty. Custom excludes don't override "the default". This is just an internal implementation detail of how the coverage providers treat certain files (like unreadable virtual files), it could've been hardcoded inside the provider, for example, but it should probably be mentioned in the docs in some way.

@btea
Copy link
Contributor Author

btea commented Dec 28, 2025

but it should probably be mentioned in the docs in some way.

I have updated the documentation.

By default, resolved.config is usually included in configFiles, but its result is an absolute path. Should this be treated as an identical value and removed?

resolved.config && slash(resolved.config),
...configFiles,


// Configs
resolved.config && slash(resolved.config),
resolved.config && basename(slash(resolved.config)),
Copy link
Member

Choose a reason for hiding this comment

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

Can you show example what cases this fixes? Does the configuration file show up in coverage in some cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vitest

As shown in the image above, resolved.config contains the full path, but it should be the same file as the second value in the configFiles array.

Copy link
Member

Choose a reason for hiding this comment

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

Does it cause issues with full path? For example, if you use vitest --config some-custom-config.ts, does that show up in coverage report?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't seem to cause any problems at the moment. Actually, this PR wasn't initially intended to address this issue.

Initially, it was here: #9348 (comment). My configuration caused the configuration content to be loaded twice, resulting in duplicate content in the second load. So, I wanted to remove duplicates.

Later, with @sheremet-va's guidance #9348 (comment), I adjusted the configuration so that Vitest's configuration loading logic worked correctly, loading only once. Then I noticed that this configuration file might also be duplicated, so I tried removing duplicates there as well. Of course, I think it's probably fine not to modify this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants