Refactor : Added Vitest to Requests Screen #2654
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
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: 1
🧹 Outside diff range and nitpick comments (2)
src/screens/Requests/Requests.spec.tsx (2)
25-34: Enhance localStorage mock implementation.While the basic methods are correctly stubbed, consider these improvements:
- Add the
lengthproperty to match the Web Storage API specification- Enhance the JSDoc comment to include example usage and return types
/** * Set up `localStorage` stubs for testing. + * + * @example + * localStorage.getItem('key') // Returns undefined by default + * localStorage.setItem('key', 'value') // Returns undefined */ vi.stubGlobal('localStorage', { getItem: vi.fn(), setItem: vi.fn(), clear: vi.fn(), removeItem: vi.fn(), + length: 0, });
54-61: Consider moving wait utility to shared test utilities.The
waitutility function is well-implemented, but it could be beneficial to move it to a shared test utilities file since it's a generic helper that could be useful in other test files.Consider:
- Moving this function to
src/utils/testUtils.ts- Exporting it alongside other test utilities
- Making the default timeout configurable through an environment variable for CI environments
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/screens/Requests/Requests.spec.tsx(2 hunks)
🔇 Additional comments (2)
src/screens/Requests/Requests.spec.tsx (2)
23-24: LGTM: Vitest import is correctly implemented.
The import of vi from 'vitest' is properly placed and follows Vitest's best practices.
Line range hint 1-300: Overall test implementation looks solid!
The test suite demonstrates:
- Comprehensive coverage of different scenarios
- Proper async handling with act()
- Good cleanup practices
- Appropriate use of Vitest features
The migration to Vitest has been implemented successfully while maintaining the existing test coverage.
| /** | ||
| * Mock `window.location` for testing redirection behavior. | ||
| */ | ||
|
|
||
| Object.defineProperty(window, 'location', { | ||
| value: { href: 'http://localhost/', assign: vi.fn(), reload: vi.fn() }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance window.location mock implementation.
The current implementation could be improved to better match the Window.Location interface:
/**
* Mock `window.location` for testing redirection behavior.
+ *
+ * @example
+ * window.location.href = '/new-path'
+ * expect(window.location.pathname).toBe('/new-path')
*/
Object.defineProperty(window, 'location', {
value: {
href: 'http://localhost/',
assign: vi.fn(),
reload: vi.fn(),
+ pathname: '/',
+ search: '',
+ hash: '',
+ origin: 'http://localhost',
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Mock `window.location` for testing redirection behavior. | |
| */ | |
| Object.defineProperty(window, 'location', { | |
| value: { href: 'http://localhost/', assign: vi.fn(), reload: vi.fn() }, | |
| }); | |
| /** | |
| * Mock `window.location` for testing redirection behavior. | |
| * | |
| * @example | |
| * window.location.href = '/new-path' | |
| * expect(window.location.pathname).toBe('/new-path') | |
| */ | |
| Object.defineProperty(window, 'location', { | |
| value: { | |
| href: 'http://localhost/', | |
| assign: vi.fn(), | |
| reload: vi.fn(), | |
| pathname: '/', | |
| search: '', | |
| hash: '', | |
| origin: 'http://localhost', | |
| }, | |
| }); |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2654 +/- ##
=====================================================
- Coverage 94.55% 83.78% -10.77%
=====================================================
Files 295 312 +17
Lines 7036 8118 +1082
Branches 1516 1830 +314
=====================================================
+ Hits 6653 6802 +149
- Misses 177 1171 +994
+ Partials 206 145 -61 ☔ View full report in Codecov by Sentry. |
|
We have a policy of unassigning contributors who close PRs without getting validation from our reviewer team. This is because:
Please be considerate of our volunteers' limited time and our desire to improve our code base. This policy is stated as a pinned post in all our Talawa repositories. Our YouTube videos explain why this practice is not acceptable to our Community. Unfortunately, if this continues we will have to close the offending PR and unassign you from the issue.
|
What kind of change does this PR introduce?
Added Vitest to Requests Screen
Issue Number: 2569
Fixes #2569
Did you add tests for your changes?
Yes
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Migrated the testing framework to Vitest.
Updated all test files and configurations to be compatible with Vitest's syntax and features.
Have you read the contributing guide?
Yes
Summary by CodeRabbit
vitestlibrary for mockinglocalStorageandwindow.location.waitfor improved asynchronous testing.Requestscomponent while preserving existing structure.