Upgrade package @testing-library/user-event from 12.8.3 to 14.5.2 #2605
Conversation
WalkthroughThe pull request includes significant updates to 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (48)
src/components/EventManagement/EventAttendance/EventStatistics.test.tsx (2)
Line range hint
207-269: Maintain consistency in userEvent usage across all test casesOther test cases in the file are still using the old userEvent.click() pattern without setup(). For consistency and to prevent potential timing issues, consider updating these cases to use the new pattern.
Here's an example of how to update the export functionality tests:
test('handles data demographics export functionality', async () => { const mockExportToCSV = jest.fn(); (exportToCSV as jest.Mock).mockImplementation(mockExportToCSV); + const user = userEvent.setup(); render( <MockedProvider mocks={mocks}> <AttendanceStatisticsModal show={true} handleClose={() => {}} statistics={mockStatistics} memberData={mockMemberData} t={(key) => key} /> </MockedProvider>, ); await waitFor(() => { expect( screen.getByRole('button', { name: 'Export Data' }), ).toBeInTheDocument(); }); - await act(async () => { const exportButton = screen.getByRole('button', { name: 'Export Data' }); - await userEvent.click(exportButton); - }); + const exportButton = screen.getByRole('button', { name: 'Export Data' }); + await user.click(exportButton); - await act(async () => { const demographicsExport = screen.getByTestId('demographics-export'); - await userEvent.click(demographicsExport); - }); + const demographicsExport = screen.getByTestId('demographics-export'); + await user.click(demographicsExport); expect(mockExportToCSV).toHaveBeenCalled(); });Apply similar updates to the remaining test cases that use userEvent.
Line range hint
207-269: Enhance test assertions for better coverageWhile the tests cover the basic functionality, consider adding more specific assertions to verify:
- The format and content of exported data
- The state changes after each user interaction
- The correct parameters passed to mockExportToCSV
Example enhancement:
test('handles data demographics export functionality', async () => { const mockExportToCSV = jest.fn(); (exportToCSV as jest.Mock).mockImplementation(mockExportToCSV); const user = userEvent.setup(); render(/* ... */); const exportButton = screen.getByRole('button', { name: 'Export Data' }); await user.click(exportButton); const demographicsExport = screen.getByTestId('demographics-export'); await user.click(demographicsExport); expect(mockExportToCSV).toHaveBeenCalled(); + expect(mockExportToCSV).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + gender: expect.any(String), + count: expect.any(Number) + }) + ]), + 'demographics_export.csv' + ); });src/screens/OrgList/OrgList.test.tsx (3)
313-314: Maintain consistency in event handlingFor consistency with the rest of the test, replace
fireEvent.submitwithuser.clicksince we're already using the userEvent API for all other interactions.await user.upload(displayImage, formData.image); -fireEvent.submit(screen.getByTestId(/submitOrganizationForm/i)); +await user.click(screen.getByTestId(/submitOrganizationForm/i));
Line range hint
419-432: Remove commented out codeThe commented timing-related code can be safely removed as the
waitForimplementation properly handles the asynchronous behavior.await user.click(screen.getByTestId(/submitOrganizationForm/i)); -// await act(async () => { -// await new Promise((resolve) => setTimeout(resolve, 1000)); -// }); await waitFor(() => expect( screen.queryByText(/Congratulation the Organization is created/i), ).toBeInTheDocument(), ); await waitFor(() => { screen.findByTestId(/pluginNotificationHeader/i); }); -// userEvent.click(screen.getByTestId(/enableEverythingForm/i)); await user.click(screen.getByTestId(/enableEverythingForm/i));
Line range hint
238-481: Overall implementation of @testing-library/user-event upgrade looks goodThe upgrade from v12.8.3 to v14.5.2 has been implemented correctly throughout the test file. The changes properly utilize the new
userEvent.setup()API and maintain consistent async/await patterns for user interactions. The tests remain functionally equivalent while becoming more reliable through proper handling of asynchronous operations.Consider creating a test utility function to handle the common form-filling operations, as this pattern is repeated in multiple test cases. This would reduce code duplication and make the tests more maintainable.
src/screens/MemberDetail/MemberDetail.test.tsx (2)
202-202: Consider adding save operation verificationWhile the click event is properly handled, consider adding expectations to verify the success of the save operation, such as checking for success toast messages or API call completion.
await user.click(screen.getByText(/Save Changes/i)); +await waitFor(() => { + expect(toast.success).toHaveBeenCalledWith(translations.successfullySaved); +});
262-266: Consider structuring the reset test more comprehensivelyWhile the test covers the reset functionality, consider structuring it to:
- Store initial values
- Modify all fields
- Reset
- Compare with initial values
const user = userEvent.setup(); +const initialValues = { + address: '', + state: '', + firstName: 'Aditya', + lastName: 'Agarwal' +}; + +// Modify all fields await user.type(screen.getByPlaceholderText(/Address/i), 'random'); await user.type(screen.getByPlaceholderText(/State/i), 'random'); +await user.type(screen.getByPlaceholderText(/First Name/i), 'Changed'); +await user.type(screen.getByPlaceholderText(/Last Name/i), 'Changed'); await user.click(screen.getByTestId('resetChangesBtn')); await wait(); -expect(screen.getByPlaceholderText(/First Name/i)).toHaveValue('Aditya'); -expect(screen.getByPlaceholderText(/Last Name/i)).toHaveValue('Agarwal'); + +// Verify reset to initial values +Object.entries(initialValues).forEach(([field, value]) => { + expect(screen.getByPlaceholderText(new RegExp(field, 'i'))).toHaveValue(value); +});src/components/EventManagement/EventAgendaItems/EventAgendaItems.test.tsx (2)
Line range hint
182-209: Update remaining userEvent calls to use setup() pattern.The test case "creates new agenda item" still uses the old userEvent pattern. For consistency and compatibility with @testing-library/user-event v14.x.x, update these calls to use the setup pattern:
- userEvent.click(screen.getByTestId('createAgendaItemBtn')); + const user = userEvent.setup(); + await user.click(screen.getByTestId('createAgendaItemBtn')); await waitFor(() => { expect( screen.getByTestId('createAgendaItemModalCloseBtn'), ).toBeInTheDocument(); }); - userEvent.type( + await user.type( screen.getByPlaceholderText(translations.enterTitle), formData.title, ); - userEvent.type( + await user.type( screen.getByPlaceholderText(translations.enterDescription), formData.description, ); - userEvent.type( + await user.type( screen.getByPlaceholderText(translations.enterDuration), formData.duration, ); const categorySelect = screen.getByTestId('categorySelect'); - userEvent.click(categorySelect); + await user.click(categorySelect);
Based on the complete test file content, I can now provide a final response:
The commented toast assertion should be uncommented and implemented
The test file shows proper setup for toast testing with the mock:
jest.mock('react-toastify', () => ({ toast: { success: jest.fn(), error: jest.fn(), }, }));The commented assertion
expect(toast.success).toBeCalledWith(translations.agendaItemCreated)should be uncommented because:
- The test is verifying agenda item creation functionality
- Toast notifications are consistently used across the codebase for user feedback
- The test has all the required setup including the toast mock
- The translations object is properly imported and contains the required message
🔗 Analysis chain
Line range hint
211-213: Verify the commented toast assertion.The commented toast assertion suggests incomplete test coverage. Consider either:
- Removing the commented code if it's no longer needed
- Implementing proper toast assertions if they are required
Based on the search results, I can see that toast notifications are widely used throughout the test files in the codebase for both success and error cases. The commented toast assertion in the review comment appears to be part of a test case for agenda item creation, and there are several other test files that demonstrate proper toast assertion patterns.
Let me verify the specific file content to understand the full context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for toast-related test patterns in other test files rg "toast\.(success|error)" -g "*.test.{ts,tsx}" -A 5 -B 5Length of output: 70653
Script:
#!/bin/bash # Get the full content of the test file cat src/components/EventManagement/EventAgendaItems/EventAgendaItems.test.tsxLength of output: 6283
src/screens/OrgPost/OrgPost.test.tsx (4)
Line range hint
398-409: Use userEvent consistently instead of mixing with fireEventFor better simulation of real user interactions and consistency, replace fireEvent with userEvent throughout the test.
await user.click(screen.getByTestId('createPostModalBtn')); const postTitleInput = screen.getByTestId('modalTitle'); - fireEvent.change(postTitleInput, { target: { value: 'Test Post' } }); + await user.type(postTitleInput, 'Test Post'); const postInfoTextarea = screen.getByTestId('modalinfo'); - fireEvent.change(postInfoTextarea, { - target: { value: 'Test post information' }, - }); + await user.type(postInfoTextarea, 'Test post information'); const createPostBtn = screen.getByTestId('createPostBtn'); await user.click(createPostBtn);
Line range hint
427-451: Standardize async event handlingThe test mixes different event handling approaches which could lead to timing issues. Standardize on userEvent for all interactions.
await user.click(screen.getByTestId('createPostModalBtn')); const postTitleInput = screen.getByTestId('modalTitle'); - fireEvent.change(postTitleInput, { target: { value: 'Test Post' } }); + await user.type(postTitleInput, 'Test Post'); const postInfoTextarea = screen.getByTestId('modalinfo'); - fireEvent.change(postInfoTextarea, { - target: { value: 'Test post information' }, - }); + await user.type(postInfoTextarea, 'Test post information');
Line range hint
628-677: Improve test coverage for sorting functionalityThe current test doesn't effectively verify the sorting behavior:
- Mocked data is defined but not used in assertions
- Hard-coded string comparisons make the test brittle
- The actual sorting interaction is not tested
Consider refactoring the test to:
- Use the mocked data in assertions
- Test the actual sorting interaction
- Verify the order of posts based on their pinned status
test('Sorting posts by pinned status', async () => { const mockedPosts = [ { _id: '1', title: 'Post 1', pinned: true }, { _id: '2', title: 'Post 2', pinned: false }, { _id: '3', title: 'Post 3', pinned: true }, { _id: '4', title: 'Post 4', pinned: false } ]; render(/* ... */); await wait(); + const user = userEvent.setup(); + + // Test initial order const initialPosts = screen.getAllByTestId('post-item'); - expect(sortedPosts[0]).toHaveTextContent('postoneThis is the first po... Aditya Shelke'); + expect(initialPosts.map(post => post.dataset.pinned)).toEqual(['true', 'true', 'false', 'false']); + + // Test sorting interaction + await user.click(screen.getByTestId('sort')); + await user.click(screen.getByTestId('oldest')); + + const sortedPosts = screen.getAllByTestId('post-item'); + expect(sortedPosts.map(post => post.dataset.pinned)).toEqual(['false', 'false', 'true', 'true']); });
411-411: Adjust test timeout valuesMultiple tests use a 15000ms timeout which seems excessive for these operations. Consider reducing the timeout to a more reasonable value (e.g., 5000ms) unless there's a specific reason for the extended duration.
- }, 15000); + }, 5000);Also applies to: 456-456, 579-579
src/screens/ManageTag/ManageTag.test.tsx (5)
Line range hint
148-165: Simplify async queries for consistencyConsider simplifying the test by consistently using
findByqueries instead of mixingwaitFor:- await waitFor(() => { - expect(screen.getAllByTestId('unassignTagBtn')[0]).toBeInTheDocument(); - }); - userEvent.click(screen.getAllByTestId('unassignTagBtn')[0]); + const unassignButtons = await screen.findAllByTestId('unassignTagBtn'); + await user.click(unassignButtons[0]);This makes the test more readable and follows Testing Library's recommended practices.
Line range hint
426-455: Consider adding explicit timing controls for scroll testingThe infinite scroll test might be flaky due to timing issues. Consider adding explicit timing controls:
+ const SCROLL_DELAY = 500; // ms fireEvent.scroll(manageTagScrollableDiv, { target: { scrollY: manageTagScrollableDiv.scrollHeight }, }); + await new Promise((resolve) => setTimeout(resolve, SCROLL_DELAY));This helps ensure consistent behavior across different test environments.
Line range hint
457-473: Standardize async handling in unassignment testConsider standardizing the async handling pattern:
- await waitFor(() => { - expect(screen.getAllByTestId('unassignTagBtn')[0]).toBeInTheDocument(); - }); - userEvent.click(screen.getAllByTestId('unassignTagBtn')[0]); + const user = userEvent.setup(); + const unassignButtons = await screen.findAllByTestId('unassignTagBtn'); + await user.click(unassignButtons[0]);This makes the test more maintainable and follows a consistent pattern.
Line range hint
478-507: Update user interaction patterns for tag editingUpdate the user interactions to use the new userEvent.setup() pattern:
+ const user = userEvent.setup(); - await userEvent.clear(tagNameInput); - await userEvent.type(tagNameInput, 'tag 1 edited'); + await user.clear(tagNameInput); + await user.type(tagNameInput, 'tag 1 edited');This aligns with @testing-library/user-event v14's recommended practices.
Line range hint
510-531: LGTM! Consider consistent userEvent setupThe test correctly handles async operations and navigation verification. For consistency with other tests, consider using userEvent.setup() here as well.
src/components/UserPortal/StartPostModal/StartPostModal.test.tsx (1)
Line range hint
38-46: Consider using built-in waitFor instead of custom wait functionThe custom
waitfunction could be replaced with@testing-library/react's built-inwaitForutility, which is designed specifically for handling async operations in tests.-async function wait(ms = 100): Promise<void> { - await act(() => { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); - }); -} +import { waitFor } from '@testing-library/react';Then update the test calls:
-await wait(); +await waitFor(() => { + // Assert the component is ready + expect(screen.getByTestId('postInput')).toBeInTheDocument(); +});scripts/custom-test-env.js (1)
4-7: Fix JSDoc tag in documentation.The
@pdfmetag is not a standard JSDoc tag. Consider updating the documentation to use proper JSDoc syntax./** - * A custom environment to set the TextEncoder and TextDecoder variables, that is required by @pdfme during testing. + * A custom environment to set the TextEncoder and TextDecoder variables, that is required by the PDF library during testing. * Providing a polyfill to the environment for the same */🧰 Tools
🪛 eslint
[error] 5-5: tsdoc-undefined-tag: The TSDoc tag "@pdfme" is not defined in this configuration
(tsdoc/syntax)
src/components/Pagination/Pagination.test.tsx (1)
Line range hint
11-19: Reduce code duplication by consolidating props.The props object is defined twice with slight variations. Consider consolidating them:
+ const baseProps = { + count: 5, + page: 10, + rowsPerPage: 5, + onPageChange: (): number => { + return 10; + }, + }; - const props = { - count: 5, - page: 10, - rowsPerPage: 5, - onPageChange: (): number => { - return 10; - }, - }; describe('Testing Pagination component', () => { - const props = {...} // Remove this + const props = baseProps; // ... first test }); + const rtlProps = { + ...baseProps, + theme: { direction: 'rtl' }, + }; - const props = {...} // Remove thisAlso applies to: 33-42
src/components/ProfileDropdown/ProfileDropdown.test.tsx (2)
122-124: Consider moving userEvent.setup() to a beforeEach blockThe
userEvent.setup()is called multiple times in different test cases. Consider moving it to a beforeEach block to improve test efficiency and reduce duplication.+let user; + +beforeEach(() => { + user = userEvent.setup(); + setItem('FirstName', 'John'); + setItem('LastName', 'Doe'); + setItem( + 'UserImage', + 'https://api.dicebear.com/5.x/initials/svg?seed=John%20Doe', + ); + setItem('SuperAdmin', false); + setItem('AdminFor', []); + setItem('id', '123'); +}); test('logout functionality clears local storage and redirects to home', async () => { - const user = userEvent.setup(); await user.click(screen.getByTestId('togDrop')); await user.click(screen.getByTestId('logoutBtn')); });Also applies to: 138-140
Line range hint
47-54: Remove duplicate afterEach blockThere are two identical afterEach blocks. Remove one of them to avoid redundancy.
afterEach(() => { jest.clearAllMocks(); localStorage.clear(); }); -afterEach(() => { - jest.clearAllMocks(); - localStorage.clear(); -});src/components/UserPasswordUpdate/UserPasswordUpdate.test.tsx (2)
Line range hint
37-43: Modernize wait implementationThe current wait implementation using
act()can be simplified using modern testing practices.-async function wait(ms = 5): Promise<void> { - await act(() => { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); - }); +const wait = async (ms = 5): Promise<void> => { + return new Promise((resolve) => setTimeout(resolve, ms)); +};
Line range hint
71-82: Standardize userEvent usage across all testsSome tests are using the old userEvent methods while others use the new setup(). Standardize all tests to use the new approach.
+ const user = userEvent.setup(); - userEvent.type( + await user.type( screen.getByPlaceholderText(/Previous Password/i), formData.previousPassword, ); - userEvent.type( + await user.type( screen.getAllByPlaceholderText(/New Password/i)[0], formData.newPassword, ); - userEvent.type( + await user.type( screen.getByPlaceholderText(/Confirm New Password/i), formData.confirmNewPassword, );src/components/EventManagement/EventAttendance/EventAttendance.test.tsx (2)
Line range hint
89-95: Standardize event handling approachThe test file mixes
fireEvent, olduserEvent, and newuserEvent.setup(). Standardize to useuserEvent.setup()throughout for consistency.+ const user = userEvent.setup(); const searchInput = screen.getByTestId('searchByName'); - fireEvent.change(searchInput, { target: { value: 'Bruce' } }); + await user.type(searchInput, 'Bruce'); // Later in the file... const sortDropdown = screen.getByTestId('sort-dropdown'); - userEvent.click(sortDropdown); - userEvent.click(screen.getByText('Sort')); + await user.click(sortDropdown); + await user.click(screen.getByText('Sort'));Also applies to: 108-114
Line range hint
20-25: Simplify wait implementationThe current wait implementation using waitFor can be simplified.
-async function wait(): Promise<void> { - await waitFor(() => { - return Promise.resolve(); - }); +const wait = async (): Promise<void> => { + return Promise.resolve(); }src/components/EventRegistrantsModal/AddOnSpotAttendee.test.tsx (1)
Line range hint
171-173: Update userEvent implementation for consistencyThis test case still uses the old userEvent pattern. For consistency and reliability, update to use the new setup pattern:
- userEvent.type(screen.getByLabelText('First Name'), 'John'); - userEvent.type(screen.getByLabelText('Last Name'), 'Doe'); + const user = userEvent.setup(); + await user.type(screen.getByLabelText('First Name'), 'John'); + await user.type(screen.getByLabelText('Last Name'), 'Doe');src/components/UserPortal/UserNavbar/UserNavbar.test.tsx (1)
Line range hint
16-22: Consider replacing custom wait() function with modern alternativesThe custom
wait()function usingsetTimeoutcould be replaced with@testing-library/react's built-in utilities for better reliability:-async function wait(ms = 100): Promise<void> { - await act(() => { - return new Promise((resolve) => { - setTimeout(resolve, ms); - }); - }); -}Consider using either:
await waitFor(() => expect(...))for assertionsscreen.findBy*queries which are async by defaultsrc/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx (1)
Line range hint
61-69: Consider removing custom wait functionSimilar to other files, the custom
wait()function could be replaced with built-in testing library utilities:Consider using either:
await waitFor(() => expect(...))for assertionsscreen.findBy*queries for async element queriesjest.advanceTimersByTime()if you need to control timeouts in testssrc/components/OrgSettings/ActionItemCategories/CategoryModal.test.tsx (1)
Line range hint
69-73: Refactor fillFormAndSubmit helper to use the new userEvent APIThe helper function still uses the old userEvent API. Consider updating it to use the new setup() pattern for consistency.
-const fillFormAndSubmit = async ( +const fillFormAndSubmit = async ( name: string, isDisabled: boolean, ): Promise<void> => { + const user = userEvent.setup(); const nameInput = screen.getByLabelText('Name *'); const isDisabledSwitch = screen.getByTestId('isDisabledSwitch'); const submitBtn = screen.getByTestId('formSubmitButton'); - fireEvent.change(nameInput, { target: { value: name } }); + await user.type(nameInput, name); if (isDisabled) { - userEvent.click(isDisabledSwitch); + await user.click(isDisabledSwitch); } - userEvent.click(submitBtn); + await user.click(submitBtn); };src/screens/UserPortal/People/People.test.tsx (1)
Line range hint
231-235: Update remaining userEvent calls to use the new APIThese lines still use the old userEvent API. For consistency, they should be updated to use the setup() pattern.
- userEvent.click(screen.getByTestId('modeChangeBtn')); - await wait(); - userEvent.click(screen.getByTestId('modeBtn1')); + const user = userEvent.setup(); + await user.click(screen.getByTestId('modeChangeBtn')); + await user.click(screen.getByTestId('modeBtn1'));src/components/OrgSettings/AgendaItemCategories/OrganizationAgendaCategory.test.tsx (1)
187-187: Use consistent event triggering approachThe code mixes fireEvent.submit() with userEvent. For consistency, consider using userEvent for all interactions.
- fireEvent.submit(screen.getByTestId('createAgendaCategoryFormSubmitBtn')); + await user.click(screen.getByTestId('createAgendaCategoryFormSubmitBtn'));src/components/UserPortal/Register/Register.test.tsx (1)
Line range hint
147-157: Maintain consistency in userEvent usage patternsThese test cases still use the old userEvent pattern without setup(). For consistency, they should be updated to use the new pattern.
Example refactor for one of the test cases:
- userEvent.type(screen.getByTestId('passwordInput'), formData.password); - userEvent.type(screen.getByTestId('emailInput'), formData.email); - userEvent.click(screen.getByTestId('registerBtn')); + const user = userEvent.setup(); + await user.type(screen.getByTestId('passwordInput'), formData.password); + await user.type(screen.getByTestId('emailInput'), formData.email); + await user.click(screen.getByTestId('registerBtn'));Also applies to: 166-176, 187-197
src/components/UpdateSession/UpdateSession.test.tsx (1)
95-99: Consider using more realistic slider value calculations.The current implementation uses hardcoded clientX values (-999, 999) which might not accurately represent the slider's actual range. Consider calculating the clientX values based on the slider's dimensions and the desired values.
- fireEvent.mouseDown(slider, { - clientX: -999, - }); - fireEvent.mouseUp(slider, { clientX: -999 }); + const sliderRect = slider.getBoundingClientRect(); + const minX = sliderRect.left; + fireEvent.mouseDown(slider, { + clientX: minX, + }); + fireEvent.mouseUp(slider, { clientX: minX });Also applies to: 116-120, 136-140
src/screens/UserPortal/Donate/Donate.test.tsx (1)
307-309: LGTM! Consider extending this patternThe implementation correctly uses
userEvent.setup()with proper async/await handling. However, this pattern should be applied consistently across all tests in the file.Consider updating other tests in the file (e.g., 'Currency is switched to USD', 'For Donation functionality', etc.) to use the same pattern:
const user = userEvent.setup(); await user.click(...); await user.type(...);Also applies to: 333-335
src/screens/SubTags/SubTags.test.tsx (1)
285-286: LGTM! Consider optimizing user setupThe implementation correctly uses
userEvent.setup()with proper async/await handling. However, the setup is repeated multiple times.Consider declaring
userEvent.setup()once at the test suite level:describe('Organisation Tags Page', () => { let user; beforeEach(() => { user = userEvent.setup(); // ... other setup code }); // ... tests using the shared user instance });Also applies to: 291-291, 303-303, 308-308, 356-364
src/components/UserPortal/OrganizationNavbar/OrganizationNavbar.test.tsx (1)
237-238: LGTM! Correct implementation of userEvent v14The changes correctly implement the new userEvent patterns required by v14, using setup() and properly awaiting all user interactions.
Consider extracting the userEvent setup to a beforeEach block since it's used across multiple tests:
let user: ReturnType<typeof userEvent.setup>; beforeEach(() => { user = userEvent.setup(); });Also applies to: 259-260, 262-262, 290-291, 293-293, 315-316, 318-318, 340-341, 343-343, 365-366, 368-368
src/components/AgendaCategory/AgendaCategoryContainer.test.tsx (2)
255-256: Consider consolidating user interaction methodsThe test mixes fireEvent.change() with userEvent methods. For consistency and better simulation of real user interactions, consider using userEvent for all interactions.
Replace fireEvent.change() with userEvent methods:
- fireEvent.change(name, { target: { value: '' } }); - await user.type(name, formData.name); + await user.clear(name); + await user.type(name, formData.name); - fireEvent.change(description, { target: { value: '' } }); - await user.type(description, formData.description); + await user.clear(description); + await user.type(description, formData.description);Also applies to: 262-262, 265-265, 270-270
301-302: Maintain consistent interaction patternsSimilar to the previous test, this section mixes fireEvent with userEvent methods.
Apply the same pattern of using userEvent methods consistently:
- fireEvent.change(nameInput, { target: { value: '' } }); - fireEvent.change(descriptionInput, { target: { value: '' } }); - await user.type(nameInput, formData.name); - await user.type(descriptionInput, formData.description); + await user.clear(nameInput); + await user.clear(descriptionInput); + await user.type(nameInput, formData.name); + await user.type(descriptionInput, formData.description);Also applies to: 310-311, 316-316
src/screens/UserPortal/Events/Events.test.tsx (1)
Line range hint
406-409: Clean up commented test code.The commented code should either be:
- Restored using the new userEvent.setup() pattern
- Removed if no longer needed
If you want to restore it, here's the updated syntax:
await wait(); const user = userEvent.setup(); await user.click(screen.getByTestId('modeChangeBtn')); await user.click(screen.getByTestId('modeBtn1'));src/screens/BlockUser/BlockUser.test.tsx (1)
448-452: Consider consolidating sequential user actionsMultiple sequential user actions could be consolidated into a single test block for better readability.
- await user.click(screen.getByTestId('nameFilter')); - await user.click(screen.getByTestId('searchByFirstName')); - const firstNameInput = screen.getByPlaceholderText(/Search by First Name/i); - await user.type(firstNameInput, 'john{enter}'); + const user = userEvent.setup(); + const firstNameInput = screen.getByPlaceholderText(/Search by First Name/i); + await Promise.all([ + user.click(screen.getByTestId('nameFilter')), + user.click(screen.getByTestId('searchByFirstName')), + user.type(firstNameInput, 'john{enter}') + ]);src/components/RecurrenceOptions/RecurrenceOptions.test.tsx (1)
247-249: Consider consolidating userEvent setupMultiple userEvent.setup() calls could be consolidated at the test level for better maintainability.
+ let user; + beforeEach(async () => { + user = userEvent.setup(); + }); - const user = userEvent.setup();Also applies to: 268-274, 280-282
src/components/Advertisements/Advertisements.test.tsx (1)
Line range hint
384-410: Consider using userEvent.setup() for consistencyThe test is using
fireEventfor user interactions while other files in the codebase are usinguserEvent.setup(). For consistency and better simulation of real user interactions, consider usinguserEvent.setup().Apply this pattern:
- fireEvent.click(screen.getByText('Create Advertisement')); - fireEvent.change(screen.getByLabelText('Enter name of Advertisement'), { - target: { value: 'Cookie Shop' }, - }); + const user = userEvent.setup(); + await user.click(screen.getByText('Create Advertisement')); + await user.type(screen.getByLabelText('Enter name of Advertisement'), 'Cookie Shop');src/screens/OrganizationPeople/OrganizationPeople.test.tsx (1)
Line range hint
1070-1088: Replace remaining fireEvent.change with userEvent.typeSome test cases still use
fireEvent.changefor input interactions. For consistency and better simulation of real user interactions, these should be replaced withuserEvent.type.Replace the fireEvent.change calls with userEvent.type:
- fireEvent.change(screen.getByTestId('firstNameInput'), { - target: { value: 'Disha' }, - }); + await user.type(screen.getByTestId('firstNameInput'), 'Disha');Also applies to: 1155-1161, 1215-1221
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)
Line range hint
1452-1493: Upgrade remaining fireEvent usage to userEventThe second test case still uses
fireEventfor interactions. For consistency and better simulation of real user interactions, these should be upgraded to useuserEvent.Replace the fireEvent calls with userEvent:
- fireEvent.click(dropdown); + await user.click(dropdown); - fireEvent.click(newDirectChatBtn); + await user.click(newDirectChatBtn); - fireEvent.click(addBtn[0]); + await user.click(addBtn[0]); - fireEvent.click(closeButton); + await user.click(closeButton);src/screens/LoginPage/LoginPage.test.tsx (1)
631-632: Well-structured migration to userEvent.setup().The changes correctly implement the new userEvent patterns from @testing-library/user-event v14. The async/await usage ensures proper handling of user interactions.
Consider these minor improvements:
- Extract user setup to a beforeEach block to reduce duplication:
describe('Testing Login Page Screen', () => { let user: ReturnType<typeof userEvent.setup>; beforeEach(() => { user = userEvent.setup(); }); // tests can now directly use the user instance });
- Consider grouping related tests using nested describe blocks for better organization:
describe('password visibility toggle', () => { // password-related tests }); describe('password validation', () => { // validation-related tests });Also applies to: 635-635, 663-664, 667-667, 695-696, 699-699, 740-741, 743-743, 770-771, 773-773, 800-801, 805-805, 830-831, 837-837, 861-862
src/components/OrgPostCard/OrgPostCard.test.tsx (1)
146-147: Successfully migrated to @testing-library/user-event v14 APIThe changes correctly implement the new async user-event API, improving test reliability by properly handling asynchronous user interactions.
Consider reducing setup duplication
The pattern
const user = userEvent.setup()is repeated in many test cases. Consider extracting it to a beforeEach block or a test helper function.describe('Testing Organization Post Card', () => { + let user; + + beforeEach(() => { + user = userEvent.setup(); + }); // ... existing test cases using the shared user instanceAlso applies to: 173-174, 192-195, 227-229, 231-231, 306-308, 310-310, 376-379, 409-412, 427-429, 431-431, 464-466, 468-468, 482-484, 500-503, 540-541, 552-553, 555-555, 559-559, 579-580, 582-582, 586-586, 608-610, 612-612, 643-645, 647-647, 683-686, 709-712, 736-737, 739-739, 741-741, 744-745, 755-756, 758-758
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (62)
package.json(2 hunks)scripts/custom-test-env.js(1 hunks)src/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx(3 hunks)src/components/AddOn/core/AddOnStore/AddOnStore.test.tsx(2 hunks)src/components/AddPeopleToTag/AddPeopleToTag.test.tsx(1 hunks)src/components/Advertisements/Advertisements.test.tsx(2 hunks)src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.test.tsx(1 hunks)src/components/AgendaCategory/AgendaCategoryContainer.test.tsx(2 hunks)src/components/AgendaItems/AgendaItemsContainer.test.tsx(14 hunks)src/components/DynamicDropDown/DynamicDropDown.test.tsx(3 hunks)src/components/EventListCard/EventListCard.test.tsx(38 hunks)src/components/EventManagement/EventAgendaItems/EventAgendaItems.test.tsx(3 hunks)src/components/EventManagement/EventAttendance/EventAttendance.test.tsx(1 hunks)src/components/EventManagement/EventAttendance/EventStatistics.test.tsx(1 hunks)src/components/EventRegistrantsModal/AddOnSpotAttendee.test.tsx(2 hunks)src/components/LeftDrawer/LeftDrawer.test.tsx(2 hunks)src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx(1 hunks)src/components/OrgPostCard/OrgPostCard.test.tsx(28 hunks)src/components/OrgSettings/ActionItemCategories/CategoryModal.test.tsx(1 hunks)src/components/OrgSettings/ActionItemCategories/OrgActionItemCategories.test.tsx(4 hunks)src/components/OrgSettings/AgendaItemCategories/AgendaCategoryUpdateModal.test.tsx(2 hunks)src/components/OrgSettings/AgendaItemCategories/OrganizationAgendaCategory.test.tsx(4 hunks)src/components/OrgSettings/General/OrgUpdate/OrgUpdate.test.tsx(1 hunks)src/components/Pagination/Pagination.test.tsx(3 hunks)src/components/ProfileDropdown/ProfileDropdown.test.tsx(3 hunks)src/components/RecurrenceOptions/CustomRecurrence.test.tsx(17 hunks)src/components/RecurrenceOptions/RecurrenceOptions.test.tsx(10 hunks)src/components/UpdateSession/UpdateSession.test.tsx(7 hunks)src/components/UserPasswordUpdate/UserPasswordUpdate.test.tsx(2 hunks)src/components/UserPortal/ChatRoom/ChatRoom.tsx(1 hunks)src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx(3 hunks)src/components/UserPortal/OrganizationNavbar/OrganizationNavbar.test.tsx(6 hunks)src/components/UserPortal/PostCard/PostCard.test.tsx(9 hunks)src/components/UserPortal/Register/Register.test.tsx(4 hunks)src/components/UserPortal/StartPostModal/StartPostModal.test.tsx(2 hunks)src/components/UserPortal/UserNavbar/UserNavbar.test.tsx(7 hunks)src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.test.tsx(1 hunks)src/screens/BlockUser/BlockUser.test.tsx(7 hunks)src/screens/CommunityProfile/CommunityProfile.test.tsx(2 hunks)src/screens/EventManagement/EventManagement.test.tsx(3 hunks)src/screens/EventVolunteers/VolunteerGroups/VolunteerGroupModal.test.tsx(5 hunks)src/screens/ForgotPassword/ForgotPassword.test.tsx(9 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.test.tsx(1 hunks)src/screens/LoginPage/LoginPage.test.tsx(8 hunks)src/screens/ManageTag/ManageTag.test.tsx(20 hunks)src/screens/MemberDetail/MemberDetail.test.tsx(3 hunks)src/screens/OrgList/OrgList.test.tsx(7 hunks)src/screens/OrgPost/OrgPost.test.tsx(16 hunks)src/screens/OrganizationActionItems/ItemModal.test.tsx(3 hunks)src/screens/OrganizationActionItems/OrganizationActionItems.test.tsx(7 hunks)src/screens/OrganizationEvents/OrganizationEvents.test.tsx(9 hunks)src/screens/OrganizationPeople/OrganizationPeople.test.tsx(20 hunks)src/screens/OrganizationTags/OrganizationTags.test.tsx(1 hunks)src/screens/SubTags/SubTags.test.tsx(3 hunks)src/screens/UserPortal/Donate/Donate.test.tsx(2 hunks)src/screens/UserPortal/Events/Events.test.tsx(7 hunks)src/screens/UserPortal/People/People.test.tsx(3 hunks)src/screens/UserPortal/Posts/Posts.test.tsx(4 hunks)src/screens/UserPortal/Settings/Settings.test.tsx(2 hunks)src/screens/UserPortal/Volunteer/Actions/Actions.test.tsx(3 hunks)src/screens/UserPortal/Volunteer/VolunteerManagement.test.tsx(1 hunks)src/setupTests.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/screens/FundCampaignPledge/FundCampaignPledge.test.tsx
👮 Files not reviewed due to content moderation or server errors (3)
- src/components/AddOn/core/AddOnStore/AddOnStore.test.tsx
- src/screens/ForgotPassword/ForgotPassword.test.tsx
- src/screens/OrganizationActionItems/OrganizationActionItems.test.tsx
🧰 Additional context used
📓 Learnings (5)
src/screens/OrganizationActionItems/OrganizationActionItems.test.tsx (1)
Learnt from: Chaitanya1672
PR: PalisadoesFoundation/talawa-admin#2049
File: src/screens/OrganizationActionItems/ActionItemUpdateModal.tsx:112-138
Timestamp: 2024-11-12T10:40:58.654Z
Learning: The `istanbul ignore next` comments in the `ActionItemUpdateModal.tsx` file were added as part of a commit that introduced tests for the `ActionItemUpdateModal` component. Removing these comments and writing tests to cover the previously ignored lines is recommended to ensure code integrity and improve test coverage.
src/screens/OrganizationActionItems/ItemModal.test.tsx (1)
Learnt from: Chaitanya1672
PR: PalisadoesFoundation/talawa-admin#2049
File: src/screens/OrganizationActionItems/ActionItemUpdateModal.tsx:112-138
Timestamp: 2024-11-12T10:40:58.654Z
Learning: The `istanbul ignore next` comments in the `ActionItemUpdateModal.tsx` file were added as part of a commit that introduced tests for the `ActionItemUpdateModal` component. Removing these comments and writing tests to cover the previously ignored lines is recommended to ensure code integrity and improve test coverage.
src/screens/OrganizationTags/OrganizationTags.test.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2398
File: src/screens/OrganizationTags/OrganizationTags.test.tsx:184-0
Timestamp: 2024-11-12T10:40:58.655Z
Learning: In the `talawa-admin` project, it's acceptable for test cases in `src/screens/OrganizationTags/OrganizationTags.test.tsx` to test multiple behaviors within a single test function without needing to split them into smaller, focused tests.
src/components/AddPeopleToTag/AddPeopleToTag.test.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2398
File: src/components/AddPeopleToTag/AddPeopleToTag.test.tsx:177-241
Timestamp: 2024-11-12T10:40:58.654Z
Learning: In `src/components/AddPeopleToTag/AddPeopleToTag.test.tsx`, prefer keeping test cases separate and more readable, even if it involves some duplication, instead of extracting common logic into helper functions.
src/components/OrgSettings/ActionItemCategories/OrgActionItemCategories.test.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2362
File: src/components/TagActions/TagActions.test.tsx:30-36
Timestamp: 2024-11-12T10:40:58.655Z
Learning: In our unit tests (e.g., `TagActions.test.tsx`), we use `setTimeout` with `act` to wait for asynchronous operations, as it provides a consistent wait time and is our standard practice.
🪛 eslint
scripts/custom-test-env.js
[error] 5-5: tsdoc-undefined-tag: The TSDoc tag "@pdfme" is not defined in this configuration
(tsdoc/syntax)
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
src/components/OrgSettings/ActionItemCategories/OrgActionItemCategories.test.tsx
[failure] 32-32:
Cannot find name 'act'.
[failure] 194-194:
Cannot find name 'act'.
🔇 Additional comments (60)
src/components/EventManagement/EventAttendance/EventStatistics.test.tsx (1)
175-185: LGTM! Proper implementation of userEvent.setup()
The changes correctly implement the new userEvent.setup() pattern, which is a key improvement in @testing-library/user-event v14. The async/await pattern is properly used with user interactions, which helps prevent timing issues.
src/screens/OrgList/OrgList.test.tsx (1)
453-455: LGTM! Proper implementation of userEvent.setup()
The changes to these test cases correctly implement the new userEvent API while maintaining the original test functionality. The async/await pattern is properly used for user interactions.
Also applies to: 479-481
src/screens/MemberDetail/MemberDetail.test.tsx (1)
159-195: LGTM! Correctly implemented userEvent.setup() pattern
The form interaction tests have been properly updated to use the new userEvent.setup() pattern with proper async/await handling. The clear->type sequence for form inputs is correctly implemented.
src/components/EventManagement/EventAgendaItems/EventAgendaItems.test.tsx (2)
40-44: LGTM! Improved type safety in console error handling.
The changes properly handle the message type by:
- Using
unknowntype for better type safety - Adding proper type narrowing before string operations
143-144: LGTM! Proper usage of userEvent.setup() pattern.
The changes correctly implement the new userEvent pattern required by @testing-library/user-event v14.x.x.
Also applies to: 151-156
src/screens/ManageTag/ManageTag.test.tsx (1)
108-112: LGTM! Proper async handling in component loading test
The changes correctly implement async rendering patterns using await act() and async queries with findByText.
src/components/UserPortal/StartPostModal/StartPostModal.test.tsx (3)
131-132: LGTM! Correctly implements the new userEvent pattern
The change properly adopts the new userEvent.setup() pattern introduced in @testing-library/user-event v14, which better handles async user interactions.
143-144: LGTM! Proper async user event handling
The implementation correctly uses the new userEvent setup pattern for typing interactions.
147-147: Consider addressing the commented-out success toast test
While the click event implementation is correct, there are commented-out expectations for the success toast. This might indicate unresolved timing issues with the post creation success case.
Let's check if similar patterns exist in other test files:
Consider handling the success case properly with one of these approaches:
// Option 1: Wait for the mutation response
await waitFor(() => {
expect(toast.success).toBeCalledWith(
'Your post is now visible in the feed.'
);
});
// Option 2: Mock the mutation response timing
const mockLink = new StaticMockLink([{
...MOCKS[0],
delay: 0 // Ensure predictable timing
}], true);Also applies to: 152-155
scripts/custom-test-env.js (1)
8-8: LGTM! Environment upgrade is correctly implemented.
The change from Environment to TestEnvironment aligns with the Jest 29.x upgrade and properly maintains the custom environment setup.
src/setupTests.ts (1)
11-12: LGTM! Simplified console mocking approach.
The change to use jest.fn() directly is a cleaner approach that:
- Simplifies error handling
- Aligns with Jest best practices
- Makes test failures easier to debug
src/screens/UserPortal/Volunteer/VolunteerManagement.test.tsx (1)
111-112: LGTM! Correct migration to userEvent.setup()
The changes correctly implement the new pattern for @testing-library/user-event v14, creating a single user instance and properly awaiting click events.
Also applies to: 118-118, 124-124
src/components/DynamicDropDown/DynamicDropDown.test.tsx (1)
45-46: LGTM! Proper implementation of userEvent.setup()
The changes correctly implement the new pattern for @testing-library/user-event v14. The mixed usage of fireEvent.keyDown for keyboard events is appropriate as it's better suited for low-level keyboard interaction testing.
Also applies to: 50-50, 83-84, 87-87, 122-123
src/components/EventRegistrantsModal/AddOnSpotAttendee.test.tsx (2)
104-107: LGTM! Correctly implemented userEvent.setup()
The implementation follows the best practices for @testing-library/user-event v14+, properly using async/await with the new setup pattern.
117-121: LGTM! Proper async handling of multiple user interactions
The sequential user interactions are correctly implemented with async/await pattern.
src/components/OrgSettings/AgendaItemCategories/AgendaCategoryUpdateModal.test.tsx (1)
78-79: LGTM! Correct implementation of async user interaction
The implementation properly uses userEvent.setup() with async/await pattern for the click event.
package.json (3)
150-151: LGTM! Proper Jest configuration for React 18
The upgrade to Jest 29.7.0 and addition of jest-environment-jsdom are appropriate for React 18 testing environment.
121-121: Verify compatibility with React version
The upgrade to @testing-library/user-event v14.5.2 is significant. Ensure compatibility with React 18 and other testing libraries.
✅ Verification successful
The previous results show @testing-library/react at v16, which is quite recent, but we need to verify the React version and actual usage patterns of userEvent to ensure compatibility. Let's analyze the codebase further to make an informed decision.
@testing-library/user-event v14.5.2 is compatible with the current setup
The upgrade is safe as the project uses React 18.3.1, and @testing-library/user-event v14.x is fully compatible with React 18. The related testing library (@testing-library/react v16) is also compatible with this setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential compatibility issues
rg -A 1 "@testing-library/(react|dom|user-event)" package.json
rg "react@\^?18" package.json
Length of output: 216
Script:
#!/bin/bash
# Check React version and related dependencies
rg "\"react\":" package.json
rg "\"react-dom\":" package.json
# Check for userEvent usage patterns to verify compatibility
rg -B2 -A2 "userEvent" --type ts --type tsx --type js --type jsx
Length of output: 206
Line range hint 121-151: Verify test suite stability after upgrades
The testing infrastructure has been significantly upgraded. Ensure all tests pass and there are no timing issues.
Consider adding integration tests that specifically verify timing-sensitive interactions to ensure stability with the new userEvent implementation.
✅ Verification successful
Based on the analysis of the test files, I can now provide a final response:
Testing infrastructure is stable with proper async handling
The codebase shows consistent and proper usage of React Testing Library's act() for managing async operations and state updates in tests. Key observations:
- Tests properly wrap async operations in
act() - Consistent usage of wait/debounce helpers for timing-sensitive tests
- No problematic patterns found in test assertions
The upgrade of @testing-library/user-event to v14.5.2 is safe as the codebase follows recommended testing practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential timing-related test issues
rg -B 2 -A 2 "act\(" "src/**/*.test.tsx"
rg -B 2 -A 2 "waitFor\(" "src/**/*.test.tsx"
Length of output: 205
Script:
#!/bin/bash
# Let's first find test files with correct extension patterns
fd -e test.tsx -e test.ts -e test.jsx -e test.js
# Then search for timing-related test patterns in all test files
rg -B 2 -A 2 "act\("
rg -B 2 -A 2 "waitFor\("
rg -B 2 -A 2 "@testing-library/user-event"
# Check if there are any existing test scripts in package.json
cat package.json | jq '.scripts | with_entries(select(.key | contains("test")))'
Length of output: 72067
src/components/UserPortal/UserNavbar/UserNavbar.test.tsx (1)
73-74: LGTM: Proper implementation of userEvent v14
The transition to userEvent.setup() is implemented correctly:
- Each test case properly initializes the user instance
- All interactions are properly awaited
- The implementation follows the recommended pattern for v14
Also applies to: 76-76, 98-99, 101-101, 123-124, 126-126, 148-150, 172-173, 175-175, 197-198, 218-220
src/components/AddOn/core/AddOnRegister/AddOnRegister.test.tsx (1)
118-125: LGTM: Proper implementation of userEvent v14
The transition to userEvent.setup() is implemented correctly with proper async/await usage for all user interactions.
Also applies to: 147-157, 179-189
src/components/OrgSettings/ActionItemCategories/CategoryModal.test.tsx (1)
125-126: LGTM! Correct usage of userEvent.setup()
The changes properly implement the new userEvent API by using setup() and awaiting the click action.
src/screens/UserPortal/People/People.test.tsx (1)
137-149: LGTM! Proper usage of act() for render calls
The render calls are correctly wrapped in act() to handle any effects that might occur during rendering.
Also applies to: 157-169, 182-194, 215-227
src/screens/UserPortal/Volunteer/Actions/Actions.test.tsx (1)
151-158: LGTM! Proper implementation of userEvent.setup()
The changes correctly implement the new userEvent.setup() pattern required by @testing-library/user-event v14. The async/await pattern is properly used to handle user interactions, which improves test reliability.
Also applies to: 172-180, 208-212, 219-223
src/components/UserPortal/Register/Register.test.tsx (1)
105-106: LGTM! Proper implementation of userEvent.setup()
The changes correctly implement the new userEvent.setup() pattern with proper async/await usage.
Also applies to: 126-127, 218-227, 249-263
src/components/OrgSettings/ActionItemCategories/OrgActionItemCategories.test.tsx (2)
194-205: LGTM! Proper implementation of userEvent.setup()
The changes correctly implement the new userEvent.setup() pattern with proper async/await usage and appropriate waiting mechanisms.
Also applies to: 233-237
🧰 Tools
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
[failure] 194-194:
Cannot find name 'act'.
220-223: LGTM! Improved assertion using queryByText
Good change to use queryByText instead of getByText for checking the presence of elements that might not exist.
src/screens/OrganizationTags/OrganizationTags.test.tsx (1)
278-292: LGTM! Good improvements to test reliability.
The changes correctly implement the new patterns from @testing-library/user-event v14:
- Using
userEvent.setup()for better async interaction handling - Using
fireEvent.submitfor form submissions, which more accurately simulates form behavior
src/components/AddPeopleToTag/AddPeopleToTag.test.tsx (1)
296-309: LGTM! Consistent implementation of async user interactions.
The changes properly implement the async patterns from @testing-library/user-event v14:
- Consistent use of
userEvent.setup() - All click events are properly awaited
- Maintains test coverage while improving reliability
src/components/UpdateSession/UpdateSession.test.tsx (3)
76-76: LGTM! Simplified test setup.
The console.warn spy implementation is cleaner and more straightforward.
158-168: LGTM! Good implementation of slider drag simulation.
The mouseDown/mouseMove/mouseUp sequence properly simulates a slider drag interaction.
222-222: LGTM! Appropriate use of fireEvent.submit.
Using fireEvent.submit is the correct approach for form submissions.
src/screens/CommunityProfile/CommunityProfile.test.tsx (1)
262-262: LGTM! Proper async handling of form submission.
The await pattern ensures reliable test execution by properly waiting for the click event to complete.
src/screens/EventVolunteers/VolunteerGroups/VolunteerGroupModal.test.tsx (2)
192-192: LGTM! Using fireEvent.submit is more appropriate for form submissions.
The change from userEvent.click to fireEvent.submit is a good practice as it directly triggers the form's submit event, which is more reliable than clicking a submit button.
242-242: LGTM! Consistent form submission handling across test cases.
The fireEvent.submit approach is consistently applied across all form submission test cases, improving test reliability.
Also applies to: 270-270, 300-300, 343-343
src/screens/UserPortal/Settings/Settings.test.tsx (1)
273-276: LGTM! Consistent implementation across test cases
The changes maintain consistency with the new userEvent patterns across test cases.
src/screens/UserPortal/Posts/Posts.test.tsx (3)
304-305: LGTM! Consistent with the new userEvent patterns
The implementation correctly uses the new userEvent.setup() pattern.
314-320: LGTM! Correct migration for file upload testing
The change from userEvent.upload to fireEvent.change is appropriate for file upload testing in the new version.
330-332: LGTM! Appropriate use of fireEvent for input changes
The change to fireEvent.change is suitable for simple input value updates.
src/components/UserPortal/ChatRoom/ChatRoom.tsx (1)
165-171: LGTM! Enhanced null safety with optional chaining
The addition of optional chaining (?.) improves code robustness by safely handling cases where chatMessageBelongsTo might be undefined.
Let's verify if similar patterns might need this improvement:
✅ Verification successful
Optional chaining is consistently applied where needed
Based on the search results, the optional chaining for chatMessageBelongsTo?._id is already properly implemented in all relevant locations in the codebase:
- In
ChatRoom.tsx, both occurrences use optional chaining - In
ChatRoom.test.tsx, the object is explicitly defined in test data - In
OrganizationMutations.ts, it's part of a GraphQL query structure where nullability is handled differently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential missing optional chaining on chatMessageBelongsTo
rg -A 2 "chatMessageBelongsTo\._id" "src/**/*.{ts,tsx}"
Length of output: 115
Script:
#!/bin/bash
# Let's try without the quotes in the glob pattern
rg -A 2 "chatMessageBelongsTo\._id" src/
Length of output: 42
Script:
#!/bin/bash
# Let's try a broader search for chatMessageBelongsTo to understand its usage
rg -A 2 "chatMessageBelongsTo" src/
Length of output: 1367
src/components/AgendaItems/AgendaItemsContainer.test.tsx (2)
34-41: LGTM: Improved console.error mock implementation.
The type check for the message before filtering React's defaultProps deprecation warning is a good practice that prevents potential type-related issues.
124-127: LGTM: Proper implementation of userEvent.setup().
The changes correctly implement the new userEvent.setup() pattern from @testing-library/user-event v14+. All user interactions are properly awaited, which helps prevent timing issues in async tests.
Also applies to: 248-251, 253-260, 272-273
src/screens/OrganizationEvents/OrganizationEvents.test.tsx (1)
Line range hint 241-270: LGTM: Appropriate use of both userEvent and fireEvent.
The test correctly uses:
- userEvent.setup() for standard DOM interactions (clicks, typing)
- fireEvent for date picker interactions where userEvent might be too strict
This mixed approach is appropriate as some third-party components work better with fireEvent.
src/screens/UserPortal/Events/Events.test.tsx (1)
23-23: LGTM: Optimized import by removing unused getItem.
The change to destructure only the needed setItem from useLocalStorage is a good optimization.
src/screens/BlockUser/BlockUser.test.tsx (1)
375-377: LGTM! Proper migration to userEvent.setup()
The changes correctly implement the new userEvent pattern by:
- Initializing setup before use
- Properly awaiting user actions
Also applies to: 383-384
src/components/RecurrenceOptions/RecurrenceOptions.test.tsx (1)
373-384: Review test timeout for complex async operations
The test contains multiple sequential async operations. Consider adjusting the timeout or implementing retry logic for better reliability.
src/components/UserPortal/PostCard/PostCard.test.tsx (7)
282-283: LGTM: Proper setup of userEvent
The initialization of userEvent with setup() and async/await pattern follows the correct usage for @testing-library/user-event v14.
332-340: LGTM: Proper handling of sequential user interactions
The sequential user interactions are correctly handled with async/await, ensuring proper test stability.
389-391: LGTM: Proper async interaction handling
The user interactions for delete functionality are correctly implemented with async/await pattern.
494-496: LGTM: Proper implementation of like/unlike interactions
The user interactions for like/unlike functionality are correctly implemented with async/await pattern.
548-550: LGTM: Proper async interaction handling
The user interactions are correctly implemented with async/await pattern.
642-646: LGTM: Proper handling of multiple interaction types
The combination of click and type interactions is correctly implemented with async/await pattern.
727-730: LGTM: Proper implementation of comment like functionality
The user interactions for comment like functionality are correctly implemented with async/await pattern.
src/components/RecurrenceOptions/CustomRecurrence.test.tsx (3)
213-215: LGTM: Proper implementation of complex user interactions
The test correctly implements multiple sequential user interactions using userEvent.setup() and async/await pattern.
Also applies to: 223-235, 245-253
479-490: LGTM: Proper form interaction implementation
The form interactions (typing and clicking) are correctly implemented with userEvent.setup() and async/await pattern.
Also applies to: 492-497, 512-524
628-639: LGTM: Proper implementation of recurring event creation
The test correctly implements the complex sequence of interactions required for creating recurring events.
Also applies to: 641-646, 662-674
src/components/EventListCard/EventListCard.test.tsx (1)
117-118: LGTM: Proper usage of userEvent.setup()
The upgrade to userEvent v14 is correctly implemented using userEvent.setup() before simulating user interactions. This is the recommended approach in the new version.
Also applies to: 179-180, 206-207, 230-231, 256-257, 283-284, 305-306, 324-325, 342-343, 386-387, 440-441, 485-486
src/screens/OrganizationPeople/OrganizationPeople.test.tsx (1)
733-740: LGTM: Proper async handling with userEvent
The upgrade implementation correctly handles async operations with proper await statements for user interactions.
Also applies to: 827-833, 887-892
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)
Line range hint 1425-1449: LGTM: Proper implementation of userEvent v14
The test case correctly implements userEvent v14 with setup and proper async handling.
src/screens/OrganizationActionItems/ItemModal.test.tsx (1)
761-761: Improved form submission handling in tests
The change from fireEvent.click to fireEvent.submit for form submissions is a good improvement as it:
- More accurately simulates the actual form submission behavior
- Ensures proper triggering of form validation and submission handlers
- Follows testing best practices for form interactions
Also applies to: 773-773, 792-792
| const user = userEvent.setup(); | ||
| await user.click(screen.getAllByTestId('unassignTagBtn')[0]); | ||
|
|
||
| userEvent.click(screen.getByTestId('unassignTagModalSubmitBtn')); | ||
| await user.click(screen.getByTestId('unassignTagModalSubmitBtn')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add UI state verification after tag unassignment
While the test verifies the success toast, it should also verify that the tag is removed from the UI after unassignment.
const user = userEvent.setup();
+const initialTagCount = screen.getAllByTestId('tagName').length;
await user.click(screen.getAllByTestId('unassignTagBtn')[0]);
await user.click(screen.getByTestId('unassignTagModalSubmitBtn'));
await waitFor(() => {
expect(toast.success).toHaveBeenCalledWith(
translations.successfullyUnassigned,
);
});
+
+// Verify tag is removed from UI
+await waitFor(() => {
+ const finalTagCount = screen.getAllByTestId('tagName').length;
+ expect(finalTagCount).toBe(initialTagCount - 1);
+});📝 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.
| const user = userEvent.setup(); | |
| await user.click(screen.getAllByTestId('unassignTagBtn')[0]); | |
| userEvent.click(screen.getByTestId('unassignTagModalSubmitBtn')); | |
| await user.click(screen.getByTestId('unassignTagModalSubmitBtn')); | |
| const user = userEvent.setup(); | |
| const initialTagCount = screen.getAllByTestId('tagName').length; | |
| await user.click(screen.getAllByTestId('unassignTagBtn')[0]); | |
| await user.click(screen.getByTestId('unassignTagModalSubmitBtn')); | |
| await waitFor(() => { | |
| expect(toast.success).toHaveBeenCalledWith( | |
| translations.successfullyUnassigned, | |
| ); | |
| }); | |
| // Verify tag is removed from UI | |
| await waitFor(() => { | |
| const finalTagCount = screen.getAllByTestId('tagName').length; | |
| expect(finalTagCount).toBe(initialTagCount - 1); | |
| }); |
| await user.upload(screen.getByTestId('addMediaField'), formData.postImage); | ||
| await user.upload(screen.getByTestId('addMediaField'), formData.postVideo); | ||
| await user.upload(screen.getByTestId('addMediaField'), formData.postImage); | ||
| await user.upload(screen.getByTestId('addMediaField'), formData.postVideo); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant upload operations
The same files are being uploaded twice consecutively, which appears to be unintentional and could affect test performance.
await user.upload(screen.getByTestId('addMediaField'), formData.postImage);
await user.upload(screen.getByTestId('addMediaField'), formData.postVideo);
- await user.upload(screen.getByTestId('addMediaField'), formData.postImage);
- await user.upload(screen.getByTestId('addMediaField'), formData.postVideo);📝 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.
| await user.upload(screen.getByTestId('addMediaField'), formData.postImage); | |
| await user.upload(screen.getByTestId('addMediaField'), formData.postVideo); | |
| await user.upload(screen.getByTestId('addMediaField'), formData.postImage); | |
| await user.upload(screen.getByTestId('addMediaField'), formData.postVideo); | |
| await user.upload(screen.getByTestId('addMediaField'), formData.postImage); | |
| await user.upload(screen.getByTestId('addMediaField'), formData.postVideo); |
| await act(async () => { | ||
| renderManageTag(link); | ||
| }); | ||
|
|
||
| userEvent.click(screen.getByTestId('addPeopleToTagBtn')); | ||
| expect(await screen.findByTestId('addPeopleToTagBtn')).toBeInTheDocument(); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByTestId('addPeopleToTagModal')).toBeInTheDocument(); | ||
| }); | ||
| userEvent.click(await screen.findByTestId('addPeopleToTagBtn')); | ||
|
|
||
| expect( | ||
| await screen.findByTestId('addPeopleToTagModal'), | ||
| ).toBeInTheDocument(); | ||
|
|
||
| userEvent.click(screen.getByTestId('closeAddPeopleToTagModal')); | ||
| userEvent.click(await screen.findByTestId('closeAddPeopleToTagModal')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update userEvent implementation to use setup()
While the async handling is correct, update to use the new userEvent.setup() pattern introduced in @testing-library/user-event v14:
- userEvent.click(await screen.findByTestId('addPeopleToTagBtn'));
+ const user = userEvent.setup();
+ await user.click(await screen.findByTestId('addPeopleToTagBtn'));This change aligns with the new version's recommended practices for simulating user interactions.
📝 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.
| await act(async () => { | |
| renderManageTag(link); | |
| }); | |
| userEvent.click(screen.getByTestId('addPeopleToTagBtn')); | |
| expect(await screen.findByTestId('addPeopleToTagBtn')).toBeInTheDocument(); | |
| await waitFor(() => { | |
| expect(screen.getByTestId('addPeopleToTagModal')).toBeInTheDocument(); | |
| }); | |
| userEvent.click(await screen.findByTestId('addPeopleToTagBtn')); | |
| expect( | |
| await screen.findByTestId('addPeopleToTagModal'), | |
| ).toBeInTheDocument(); | |
| userEvent.click(screen.getByTestId('closeAddPeopleToTagModal')); | |
| userEvent.click(await screen.findByTestId('closeAddPeopleToTagModal')); | |
| const user = userEvent.setup(); | |
| await act(async () => { | |
| renderManageTag(link); | |
| }); | |
| expect(await screen.findByTestId('addPeopleToTagBtn')).toBeInTheDocument(); | |
| await user.click(await screen.findByTestId('addPeopleToTagBtn')); | |
| expect( | |
| await screen.findByTestId('addPeopleToTagModal'), | |
| ).toBeInTheDocument(); | |
| await user.click(await screen.findByTestId('closeAddPeopleToTagModal')); |
| userEvent.click(screen.getByTestId(/nextPage/i)); | ||
| userEvent.click(screen.getByTestId(/previousPage/i)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update to newer userEvent.setup() pattern.
Since we're upgrading to @testing-library/user-event@14.x, we should use the new setup pattern:
- userEvent.click(screen.getByTestId(/nextPage/i));
- userEvent.click(screen.getByTestId(/previousPage/i));
+ const user = userEvent.setup();
+ await user.click(screen.getByTestId(/nextPage/i));
+ await user.click(screen.getByTestId(/previousPage/i));Also applies to: 59-60
| @@ -1,14 +1,13 @@ | |||
| import React from 'react'; | |||
| import type { RenderResult } from '@testing-library/react'; | |||
| import { render, screen, waitFor } from '@testing-library/react'; | |||
| import { fireEvent, render, screen, waitFor } from '@testing-library/react'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using userEvent.setup() for consistency
The current changes use fireEvent, which is more low-level and doesn't simulate real browser behavior as accurately as userEvent. Consider aligning with the project's migration pattern seen in other files by using userEvent.setup().
Example migration:
- import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+ import { render, screen, waitFor } from '@testing-library/react';
+ import userEvent from '@testing-library/user-event';
test('Testing back button navigation when userType is SuperAdmin', async () => {
setItem('SuperAdmin', true);
renderEventManagement();
+ const user = userEvent.setup();
const backButton = screen.getByTestId('backBtn');
- fireEvent.click(backButton);
+ await user.click(backButton);
await waitFor(() => {
const eventsScreen = screen.getByTestId('eventsScreen');
expect(eventsScreen).toBeInTheDocument();
});
});Similar changes should be applied to all other click events in the file.
Also applies to: 68-68, 79-79, 84-84, 88-88, 94-94, 100-100, 106-106
| const user = userEvent.setup(); | ||
| await user.click(screen.getByText('People')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update remaining userEvent.click calls to use setup pattern.
While this section correctly uses the new userEvent.setup() pattern, there's an inconsistent usage at line 339 that still uses the old userEvent.click directly. Consider updating it for consistency.
Apply this change to line 339:
- userEvent.click(peopelBtn);
+ const user = userEvent.setup();
+ await user.click(peopelBtn);Committable suggestion skipped: line range outside the PR's diff.
| const user = userEvent.setup(); | ||
| await user.click(screen.getByText('Dashboard')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update remaining userEvent calls for consistency
While this test correctly implements the new userEvent pattern, other tests in the file still use the old userEvent.click pattern (e.g., line 397).
Update all userEvent calls in the file to use the new pattern. For example:
- userEvent.click(peopelBtn);
+ const user = userEvent.setup();
+ await user.click(peopelBtn);This ensures consistency and prevents potential timing issues with user interactions.
Committable suggestion skipped: line range outside the PR's diff.
| const user = userEvent.setup(); | ||
| await user.type(screen.getByTestId('inputAddress'), 'random'); | ||
| await wait(); | ||
| userEvent.click(screen.getByTestId('resetChangesBtn')); | ||
| fireEvent.click(screen.getByTestId('resetChangesBtn')); |
There was a problem hiding this comment.
💡 Codebase verification
Migration pattern is inconsistent across test files
The codebase shows mixed usage of userEvent patterns:
- Some files use the recommended
userEvent.setup()pattern (e.g.,Actions.test.tsx,OrgPost.test.tsx) - Others use direct
userEventmethods (e.g.,Users.test.tsx,SubTags.test.tsx) - Some files mix
userEventwithfireEvent(e.g.,Settings.test.tsx,OrgList.test.tsx)
For consistent testing patterns:
- All test files should use
userEvent.setup()for user interactions - Avoid mixing
fireEventwithuserEventas per testing-library best practices - Migrate remaining direct
userEventcalls to use the setup pattern
🔗 Analysis chain
LGTM! Correct migration to userEvent.setup()
The changes correctly implement the new pattern recommended by @testing-library/user-event v14, using setup() for initialization and async user interactions.
Let's verify this migration pattern across other test files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of userEvent.setup() pattern
rg -A 2 "userEvent\.setup\(\)" "src/**/*.test.{ts,tsx}"
Length of output: 120
Script:
#!/bin/bash
# Let's try without the quotes around the glob pattern
rg -A 2 "userEvent\.setup\(\)" src/**/*.test.{ts,tsx}
# Backup search to find userEvent usage patterns
rg -A 2 "userEvent\." src/**/*.test.{ts,tsx}
Length of output: 614
Script:
#!/bin/bash
# Let's try using fd to find test files first, then pipe to ripgrep
fd -e test.tsx -e test.ts | xargs rg -A 2 "userEvent\."
# Also search for fireEvent usage to understand the mixing patterns
fd -e test.tsx -e test.ts | xargs rg -A 2 "fireEvent\."
Length of output: 67176
| fireEvent.change(mediaInput, { | ||
| target: { | ||
| files: [mediaFile], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Consider using userEvent.upload for file inputs
The change from userEvent.upload to fireEvent.change for file inputs is a regression in terms of testing fidelity. userEvent.upload provides a more realistic simulation of user interactions with file inputs.
- fireEvent.change(mediaInput, {
- target: {
- files: [mediaFile],
- },
- });
+ const user = userEvent.setup();
+ await user.upload(mediaInput, mediaFile);📝 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.
| fireEvent.change(mediaInput, { | |
| target: { | |
| files: [mediaFile], | |
| }, | |
| }); | |
| const user = userEvent.setup(); | |
| await user.upload(mediaInput, mediaFile); |
| await waitFor(() => { | ||
| expect(toast.success).toHaveBeenCalledWith(translations.eventUpdated); | ||
| console.log(translations.eventUpdated); |
There was a problem hiding this comment.
Replace console.log with proper assertions
Several test cases are using console.log instead of proper assertions to verify the expected behavior. This reduces the effectiveness of the tests.
Replace the console.log statements with proper assertions. For example:
- console.log(translations.eventUpdated);
+ expect(toast.success).toHaveBeenCalledWith(translations.eventUpdated);Also applies to: 472-472, 712-712, 779-779, 842-842, 883-883
|
Please fix the failing tests |
|
@mohamedrehan1 I am closing this as there are a lot of files here that need changes. Please let me know if you are actively working on it and it needs to be reopened. |
when i upgraded to 14.5.2 there was 70 failing tests so i start fixing all of them so most of changes coderabbitai asked me to do will make test fall for example he asked ( Consider using userEvent.upload for file inputs) but when i use userEvent.upload for file inputs i got this error thrown: "Exceeded timeout of 15000 ms for a test. |
Description
Updated project dependencies and configurations to their latest stable versions. Key updates include:
Fixes #2072
Testing Library Updates
Test Fixes & Updates
Bug Fixes
Summary by CodeRabbit
userEvent.setup()across various components, improving asynchronous handling.userEventmethods to usingawait user.click()andawait user.type()for better control of asynchronous operations.fireEvent.submit()for more accurate event handling.