-
-
Notifications
You must be signed in to change notification settings - Fork 813
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
Refactored src/screens/OrgList from jest to vitest #3200
Refactored src/screens/OrgList from jest to vitest #3200
Conversation
WalkthroughThis pull request focuses on refactoring the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (4)
src/screens/OrgList/OrgList.spec.tsx (4)
43-50
: Consider enhancing test cleanupWhile the migration to Vitest's mocking system is correct, consider adding
vi.restoreAllMocks()
in the afterEach block to ensure all spies are properly restored to their original state. This prevents any potential leakage between tests.afterEach(() => { localStorage.clear(); cleanup(); vi.clearAllMocks(); + vi.restoreAllMocks(); });
Line range hint
34-41
: Optimize wait helper functionThe custom wait helper function could be improved by using Vitest's built-in timer utilities.
-async function wait(ms = 100): Promise<void> { - await act(() => { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); - }); +async function wait(ms = 100): Promise<void> { + await act(async () => { + vi.advanceTimersByTime(ms); + }); }
Line range hint
367-369
: Remove commented codeRemove the commented out code as it's no longer needed and may cause confusion.
- // await act(async () => { - // await new Promise((resolve) => setTimeout(resolve, 1000)); - // });
Line range hint
1-1
: Consider adding Vitest configuration fileSince this is a migration to Vitest, consider adding a
vitest.config.ts
file to centralize test configuration, including timeout settings and other Vitest-specific options.Would you like me to generate a sample
vitest.config.ts
file with recommended settings?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/OrgList/OrgList.spec.tsx
(3 hunks)src/screens/OrgList/OrgListMocks.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- src/screens/OrgList/OrgListMocks.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
src/screens/OrgList/OrgList.spec.tsx (3)
29-31
: LGTM: Proper Vitest configuration setupThe migration from Jest to Vitest includes the correct import and timeout configuration. The timeout value of 30000ms is maintained from the previous Jest configuration.
456-456
: LGTM: Proper migration of toast spyThe toast spy has been correctly migrated from Jest to Vitest.
Line range hint
1-558
: Verify complete migration from JestLet's ensure no Jest-specific code remains in the codebase.
the check that is failing is when previous jest cases are run, as migrating this one reduces the threshold coverage below 20%. |
How can that threshold be lowered? |
@palisadoes I think we can make changes in the jest.config.js file: |
I updated it. Please merge with the latest upstream |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3200 +/- ##
=====================================================
+ Coverage 18.70% 89.81% +71.10%
=====================================================
Files 308 329 +21
Lines 7798 8526 +728
Branches 1697 1854 +157
=====================================================
+ Hits 1459 7658 +6199
+ Misses 6241 636 -5605
- Partials 98 232 +134 ☔ View full report in Codecov by Sentry. |
de5330b
into
PalisadoesFoundation:develop-postgres
* prevented unnecessary page reload with complementary test * Update jest.config.js * Fixes #2986 - Multiple UI Updates (#3165) * UI fixes on organisation pages * Added TSDoc for Truncated Text * Added Debouncer * Update src/components/OrgListCard/OrgListCard.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Added code rabbit suggestions * Fixed test error --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * refactore src/screens/OrgList from jest to vitest (#3200) * Improved Code Coverage in src/components/Venues/VenueModal.tsx (#3203) * Improved Code Coverage in src/components/Venues/VenueModal.tsx * removed the ignore statements from VenueModal.tsx * Removed istanbul ignore lines. Code coverage remians 100% (#3207) * refactored src/screens/FundCampaignPledge from jest to vitest (#3208) * prettier formatting and disabled ts-specific rules for js in eslint (#3186) * Improve Code Coverage in src/screens/UserPortal/Settings/Settings.tsx (#3189) * Preventing Overflow of images in Advertisement and Venue Post modals (#3204) * improve code coverage of src/screens/EventManagement (#3149) * code coverage * jest global coverage decreased * global jest coverage * rename file problem solved * changes requested resolved * fix: update Chat section title to 'Chats' (#3216) * removed stale comment line * Revert "removed stale comment line" This reverts commit e0fa894. * removed stale comment line --------- Co-authored-by: Peter Harrison <[email protected]> Co-authored-by: Mehul Aggarwal <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Syed Ali Ul Hasan <[email protected]> Co-authored-by: harshk-89 <[email protected]> Co-authored-by: Amaan ali <[email protected]> Co-authored-by: Yugal Sadhwani <[email protected]> Co-authored-by: Pranav Nathe <[email protected]> Co-authored-by: prathmesh703 <[email protected]> Co-authored-by: Nivedita <[email protected]>
What kind of change does this PR introduce?
Refactoring
Issue Number:
Fixes #2565
Did you add tests for your changes?
Yes
Snapshots/Videos:
N/A
If relevant, did you update the documentation?
N/A
Summary
Refactored src/screens/OrgList/OrgList.test.tsx to src/screens/OrgList/OrgList.spec.tsx
Does this PR introduce a breaking change?
No
Other information
N/A
Have you read the contributing guide?
Summary by CodeRabbit
Testing Framework
Mock Data
adminUser
variable