-
Notifications
You must be signed in to change notification settings - Fork 5
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
chore: Use vitest for testing #71
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 97.04% 95.60% -1.45%
==========================================
Files 43 47 +4
Lines 1015 2889 +1874
Branches 119 364 +245
==========================================
+ Hits 985 2762 +1777
- Misses 28 123 +95
- Partials 2 4 +2 ☔ View full report in Codecov by Sentry. |
function setupTest( | ||
test: (process: (input: string) => Promise<string>) => Promise<void> | ||
): (callback: () => void) => void { | ||
return (callback: () => void) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test used test('...', done => {})
syntax, which is not supported by vitest, had to rewrite
Codecov complains about no test coverage in vitest configuration files. All expected, ignoring this |
vitest.build.mjs
Outdated
|
||
export default defineConfig({ | ||
test: { | ||
include: ['./src/build/**/*.test.ts'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, in the old configuration the tests in src/shared
would be covered by both the browser configuration (jest.browser.config.js
) and the build configuration (jest.build.config.js
), wheras after thees changes, they will only be covered by the browser configuration (vitest.browser.mjs
) and not by the build configuration (vitest.build.mjs
), because of this include path. Is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not intentional, thanks for finding. Fixed now
Are Codecov reports expected to work once Vitest releases this fix, or are further changes required from our side? |
No, this is unrelated. It complains about vitest config files coverage. See the comment above: #71 (comment) Because this is first time setup only, I am going to simply skip this check |
Oh I see. But would it be possible to exclude these files from coverage? |
I think I found a fix |
Improve dev experience, because we cannot build this package on Mac sass/dart-sass#1692
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.