Move remaining non-loot-core packages to latest vitest #4856
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
jfdoming
changed the title
jfdoming
changed the title
WalkthroughThis change migrates the testing framework for the Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/api/vitest.config.ts (1)
1-9: Good configuration, but fix the TypeScript union typeThe new Vitest configuration properly sets up testing with globals enabled and filters console output to only show errors. This is a well-structured configuration for test environments.
There's an issue with using
voidin a union type on line 4. Replace it withundefinedfor better TypeScript practices:- onConsoleLog(log: string, type: 'stdout' | 'stderr'): boolean | void { + onConsoleLog(log: string, type: 'stdout' | 'stderr'): boolean | undefined {🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/api/__snapshots__/methods.test.ts.snapis excluded by!**/*.snapupcoming-release-notes/4856.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
packages/api/jest.config.js(0 hunks)packages/api/methods.test.ts(1 hunks)packages/api/package.json(2 hunks)packages/api/vitest.config.ts(1 hunks)packages/desktop-client/package.json(1 hunks)packages/desktop-client/vite.config.mts(1 hunks)packages/eslint-plugin-actual/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/api/jest.config.js
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: MatissJanis
PR: actualbudget/actual#4108
File: tsconfig.json:22-22
Timestamp: 2025-01-08T20:54:04.680Z
Learning: Jest testing framework is used in multiple packages (api, crdt, loot-core) while desktop-client uses Vitest. Both Jest and Vitest types should be kept in the root tsconfig.json.
🧬 Code Graph Analysis (1)
packages/api/methods.test.ts (1)
packages/desktop-client/src/build-shims.js (1)
global(1-1)
🪛 Biome (1.9.4)
packages/api/vitest.config.ts
[error] 4-4: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (7)
packages/eslint-plugin-actual/package.json (1)
18-18: Version bump approvedThe vitest package has been updated to version ^3.0.2, aligning with the PR objective to upgrade all non-loot-core packages to the latest Vitest.
packages/desktop-client/package.json (1)
83-83: Version bump approvedThe vitest package has been updated to version ^3.0.2, consistent with the PR objective to upgrade all non-loot-core packages to the latest Vitest.
packages/api/methods.test.ts (1)
9-9:❓ Verification inconclusive
Setting global testing flag instead of restoring date
This change sets a global testing flag at the beginning rather than calling
global.restoreDateNow()inside the beforeEach block. This aligns with the migration from Jest to Vitest.To ensure this change doesn't introduce issues, verify that all tests dependent on date handling still pass with this new approach by running the test suite for the API package:
🏁 Script executed:
#!/bin/bash # Navigate to the API package and run tests cd packages/api && yarn testLength of output: 248
Action Required: Confirm API tests pass with new global.IS_TESTING flag
I’ve replaced
global.restoreDateNow()in our Vitest migration with a singleglobal.IS_TESTING = trueat the top of methods.test.ts. To ensure no date‐related tests break:
- Install dependencies and run the test suite:
yarn install cd packages/api && yarn test- Verify that all tests—especially those relying on mocked dates—pass successfully.
packages/api/package.json (3)
17-17: Build dependency added for CRDT packageAdding this script ensures the CRDT package is built before running tests, which is now referenced in the test script.
22-22: Successfully migrated from Jest to VitestThe test script now properly builds dependencies and runs tests using Vitest instead of Jest. This change aligns with the PR objective of migrating non-loot-core packages to Vitest.
35-36: Dependencies updated for Vitest migrationAppropriate dependencies have been updated to support Vitest migration. The TypeScript version has been updated to 5.8.2 and Vitest to 3.0.2.
packages/desktop-client/vite.config.mts (1)
197-200: Improved test output by filtering console logsThis change adds filtering for console output during tests, allowing only error logs to be displayed. This reduces noise in test output and makes errors more visible, consistent with similar changes in the API package's Vitest configuration.
Follow-up PR to #4851 and #4840