Migrated src/screens/SubTags from Jest to Vitest #2739
Conversation
WalkthroughThis pull request focuses on migrating the test suite for the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
|
![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 (2)
src/screens/SubTags/SubTags.spec.tsx (2)
118-118: Clear additional mocks if needed.vi.clearAllMocks() is a solid approach, but if you rely on timers or user-event stubs, consider also clearing timers or restoring modules for thorough cleanup.
122-122: Test naming consistency.Using “it” blocks is consistent with Vitest style. Ensure test descriptions are consistently phrased to maintain clarity in test reports.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/SubTags/SubTags.spec.tsx(14 hunks)src/screens/SubTags/SubTags.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/screens/SubTags/SubTags.tsx
🔇 Additional comments (14)
src/screens/SubTags/SubTags.spec.tsx (14)
24-25: Nice Migration to Vitest Imports.
You've correctly replaced Jest imports with Vitest equivalents. This makes the test suite consistent with Vitest APIs.
132-132: Good coverage of error rendering scenario.
No issues found. This test ensures correct fallback when the query fails.
142-142: Modal open/close test is robust.
This test thoroughly verifies modal behavior through clicks and waiting for elements to unmount. Nicely done.
164-164: Manage Tag navigation is validated.
The flow verifying navigation to “manage tag” is straightforward. Good approach using screen.getByTestId.
179-179: Intra-tag navigation tested properly.
This ensures consistent user interactions for exploring sub tags.
194-194: Breadcrumb click behavior validated.
The test ensures the correct redirection after breadcrumb clicks. Very thorough coverage.
209-209: Navigation back to all tags is confirmed.
This meets user expectation for returning to the main tags screen.
224-224: “Manage Current Tag” button is effectively tested.
Ensures that the user can truly navigate to manage the currently selected tag.
239-239: Search functionality is well tested.
Verifying the count of results ensures accurate data flow for the search feature.
260-260: Sorting logic coverage is strong.
This test checks both ascending and descending states of tag ordering, ensuring robust coverage.
346-346: Add sub tag test ensures success toast.
Using toast.success is properly validated. Confirm that no unexpected toast error occurs when successful.
48-51: Check the Toastify mock usage.
Using vi.mock for 'react-toastify' is appropriate. Ensure you verify in your tests that toast messages are invoked in the correct sequences (e.g., success before error).
✅ Verification successful
Let me search for the test file and its related implementation to verify the toast message sequences.
Toast messages are correctly sequenced and tested
The implementation shows that toast messages are used appropriately and mutually exclusive - success is called only after successful tag creation, while error is called in the error handling block. The test file includes a specific test case that verifies the success toast message is called with the correct translation string after adding a new sub tag.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's look at the test file content
cat src/screens/SubTags/SubTags.spec.tsx
# Then search for toast usage in the implementation
rg "toast\." src/screens/SubTags/SubTags.tsx -A 2
Length of output: 10615
111-112: Confirm partial mocking approach for react-router-dom.
Using vi.importActual inside vi.mock is an advanced approach for partial mocking. Make sure you confirm no essential functionality is inadvertently overridden.
✅ Verification successful
Let me gather more information about the mocking approach used in the codebase.
Let me try again with corrected file type flags.
Let me try one more search with different flags to find more context.
Based on the analysis of the test file and other mocking patterns in the codebase, I can now provide a final response:
The partial mocking approach for react-router-dom is safe and consistent with codebase patterns.
The partial mocking implementation is being used correctly and safely because:
- It's used in a
beforeEachblock, ensuring clean setup for each test - It's properly cleaned up in
afterEachwithvi.clearAllMocks() - The pattern is consistently used across multiple test files in the codebase (e.g., in UserPortal, OrganizationDashboard, etc.)
- The mock preserves the original functionality while allowing for specific overrides when needed
- No essential functionality is being overridden as it's importing the actual implementation with
vi.importActual
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for similar partial mocking patterns with vi.mock and vi.importActual
rg -U "vi\.mock.*\(\s*'react-router-dom'.*\{[\s\S]*?vi\.importActual.*\}" --type ts --type tsx
# Look for other test files that might be using react-router-dom mocks
rg "vi\.mock.*'react-router-dom'" --type ts --type tsx
Length of output: 221
Script:
#!/bin/bash
# Search for similar partial mocking patterns with vi.mock and vi.importActual
rg -U "vi\.mock.*\(\s*'react-router-dom'.*\{[\s\S]*?vi\.importActual.*\}" -t typescript -t tsx
# Look for other test files that might be using react-router-dom mocks
rg "vi\.mock.*'react-router-dom'" -t typescript -t tsx
# Look for the specific test file and its context
rg -A 5 "vi\.mock.*'react-router-dom'" src/screens/SubTags/SubTags.spec.tsx
Length of output: 455
Script:
#!/bin/bash
# Look for the complete test file content to understand the mocking context
cat src/screens/SubTags/SubTags.spec.tsx
# Search for any other vi.mock usage in spec files
find . -name "*.spec.tsx" -o -name "*.test.tsx" | xargs rg "vi\.mock"
Length of output: 21100
317-317: Infinite scroll test replicates user scrolling behavior.
Simulating the scroll event is a good approach. Confirm the appended tags appear correctly after scrolling.
✅ Verification successful
Let me check the test implementation to verify the infinite scroll behavior.
Let me try again with a more generic file type.
Let me get more context to see the complete test implementation.
Let me get the final part of the test to see the assertions after scrolling.
Let me try one more time with escaped special characters.
Test implementation correctly verifies infinite scroll behavior
The test implementation properly verifies the infinite scroll functionality by:
- Getting initial count of tags using
manageTagBtntest ID - Simulating scroll to bottom using
fireEvent.scroll - Verifying more tags are loaded by checking the final count is greater than initial count
- Ensuring UI stability by verifying "Add Child Tag" button remains present
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Let's examine the test implementation
rg -A 10 "Fetches more sub tags with infinite scroll" --type tsx
Length of output: 94
Script:
#!/bin/bash
# Let's examine the test implementation
rg -A 10 "Fetches more sub tags with infinite scroll" --type ts
Length of output: 788
Script:
#!/bin/bash
# Let's examine the complete test implementation with more context
rg -B 5 -A 20 "Fetches more sub tags with infinite scroll" --type ts
Length of output: 1716
Script:
#!/bin/bash
# Let's examine the complete test implementation including the final assertions
rg -B 20 -A 5 "target: { scrollY: subTagsScrollableDiv.scrollHeight }" --type ts
Length of output: 229
Script:
#!/bin/bash
# Let's examine the complete test implementation including the final assertions
rg -B 20 -A 10 "scrollHeight" --type ts
Length of output: 16714
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2739 +/- ##
=====================================================
+ Coverage 75.01% 87.06% +12.05%
=====================================================
Files 296 313 +17
Lines 7359 8210 +851
Branches 1608 1852 +244
=====================================================
+ Hits 5520 7148 +1628
+ Misses 1572 881 -691
+ Partials 267 181 -86 ☔ View full report in Codecov by Sentry. |
src/screens/SubTags/SubTags.spec.tsx
Outdated
| @@ -22,6 +21,8 @@ import i18n from 'utils/i18nForTest'; | |||
| import SubTags from './SubTags'; | |||
| import { MOCKS, MOCKS_ERROR_SUB_TAGS } from './SubTagsMocks'; | |||
| import { InMemoryCache, type ApolloLink } from '@apollo/client'; | |||
| import { vi, beforeEach, afterEach, expect, it } from 'vitest'; | |||
| import '@testing-library/jest-dom/vitest'; | |||
There was a problem hiding this comment.
Why was this imported if we are migrating from jest and it doesn't appear to be used?
There was a problem hiding this comment.
Sure, I am removing this and will push very soon.
Thanks for guiding.
PratapRathi
force-pushed
the
Migrate-src/screens/SubTags-from-Jest-to-Vitest
branch
from
abdba2a to
00f2067
Compare
palisadoes
merged commit cd0b041
into
PalisadoesFoundation:develop-postgres
PratapRathi
deleted the
Migrate-src/screens/SubTags-from-Jest-to-Vitest
branch
What kind of change does this PR introduce?
This PR migrates the test cases in src/screens/SubTags from Jest to Vitest, ensuring compatibility with Vitest and maintaining 100% test coverage.
Issue Number:
Fixes #2570
Did you add tests for your changes?
Yes
Snapshots/Videos:
If relevant, did you update the documentation?
N/A
Summary
Does this PR introduce a breaking change?
No
Other information
N/A
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Tests
Chores