-
-
Notifications
You must be signed in to change notification settings - Fork 860
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
setup of vitest #3238
setup of vitest #3238
Conversation
Signed-off-by: NishantSinghhhhh <nishantsingh_230137@aitpune.edu.in>
WalkthroughThe pull request modifies the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
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
|
@palisadoes , I have added the setup file for vitest |
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)
vitest.setup.ts (2)
12-23
: Consider improving type safety and error handling.The current implementation could be enhanced for better type safety and more comprehensive error handling.
Consider these improvements:
- console.error = (...args: unknown[]) => { - const firstArg = args[0]; - if ( - typeof firstArg === 'string' && - /Warning: ReactDOM.render is no longer supported in React 18./.test( - firstArg, - ) - ) { + console.error = (...args: unknown[]) => { + const [firstArg, ...rest] = args; + const REACT_18_WARNINGS = [ + 'Warning: ReactDOM.render is no longer supported in React 18.', + // Add other React 18 warnings here + ]; + if ( + typeof firstArg === 'string' && + REACT_18_WARNINGS.some(warning => firstArg.includes(warning)) + ) {
4-5
: Consider enhancing documentation.While the current comments are helpful, consider adding more detailed documentation about:
- Why specific React 18 warnings are being suppressed
- The importance of the cleanup process
- Any potential side effects of these configurations
Example documentation:
-// Basic cleanup after each test +/** + * Cleanup after each test + * This ensures that: + * 1. DOM is cleared between tests + * 2. No test side-effects leak into other tests + * 3. Each test starts with a fresh environment + */ -// Simple console error handler for React 18 warnings +/** + * Custom console error handler for React 18 + * Suppresses specific React 18 migration warnings during testing + * while preserving other important error messages + * @see https://react.dev/blog/2022/03/08/react-18-upgrade-guide + */Also applies to: 9-10
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
vitest.setup.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (3)
vitest.setup.ts (3)
1-8
: Well-structured test cleanup implementation!The cleanup setup follows testing-library best practices by ensuring each test starts with a clean DOM state.
26-28
: LGTM! Proper cleanup of console overrides.The afterAll hook correctly restores the original console.error functionality, ensuring a clean test environment.
9-24
: Verify the impact of suppressing React 18 warnings.While suppressing the ReactDOM.render warning is reasonable for React 18 migration, ensure this doesn't hide other important warnings.
Let's check for other potential React 18 warnings that might need handling:
✅ Verification successful
Warning suppression is properly implemented and scoped
The warning suppression is specifically targeting only the ReactDOM.render deprecation warning in React 18, while properly preserving all other console errors. This is a safe and common practice during React 18 migration, especially in test environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for React 18 related warnings in test files rg -i "warning.*react 18" --type ts --type tsxLength of output: 76
Script:
#!/bin/bash # Find all TypeScript and test setup files echo "=== TypeScript and Setup Files ===" fd "\.(ts|tsx)$" # Search for React 18 related warnings and patterns echo -e "\n=== React 18 Related Patterns ===" rg -i "react 18|reactdom\.render" # Look for console.error interceptors echo -e "\n=== Console Error Interceptors ===" rg "console\.error.*=.*\(" -A 5Length of output: 31912
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3238 +/- ##
=====================================================
+ Coverage 10.74% 90.17% +79.43%
=====================================================
Files 309 330 +21
Lines 7802 8488 +686
Branches 1729 1933 +204
=====================================================
+ Hits 838 7654 +6816
+ Misses 6900 604 -6296
- Partials 64 230 +166 ☔ View full report in Codecov by Sentry. |
How will this affect all the jest testing? |
@palisadoes , This is not affecting jest testing in any way , the jestTest setup file that was made , I have not changed any code in that , I have just added code to the pre-existing file vitest.setup.ts |
ea2f4ea
into
PalisadoesFoundation:develop-postgres
PR Description for vitest.setup.ts Changes
What kind of change does this PR introduce?
Testing configuration: Adding/modifying Vitest setup configuration for the project.
Issue Number:
Fixes : #2753
Did you add tests for your changes?
Yes, the changes are related to the testing infrastructure itself.
Snapshots/Videos:
Screencast from 2025-01-10 12-03-08.webm
jest tests are passing
vitests are passing too
https://github.com/user-attachments/assets/bd6c6d90-ee7b-4f7d-884c-bc8e114d2dfa
If relevant, did you update the documentation?
No
Summary
This PR introduces/modifies the vitest.setup.ts file which configures the testing environment for the project. The changes include:
These changes ensure consistent and reliable test execution across the project.
Does this PR introduce a breaking change?
No
Other information
The setup file follows Vitest best practices and maintains compatibility with existing test suites.
Have you read the contributing guide?
Yes