added vitest to chat.jsx #2649
added vitest to chat.jsx #2649
Conversation
Signed-off-by: NishantSinghhhhh <[email protected]>
|
Warning Rate limit exceeded@NishantSinghhhhh has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request includes updates to the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
🧹 Outside diff range and nitpick comments (6)
src/screens/UserPortal/Chat/Chat.spec.tsx (6)
1473-1485: Consider moving window mocks to a separate test utils file.The window mock implementations could be extracted to a separate test utils file to improve maintainability and reusability across test files.
// Create a new file: src/test/utils/windowMocks.ts +export const setupWindowMocks = (): void => { + window.HTMLElement.prototype.scrollIntoView = vi.fn(); + + Object.defineProperty(window, 'matchMedia', { + writable: true, + value: vi.fn().mockImplementation((query) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); +};Then import and use it in your test file:
+import { setupWindowMocks } from 'test/utils/windowMocks'; + describe('Testing Chat Screen [User Portal]', () => { - window.HTMLElement.prototype.scrollIntoView = vi.fn(); - Object.defineProperty(window, 'matchMedia', {...}); + setupWindowMocks();
Line range hint
1507-1521: Add assertions to verify component rendering.The test currently only checks if the component renders without errors. Consider adding specific assertions to verify key elements are present.
test('Screen should be rendered properly', async () => { render( <MockedProvider addTypename={false} mocks={mock}> <BrowserRouter> <Provider store={store}> <I18nextProvider i18n={i18nForTest}> <Chat /> </I18nextProvider> </Provider> </BrowserRouter> </MockedProvider>, ); await wait(); + + // Verify key elements are present + expect(screen.getByText('Messages')).toBeInTheDocument(); + expect(screen.getByTestId('contactCardContainer')).toBeInTheDocument(); + expect(screen.getByTestId('dropdown')).toBeInTheDocument(); });
Line range hint
1522-1597: Add test coverage for chat creation functionality.Current tests only verify UI interactions (button clicks, modal visibility). Consider adding tests for:
- Successful chat creation
- Error handling
- Form validation
Example addition:
test('successfully creates a direct chat', async () => { render(/* ... */); // Open new chat modal fireEvent.click(await screen.findByTestId('dropdown')); fireEvent.click(await screen.findByTestId('newDirectChat')); // Fill form fireEvent.change(screen.getByTestId('userSelect'), { target: { value: 'Test User' } }); // Submit and verify fireEvent.click(screen.getByTestId('submitBtn')); // Wait for success message expect(await screen.findByText('Chat created successfully')).toBeInTheDocument(); // Verify new chat appears in list expect(await screen.findByText('Test User')).toBeInTheDocument(); });
Line range hint
1605-1628: Remove debug statement and improve error handling.The test includes an unnecessary
screen.debug()and could handle errors better.test('sidebar', async () => { render( <MockedProvider addTypename={false} mocks={mock}> <BrowserRouter> <Provider store={store}> <I18nextProvider i18n={i18nForTest}> <Chat /> </I18nextProvider> </Provider> </BrowserRouter> </MockedProvider>, ); - screen.debug(); await waitFor(async () => { const closeMenuBtn = await screen.findByTestId('closeMenu'); expect(closeMenuBtn).toBeInTheDocument(); - if (closeMenuBtn) { - closeMenuBtn.click(); - } else { - throw new Error('Close menu button not found'); - } + fireEvent.click(closeMenuBtn); + expect(screen.queryByTestId('menu')).not.toBeInTheDocument(); }); });
Line range hint
1629-1654: Enhance responsive behavior testing.The responsive test should verify more aspects of the mobile view behavior.
test('Testing sidebar when the screen size is less than or equal to 820px', async () => { resizeWindow(800); render(/* ... */); await waitFor(() => { expect(screen.getByText('My Organizations')).toBeInTheDocument(); expect(screen.getByText('Talawa User Portal')).toBeInTheDocument(); }); const openMenuBtn = await screen.findByTestId('openMenu'); expect(openMenuBtn).toBeInTheDocument(); fireEvent.click(openMenuBtn); const closeMenuBtn = await screen.findByTestId('closeMenu'); expect(closeMenuBtn).toBeInTheDocument(); + + // Verify menu is visible + expect(screen.getByTestId('menu')).toBeInTheDocument(); + + // Verify menu closes + fireEvent.click(closeMenuBtn); + expect(screen.queryByTestId('menu')).not.toBeInTheDocument(); + + // Verify content adjusts to mobile view + expect(screen.getByTestId('chatContainer')).toHaveStyle({ width: '100%' }); });
Line range hint
1-1654: Overall successful migration to Vitest with room for improvement.The migration from Jest to Vitest has been completed successfully. Consider these improvements for better test quality:
- Extract common test utilities
- Add more comprehensive assertions
- Improve error handling
- Enhance responsive testing
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)src/screens/UserPortal/Chat/Chat.spec.tsx(7 hunks)vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (2)
vitest.config.ts (1)
5-5: LGTM! SVG plugin configuration is correct.
The addition of vite-plugin-svgr and its configuration is properly implemented for handling SVG imports in the test environment.
Also applies to: 10-10
src/screens/UserPortal/Chat/Chat.spec.tsx (1)
1489-1499: LGTM! Good practice consolidating mock data.
Consolidating mock data outside of tests improves reusability and maintainability.
Signed-off-by: NishantSinghhhhh <[email protected]>
|
@palisadoes , I have to add 1 plugin named vite-plugin-svg because vite was not able to read the svgs and tests were failing |
|
@NishantSinghhhhh Please fix the failing tests. |
|
1 → PR Workflow / Checks if sensitive files have been changed without authorization: 2 → PR Workflow / Test Application (pull_request): |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/screens/UserPortal/Chat/Chat.spec.tsx (4)
25-92: Excellent documentation structure!The comprehensive documentation greatly improves maintainability. Consider adding a section about test coverage metrics to help track testing completeness.
94-105: Consider enhancing type safety in helper functions.While the implementation is correct, consider adding runtime validation for the width parameter in resizeWindow.
const resizeWindow = (width: number): void => { + if (width <= 0) { + throw new Error('Window width must be positive'); + } window.innerWidth = width; fireEvent(window, new Event('resize')); };
1558-1568: Consider adding type definitions for mock data.While consolidating mocks outside tests improves reusability, adding type definitions would enhance maintainability and catch potential issues early.
interface MockData { request: { query: any; variables: Record<string, any>; }; result: { data: Record<string, any>; }; } const mock: MockData[] = [ ...USER_JOINED_ORG_MOCK, ...GROUP_CHAT_BY_ID_QUERY_MOCK, // ... rest of the mocks ];
Line range hint
1576-1723: Clean up debugging code and improve test reliability.Several improvements needed:
- Remove
screen.debug()call in the sidebar test- Consider extracting magic numbers (like wait times) to named constants
- Add retry logic for potentially flaky tests
- screen.debug(); await waitFor(async () => {// Add at the top of the file const TEST_CONSTANTS = { ANIMATION_WAIT: 100, MOBILE_BREAKPOINT: 820, MOBILE_TEST_WIDTH: 800 } as const; // Update test test('Testing sidebar when the screen size is less than or equal to 820px', async () => { resizeWindow(TEST_CONSTANTS.MOBILE_TEST_WIDTH); // ... rest of the test });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/screens/UserPortal/Chat/Chat.spec.tsx(7 hunks)
🔇 Additional comments (2)
src/screens/UserPortal/Chat/Chat.spec.tsx (2)
1-24: LGTM! Import statements are well-organized.
The migration to Vitest is properly reflected in the imports, with all necessary testing utilities included.
Line range hint 1542-1723: Verify test coverage after migration.
Ensure that the migration to Vitest hasn't reduced test coverage.
✅ Verification successful
Based on the gathered information, I can now provide the final response:
Test coverage is sufficient after Vitest migration
The test suite adequately covers the critical paths:
- Window resize behavior is properly tested with the
resizeWindowhelper function and specific mobile view test case - Chat data handling is covered through comprehensive mock data (
CHATS_LIST_MOCK) - Both conditional branches identified (
window.innerWidth <= 820andchatsListDatacheck) are exercised in tests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any untested code paths in the Chat component
# Look for potential untested paths in the Chat component
ast-grep --pattern 'if ($_) {
$$$
}' src/screens/UserPortal/Chat/Chat.tsx
# Look for event handlers that might need testing
ast-grep --pattern 'onClick={$_}' src/screens/UserPortal/Chat/Chat.tsx
Length of output: 545
Script:
#!/bin/bash
# Let's check the test coverage by examining if the identified conditions are tested
# Check test file for window resize tests
rg -A 5 "window\.innerWidth" src/screens/UserPortal/Chat/Chat.spec.tsx
# Check test file for chatsListData handling
rg -A 5 "chatsListData" src/screens/UserPortal/Chat/Chat.spec.tsx
# Check for any mock data related to chat list
rg "CHATS_LIST_MOCK" src/screens/UserPortal/Chat/Chat.spec.tsx
Length of output: 417
Script:
#!/bin/bash
# Let's examine the test coverage more thoroughly
# Check the actual test implementation for window resize
rg -B 5 -A 10 "resizeWindow" src/screens/UserPortal/Chat/Chat.spec.tsx
# Check the mock data structure for chats
rg -A 10 "const CHATS_LIST_MOCK = \[" --type ts
# Look for any assertions related to chat list rendering
rg "expect.*chat.*toBeInTheDocument" src/screens/UserPortal/Chat/Chat.spec.tsx
Length of output: 4602
|
Please fix the failing |
Signed-off-by: NishantSinghhhhh <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/screens/UserPortal/Chat/Chat.spec.tsx (3)
1572-1581: Consider enhancing test cleanup.While the current cleanup is good, consider adding these additional cleanup steps for more robust tests:
beforeEach(() => { setItem('userId', '1'); vi.clearAllMocks(); localStorage.clear(); vi.resetModules(); }); afterEach(() => { localStorage.clear(); + // Reset any timers + vi.clearAllTimers(); + // Reset any pending promises + vi.clearAllMocks(); + // Reset document body + document.body.innerHTML = ''; });
Line range hint
1583-1597: Add assertions to the render test.The screen render test should verify the presence of key UI elements.
test('Screen should be rendered properly', async () => { render( <MockedProvider addTypename={false} mocks={mock}> <BrowserRouter> <Provider store={store}> <I18nextProvider i18n={i18nForTest}> <Chat /> </I18nextProvider> </Provider> </BrowserRouter> </MockedProvider>, ); await wait(); + // Verify key UI elements + expect(screen.getByText('Messages')).toBeInTheDocument(); + expect(screen.getByTestId('contactCardContainer')).toBeInTheDocument(); + expect(screen.getByTestId('dropdown')).toBeInTheDocument(); });
Line range hint
1705-1730: Enhance mobile view test assertions.The mobile view test should verify more UI states and interactions.
test('Testing sidebar when the screen size is less than or equal to 820px', async () => { resizeWindow(800); render( <MockedProvider addTypename={false} mocks={mock}> <BrowserRouter> <Provider store={store}> <I18nextProvider i18n={i18nForTest}> <Chat /> </I18nextProvider> </Provider> </BrowserRouter> </MockedProvider>, ); await waitFor(() => { expect(screen.getByText('My Organizations')).toBeInTheDocument(); expect(screen.getByText('Talawa User Portal')).toBeInTheDocument(); }); const openMenuBtn = await screen.findByTestId('openMenu'); expect(openMenuBtn).toBeInTheDocument(); fireEvent.click(openMenuBtn); const closeMenuBtn = await screen.findByTestId('closeMenu'); expect(closeMenuBtn).toBeInTheDocument(); + + // Verify sidebar state after opening + expect(screen.getByTestId('sidebar')).toHaveClass('open'); + + // Close sidebar + fireEvent.click(closeMenuBtn); + + // Verify sidebar state after closing + expect(screen.getByTestId('sidebar')).not.toHaveClass('open'); + expect(openMenuBtn).toBeInTheDocument(); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/UserPortal/Chat/Chat.spec.tsx(7 hunks)vitest.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- vitest.config.ts
🔇 Additional comments (2)
src/screens/UserPortal/Chat/Chat.spec.tsx (2)
1548-1557: LGTM: Proper mock implementation of window.matchMedia.
The mock implementation correctly uses Vitest's vi.fn() and includes all necessary event listeners for testing media queries.
98-109: LGTM: Well-implemented helper functions.
Both resizeWindow and wait helpers are properly implemented:
resizeWindowcorrectly simulates window resizingwaitproperly wraps setTimeout in act for handling async operations
Screencast.from.2024-12-14.02-09-13.webmI've made several changes to ensure that the tests pass, but I haven't modified any Jest test files myself. Despite this, the [PR Workflow / Test Application (pull_request)] test is still failing. This could possibly be due to a recent PR in the develop-postgres branch, though I'm not entirely sure. Can you help me resolve this issue?" |
PR Title: Refactor Chat Component Tests: Migrate from Jest to Vitest
Issue Number:
Fixes: #2572
Did you add tests for your changes?
Yes
Snapshots/Videos:
Screencast.from.2024-12-13.00-58-58.webm
Summary:
This PR refactors the test suite for the Chat.tsx component by migrating from Jest to Vitest, in alignment with the project's updated testing framework. The following changes were made:
Updated the testing configuration to be compatible with Vitest.
Refactored all test files related to Chat.tsx to use Vitest's syntax and features.
Consolidated mock definitions to reduce redundancy across test cases for better maintainability.
#Does this PR introduce a breaking change?
Yes
This PR introduces a breaking change as it requires installing the vite-plugin-svgr package using the command:
Reason:
The vite-plugin-svgr plugin is necessary to handle SVG imports in the Vitest environment. Jest previously handled these imports using a different configuration. This change ensures compatibility with Vitest, enabling seamless handling of SVG files as React components within tests and during development.
This breaking change requires developers to update their dependencies and ensure vite-plugin-svgr is installed when migrating to Vitest.
Summary by CodeRabbit
New Features
vite-plugin-svgrdependency to enhance functionality.Bug Fixes
Chat.spec.tsxfile for better maintainability and robustness.Documentation