Skip to content

test: collect coverage from untested files too#2239

Draft
AriPerkkio wants to merge 8 commits intonpmx-dev:mainfrom
AriPerkkio:test/coverage-includes
Draft

test: collect coverage from untested files too#2239
AriPerkkio wants to merge 8 commits intonpmx-dev:mainfrom
AriPerkkio:test/coverage-includes

Conversation

@AriPerkkio
Copy link
Copy Markdown

@AriPerkkio AriPerkkio commented Mar 23, 2026

🔗 Linked issue

(looks like everyone leaves this empty)

🧭 Context

Currently code coverage is not collected from untested files. You'll need to define coverage.include to instruct Vitest which files are considered as source files.

https://vitest.dev/guide/coverage.html#including-and-excluding-files-from-coverage-report

📚 Description

Defines source directories in coverage.includes with extensions of ts and vue.

This drops code coverage:

  --------------------------------------------|---------|----------|---------|---------|
  File                                        | % Stmts | % Branch | % Funcs | % Lines |
  --------------------------------------------|---------|----------|---------|---------|
- All files                                   |    59.1 |    45.68 |   54.58 |   60.41 |
+ All files                                   |   44.05 |    37.21 |   41.92 |   44.26 |

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs.npmx.dev Ready Ready Preview, Comment Mar 28, 2026 1:06am
npmx.dev Ready Ready Preview, Comment Mar 28, 2026 1:06am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
npmx-lunaria Ignored Ignored Mar 28, 2026 1:06am

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07335604-7f4b-4e48-a8d8-5d09cdeea004

📥 Commits

Reviewing files that changed from the base of the PR and between 48718a3 and c984b60.

📒 Files selected for processing (1)
  • vite.config.ts

📝 Walkthrough

Walkthrough

Updated vite.config.ts to adjust Vitest v8 coverage configuration: added an include filter {app,cli,server,shared}/**/*.{ts,vue} and retained the existing exclude filters **/node_modules/** and **/*.json. Coverage collection is now explicitly targeted to .ts and .vue files under the app, cli, server and shared directories.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly explains the context, rationale, and impact of adding coverage.include configuration to Vitest, with specific file changes and metrics provided.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@ghostdevv ghostdevv left a comment

Choose a reason for hiding this comment

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

I re-added the exclude as I think it should still be there? WDYT?

Also the CI has been failing, any ideas? If not I'll try and take a look soon 🙏 I'll mark as draft while we work on that, and then we can make it ready again when it's working!

@ghostdevv ghostdevv marked this pull request as draft March 23, 2026 17:59
auto-merge was automatically disabled March 23, 2026 17:59

Pull request was converted to draft

@AriPerkkio
Copy link
Copy Markdown
Author

I re-added the exclude as I think it should still be there? WDYT?

*.json does not match the include glob's extensions (.ts, .vue) so it's not needed at all. Does app/, cli/, server/ or shared/ directories contain node_modules? 🤔

Also node_modules is always excluded so it doesn't need to be added. https://github.com/vitest-dev/vitest/blob/1f2d318493363855b66a22caaf7c1c10579029d5/packages/vitest/src/node/config/resolveConfig.ts#L497-L523

Also the CI has been failing, any ideas? If not I'll try and take a look soon 🙏 I'll mark as draft while we work on that, and then we can make it ready again when it's working!

No idea really. Tests should not fail based on collected coverage.

@ghostdevv
Copy link
Copy Markdown
Contributor

I re-added the exclude as I think it should still be there? WDYT?

*.json does not match the include glob's extensions (.ts, .vue) so it's not needed at all. Does app/, cli/, server/ or shared/ directories contain node_modules? 🤔

Also node_modules is always excluded so it doesn't need to be added. vitest-dev/vitest@1f2d318/packages/vitest/src/node/config/resolveConfig.ts#L497-L523

yeaaa you right, I kinda hoped it'd fix CI though 😆

Also the CI has been failing, any ideas? If not I'll try and take a look soon 🙏 I'll mark as draft while we work on that, and then we can make it ready again when it's working!

No idea really. Tests should not fail based on collected coverage.

🫠

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

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