-
-
Notifications
You must be signed in to change notification settings - Fork 869
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: src/utils/useSession.test.tsx from Jest to Vitest #2952
Refactor: src/utils/useSession.test.tsx from Jest to Vitest #2952
Conversation
WalkthroughThis pull request involves the removal of the existing Jest test suite for the Changes
Assessment against linked issues
Possibly related issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (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
|
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/useSession.spec.tsx (2)
478-509
: Address Prettier warnings.ESLint/Prettier flagged spacing, indentation, and missing trailing commas. Align the code with the project’s formatting rules to maintain consistency and avoid future merge conflicts.
Here’s an example partial fix for indentation/trailing commas around lines 505 and 539:
- expect(toast.warning).toHaveBeenCalledWith('sessionWarning') + expect(toast.warning).toHaveBeenCalledWith('sessionWarning');And:
- expect(window.removeEventListener).toHaveBeenCalledWith( - 'mousemove' - expect.any(Function) - ); + expect(window.removeEventListener).toHaveBeenCalledWith( + 'mousemove', + expect.any(Function), + );Also applies to: 517-547, 578-589, 598-627
🧰 Tools
🪛 eslint
[error] 478-478: Delete
··
(prettier/prettier)
[error] 488-488: Delete
··
(prettier/prettier)
[error] 491-491: Delete
··
(prettier/prettier)
[error] 494-494: Delete
··
(prettier/prettier)
[error] 497-497: Delete
··
(prettier/prettier)
[error] 500-500: Delete
··
(prettier/prettier)
[error] 503-503: Delete
··
(prettier/prettier)
[error] 505-505: Insert
,
(prettier/prettier)
581-582
: Unused parameters in arrow functions.ESLint indicates that
_
is assigned but never used. Consider renaming these unused parameters to_unused
or omitting them altogether if not needed.- const hasWarningTimeout = timeoutCalls.some((call: Parameters<typeof setTimeout>) => { - const [_, ms] = call; + const hasWarningTimeout = timeoutCalls.some((timeoutCall: Parameters<typeof setTimeout>) => { + const [, ms] = timeoutCall;Also applies to: 587-588, 620-620
🧰 Tools
🪛 eslint
[error] 581-581: Insert
··
(prettier/prettier)
[error] 581-581: '_' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 582-582: Replace
····
with······
(prettier/prettier)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/useSession.spec.tsx
(1 hunks)
🧰 Additional context used
🪛 eslint
src/utils/useSession.spec.tsx
[error] 478-478: Delete ··
(prettier/prettier)
[error] 488-488: Delete ··
(prettier/prettier)
[error] 491-491: Delete ··
(prettier/prettier)
[error] 494-494: Delete ··
(prettier/prettier)
[error] 497-497: Delete ··
(prettier/prettier)
[error] 500-500: Delete ··
(prettier/prettier)
[error] 503-503: Delete ··
(prettier/prettier)
[error] 505-505: Insert ,
(prettier/prettier)
[error] 509-512: Delete ⏎⏎⏎
(prettier/prettier)
[error] 517-517: Delete ··
(prettier/prettier)
[error] 531-531: Insert ,
(prettier/prettier)
[error] 535-535: Insert ,
(prettier/prettier)
[error] 539-539: Insert ,
(prettier/prettier)
[error] 547-547: Delete ··
(prettier/prettier)
[error] 578-578: Delete ··
(prettier/prettier)
[error] 580-580: Insert ⏎····
(prettier/prettier)
[error] 581-581: Insert ··
(prettier/prettier)
[error] 581-581: '_' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 582-582: Replace ····
with ······
(prettier/prettier)
[error] 583-583: Replace }
with ··},⏎··
(prettier/prettier)
[error] 586-586: Insert ⏎····
(prettier/prettier)
[error] 587-587: Insert ··
(prettier/prettier)
[error] 587-587: '_' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 588-588: Insert ··
(prettier/prettier)
[error] 589-589: Replace }
with ··},⏎··
(prettier/prettier)
[error] 598-600: Delete ⏎⏎
(prettier/prettier)
[error] 601-601: Replace .spyOn(console,·'error')
with ⏎····.spyOn(console,·'error')⏎····
(prettier/prettier)
[error] 603-603: Delete ··
(prettier/prettier)
[error] 605-605: Insert ⏎····
(prettier/prettier)
[error] 620-620: 'error' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 624-624: Delete ··
(prettier/prettier)
[error] 632-632: Delete ··
(prettier/prettier)
[error] 667-667: Replace call
with (call)
(prettier/prettier)
[error] 669-669: Replace Math.abs(duration·-·expectedWarningTime)·<=·expectedWarningTime·*·0.05
with (⏎······Math.abs(duration·-·expectedWarningTime)·<=·expectedWarningTime·*·0.05⏎····)
(prettier/prettier)
[error] 672-672: Replace call
with (call)
(prettier/prettier)
[error] 674-674: Replace Math.abs(duration·-·expectedSessionTime)·<=·expectedSessionTime·*·0.05
with (⏎······Math.abs(duration·-·expectedSessionTime)·<=·expectedSessionTime·*·0.05⏎····)
(prettier/prettier)
🔇 Additional comments (19)
src/utils/useSession.spec.tsx (19)
1-10
: Well-structured test arrangement.
The new test file cleanly migrates from Jest to Vitest, demonstrating thorough coverage of session-related scenarios. Mocking react-toastify
and react-i18next
is concise and idiomatic, ensuring each test is isolated. Overall, these top lines do not raise immediate concerns.
13-23
: Mocks align well with Vitest standards.
By mocking react-toastify
and utils/errorHandler
in this manner, each test runs in a self-contained environment. The approach is logically consistent and helps avoid side effects from external modules.
25-29
: Appropriate i18n mocking.
Mocking react-i18next
ensures the tests avoid real translation complexities and remain stable.
75-110
: Effective usage of timers and event dispatch for session tests.
Fake timers are used correctly to advance time and validate warnings, and the test dispatches a visibility change event to simulate the user returning to the tab. This thoroughly validates session events.
112-147
: Good coverage for visibility change edge cases.
The hidden state scenario ensures no premature warnings. This test verifies correct removal of event listeners, covering potential memory leaks or unexpected calls when the document is not visible.
149-184
: Robust asserts for event listeners.
You verify that the listeners mousemove
, keydown
, and visibilitychange
are attached correctly. This coverage ensures that any future changes to event listener logic must pass these tests.
187-209
: Clear handling of session timeout logic.
Fake timers are advanced to simulate session expiration. The test includes checks for both localStorage clearance and toast calls. This ensures correct log-out behavior.
233-279
: Thoughtful handling of token revocation failures.
Capturing console.error
and verifying that it was called ensures robust error handling. This test effectively defends against silent failures in token revocation logic.
281-295
: Use of global spies to verify session timeout is well-designed.
By spying on global.setTimeout
, the suite ensures the correct scheduling of session events. This is a standard practice when testing time-based logic.
297-318
: Correct usage of mocked error-handling for GraphQL queries.
Ensuring errorHandler
is called during query failure guards against unhandled rejections or missing error checks.
320-356
: Coverage of endSession logic.
Verifying that event listeners are removed confirms that resources are cleaned up and no memory leaks remain after the session ends.
358-401
: Smart approach to tab visibility-based inactivity.
This test scenario simulates leaving and returning to the tab with partial inactivity across multiple intervals, ensuring that timeouts are recalculated appropriately.
403-450
: Proper session timeout during hidden state inactivity.
Especially useful for real-life scenarios, the test confirms correct logout behavior even if a user remains away from the tab for extended periods of time.
452-474
: Verification of logout flow and token revocation.
Ensuring that localStorage is cleared and a toast is displayed meets user expectations for sign-out flows. This coverage is important for security checks.
475-509
: Extend session functionality validated.
The test confirms that extending the session resets warnings, ensuring a smooth user experience for those extending sessions.
🧰 Tools
🪛 eslint
[error] 478-478: Delete ··
(prettier/prettier)
[error] 488-488: Delete ··
(prettier/prettier)
[error] 491-491: Delete ··
(prettier/prettier)
[error] 494-494: Delete ··
(prettier/prettier)
[error] 497-497: Delete ··
(prettier/prettier)
[error] 500-500: Delete ··
(prettier/prettier)
[error] 503-503: Delete ··
(prettier/prettier)
[error] 505-505: Insert ,
(prettier/prettier)
513-543
: Unmount listener cleanup.
Confirming removeEventListener
was called on unmount helps avoid memory leaks. The approach is consistent with React’s lifecycle.
🧰 Tools
🪛 eslint
[error] 517-517: Delete ··
(prettier/prettier)
[error] 531-531: Insert ,
(prettier/prettier)
[error] 535-535: Insert ,
(prettier/prettier)
[error] 539-539: Insert ,
(prettier/prettier)
544-596
: Graceful handling of missing community data.
Even when getCommunityData
returns null
, the tests confirm that session timeouts are still set properly with default values—a robust fallback scenario.
🧰 Tools
🪛 eslint
[error] 547-547: Delete ··
(prettier/prettier)
[error] 578-578: Delete ··
(prettier/prettier)
[error] 580-580: Insert ⏎····
(prettier/prettier)
[error] 581-581: Insert ··
(prettier/prettier)
[error] 581-581: '_' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 582-582: Replace ····
with ······
(prettier/prettier)
[error] 583-583: Replace }
with ··},⏎··
(prettier/prettier)
[error] 586-586: Insert ⏎····
(prettier/prettier)
[error] 587-587: Insert ··
(prettier/prettier)
[error] 587-587: '_' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 588-588: Insert ··
(prettier/prettier)
[error] 589-589: Replace }
with ··},⏎··
(prettier/prettier)
600-627
: Error handling on event listener registration.
The test forcibly throws an error and checks against console.error
, ensuring the code logs unexpected issues properly. This scenario can reveal hidden device or environment constraints.
🧰 Tools
🪛 eslint
[error] 601-601: Replace .spyOn(console,·'error')
with ⏎····.spyOn(console,·'error')⏎····
(prettier/prettier)
[error] 603-603: Delete ··
(prettier/prettier)
[error] 605-605: Insert ⏎····
(prettier/prettier)
[error] 620-620: 'error' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 624-624: Delete ··
(prettier/prettier)
629-682
: Timeout data updates are validated.
Your approach of fetching updated session timeouts and verifying their scheduling with setTimeout
ensures timeouts match new data. The test is thorough, though appear aware of the tolerance check around ±5%.
🧰 Tools
🪛 eslint
[error] 632-632: Delete ··
(prettier/prettier)
[error] 667-667: Replace call
with (call)
(prettier/prettier)
[error] 669-669: Replace Math.abs(duration·-·expectedWarningTime)·<=·expectedWarningTime·*·0.05
with (⏎······Math.abs(duration·-·expectedWarningTime)·<=·expectedWarningTime·*·0.05⏎····)
(prettier/prettier)
[error] 672-672: Replace call
with (call)
(prettier/prettier)
[error] 674-674: Replace Math.abs(duration·-·expectedSessionTime)·<=·expectedSessionTime·*·0.05
with (⏎······Math.abs(duration·-·expectedSessionTime)·<=·expectedSessionTime·*·0.05⏎····)
(prettier/prettier)
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.
You need to delete src/utils/useSession.test.tsx as requested in the issue
Please fix the failing tests |
Reopening. You don't have to close the PR. Just make the changes in your local branch and then push to your origin. The PR will automatically get updated and re-run all the workflows. |
What kind of change does this PR introduce?
Refactoring from Jest to Vitest for the specified test file.
Issue Number
Fixes #2754
Did you add tests for your changes?
Yes, all test cases were updated and verified to work with Vitest.
Snapshots/Videos
If relevant, did you update the documentation?
Not applicable, as this is a refactor of existing tests.
Summary
.test.tsx
to.spec.tsx
.npm run test:vitest
.Does this PR introduce a breaking change?
No, it is a refactor of existing test cases with no changes to the codebase logic.
Other Information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
useSession
hook.