-
-
Notifications
You must be signed in to change notification settings - Fork 758
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
[REFACTOR]: Jest to Vitest migration for chartToPdf.test.ts #2864
[REFACTOR]: Jest to Vitest migration for chartToPdf.test.ts #2864
Conversation
WalkthroughThis pull request refactors the unit tests for CSV export functions in the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/utils/chartToPdf.spec.ts (2)
23-30
: Strong usage of DOM method mocks.
Mockingdocument.createElement
,appendChild
, andremoveChild
helps in verifying side effects without requiring an actual DOM. Ensure that these mocks are reset or cleared after each test to prevent cross-test interference.
83-106
: Appropriate coverage of event labels and attendance trends.
TestingexportTrendsToCSV
with valid numeric arrays confirms correct CSV generation. Consider adding a test for mismatched array lengths or empty arrays inexportTrendsToCSV
, similar toexportDemographicsToCSV
.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/utils/chartToPdf.spec.ts
(1 hunks)src/utils/chartToPdf.test.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- src/utils/chartToPdf.test.ts
🔇 Additional comments (7)
src/utils/chartToPdf.spec.ts (7)
1-2
: Good use of Vitest imports.
Nice to see the direct import of Vitest’s testing utilities and the Mock
type, ensuring strong type checking across the test suite.
18-21
: Validate mocking for URL methods.
Mocking URL.createObjectURL
and URL.revokeObjectURL
is a standard approach in a browser environment test scenario. Good job isolating external dependencies.
51-67
: Great coverage for CSV formatting validations.
The test verifies that the generated link is properly created and removed, the CSV is triggered for download, and the object URL is revoked. This confirms a typical CSV export flow.
69-75
: Useful edge case checks for empty data and filename.
Throwing an error ensures that these scenarios are caught early and surfaces valuable feedback.
77-81
: Validation of appending “.csv” extension.
This test ensures that your function is user-friendly by automatically appending the file extension if it’s omitted.
108-151
: Robust test coverage for demographics export scenarios.
These tests cover valid data, missing category names, array length mismatches, and safe filename generation with timestamps. This thoroughness ensures high reliability.
3-7
: Confirm that the tested functions exist and remain importable.
If chartToPdf
is also undergoing changes, confirm that the named exports remain in sync with these test implementations.
✅ Verification successful
All exported functions exist and match the imports in the test file
The verification confirms that all three functions (exportToCSV
, exportTrendsToCSV
, and exportDemographicsToCSV
) are properly exported from src/utils/chartToPdf.ts
and match the imports in the test file. They are defined as:
- Line 3:
export const exportToCSV
- Line 50:
export const exportTrendsToCSV
- Line 80:
export const exportDemographicsToCSV
The exports and imports are in sync, and no issues were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching to ensure that 'exportToCSV', 'exportTrendsToCSV', and
# 'exportDemographicsToCSV' exist in 'chartToPdf.ts' or 'chartToPdf.js'
ast-grep --pattern $'export function exportToCSV($_) {\n $$$\n}\n\nexport function exportTrendsToCSV($_) {\n $$$\n}\n\nexport function exportDemographicsToCSV($_) {\n $$$\n}'
Length of output: 168
Script:
#!/bin/bash
# Let's try a different approach to find these exports
# First, let's check if the file exists
fd "chartToPdf.ts|chartToPdf.js" src/
# Then let's look for exports in the file
rg "export.*(?:exportToCSV|exportTrendsToCSV|exportDemographicsToCSV)" src/utils/chartToPdf.ts -p
Length of output: 319
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2864 +/- ##
=====================================================
+ Coverage 64.43% 87.81% +23.38%
=====================================================
Files 296 313 +17
Lines 7369 8224 +855
Branches 1609 1855 +246
=====================================================
+ Hits 4748 7222 +2474
+ Misses 2386 802 -1584
+ Partials 235 200 -35 ☔ View full report in Codecov by Sentry. |
87bfd94
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
This PR migrates the test cases in src/utils/chartToPdf.test.ts from Jest to Vitest, ensuring compatibility with Vitest .
✅ Replace Jest-specific functions and mocks with Vitest equivalents
✅ Ensure all tests in src/utils/chartToPdf.test.ts from Jest to Vitest.pass after migration using npm run test:vitest
✅ Maintain the test coverage for the file as 100% after migration
✅ Upload a video or photo for this specific file coverage is 100% in the PR description
Issue Number:
Fixes #2760
Did you add tests for your changes?
No
Snapshots/Videos:
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Yes
Summary by CodeRabbit