Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, search bar layout - DropDownButton: white bg + gray text default; blue bg + white text on hover/active - UserGlobalScreen + UserScreen: replace placeholder fallback with buttonLabel='▼' - Toolbar: wrap SearchBar in .searchBarWrapper for proper flex-item sizing - Toolbar: change flex-wrap to nowrap so search, filters, actions stay on one row
- Rework sidebar toggle: hamburger when collapsed, branding + circle chevron when expanded - Fix collapse button with full-height wrapper and flexbox centering for reliable click target - Add sidebar right border to separate it visually from page content - Sync ProfileCard visibility with sidebar animation via max-height/opacity CSS transitions - Make ProfileCard fully clickable, navigating to profile settings - Fix blue hover on ProfileCard and SidebarOrgSection by adding variant=link - Fix icon sizes: forward width/height in IconComponent for My Organizations and Settings; add strokeWidth 1.5 to normalise thin-stroke SVGs (Plugins); add iconSize prop to SidebarNavItem to compensate for FaUsers wide viewBox (640x512) - Fix Create Org button background and alignment - Fix Switch to Portal icon spacing - Fix Users table number alignment and text size - Remove duplicate search icon in Toolbar - Fix Create Post button default and hover styles - Replace hardcoded CSS values with design tokens in LeftDrawer and UserSidebar - Fix MUI barrel import in IconComponent to use deep path imports
- Remove duplicate search button (showSearchButton=false keeps leading icon only) - Increase gap between search bar and controls from 2px to 12px - Style Create Event button to match SortingButton: white bg, blue on hover - Fix responsiveness: allow search bar to shrink so buttons stay on same row when sidebar is open (min-width reduced from 420px to 160px)
Fix 6 test files with failures caused by the Toolbar/SearchFilterBar refactoring: update mock targets, fix CSS class assertions, replace per-character userEvent.type with atomic setNativeInputValue for server-side search, and correct post-search data assertions.
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
WalkthroughVite config now conditionally includes Istanbul and gates proxy request/response logging behind env flags. Many test files were updated: mocks swapped to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.1)src/components/AdminPortal/Advertisements/Advertisements.spec.tsxError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep-rules/no-dynamic-timestamps-in-tests.yml ✖ Caused by src/screens/AdminPortal/SubTags/SubTags.spec.tsxError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep-rules/no-dynamic-timestamps-in-tests.yml ✖ Caused by src/screens/AdminPortal/Users/Users.spec.tsxError: Cannot parse rule /ast-grep-rules/../git/.coderabbit/ast-grep-rules/no-dynamic-timestamps-in-tests.yml ✖ Caused by
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The linked issue must be assigned to the PR author. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsx (1)
78-91:⚠️ Potential issue | 🔴 CriticalReplace
vi.clearAllMocks()withvi.restoreAllMocks()in lifecycle hooks; move async assertions intowaitForblocksThree blocking test-stability issues:
Lines 125 & 135 —
vi.clearAllMocks()is forbidden. Usevi.restoreAllMocks()in bothbeforeEachandafterEach.Line 135-136 —
afterEachmissingvi.restoreAllMocks(). Must include bothcleanup()ANDvi.restoreAllMocks()for sharded CI.Lines 494 & 510 — Assertions follow awaited user interactions without
waitFor. Move intowaitForblocks:
- Line 494:
expect(mainContainer).toHaveClass('expand')followsawait user.click(closeButton)- Line 510:
expect(mainContainer).toHaveClass('contract')followsawait user.click(openButton)Suggested patch
beforeEach(() => { - vi.clearAllMocks(); + vi.restoreAllMocks(); }); afterEach(() => { - vi.clearAllMocks(); + vi.restoreAllMocks(); cleanup(); Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: originalInnerWidth, }); }); it('should apply expand class when drawer is hidden', async () => { const user = userEvent.setup(); renderComponent(); const closeButton = screen.getByTestId('closeMenu'); await user.click(closeButton); const mainContainer = screen.getByTestId('mainpageright'); - expect(mainContainer).toHaveClass('expand'); + await waitFor(() => expect(mainContainer).toHaveClass('expand')); }); it('should apply contract class when drawer is shown', async () => { const user = userEvent.setup(); renderComponent(); const closeButton = screen.getByTestId('closeMenu'); await user.click(closeButton); const openButton = screen.getByTestId('openMenu'); await user.click(openButton); const mainContainer = screen.getByTestId('mainpageright'); - expect(mainContainer).toHaveClass('contract'); + await waitFor(() => expect(mainContainer).toHaveClass('contract')); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsx` around lines 78 - 91, Replace vi.clearAllMocks() calls with vi.restoreAllMocks() in the test lifecycle hooks: update the beforeEach to use vi.restoreAllMocks() instead of vi.clearAllMocks(), and ensure afterEach calls both cleanup() and vi.restoreAllMocks() (add vi.restoreAllMocks() if missing). Also make the two post-interaction assertions stable by moving them into waitFor blocks: after awaiting user.click(closeButton) wrap expect(mainContainer).toHaveClass('expand') inside await waitFor(() => expect(mainContainer).toHaveClass('expand')), and after awaiting user.click(openButton) wrap expect(mainContainer).toHaveClass('contract') inside await waitFor(() => expect(mainContainer).toHaveClass('contract')).src/screens/UserPortal/Organizations/Organizations.spec.tsx (1)
539-560:⚠️ Potential issue | 🟡 MinorUpdate test name to match Enter-key behavior
Issue Goals:
#7544intent is addressed by moving search submission away fromsearchBtnto Enter-key flow.- Gap: test description is stale: Line 539 says “clicking search button” while Line 560 now submits via keyboard.
Tests (incl. Flakiness):
- ✅ Awaited user interactions and post-action assertions are properly synchronized.
- ✅ No hardcoded delays introduced in changed segments.
Components/Policy:
- ✅
afterEachincludes bothcleanup()andvi.restoreAllMocks().GraphQL:
- ✅ No blocking request-shape issue in changed lines.
i18n & a11y:
- ✅
I18nextProviderremains present.Security:
- ✅ No security concerns in this changed segment.
Action Items:
- Rename the test at Line 539 to reflect keyboard submission (Enter key), e.g. “should search organizations when pressing Enter key”.
Suggested diff
-test('should search organizations when clicking search button', async () => { +test('should search organizations when pressing Enter key', async () => {As per coding guidelines, “Issue goals (Priority
#1): … list gaps with file:line” and flakiness checks require clear, deterministic async test intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx` around lines 539 - 560, Update the Jest test title string for the test that currently reads "should search organizations when clicking search button" to reflect the new Enter-key submission flow (e.g., "should search organizations when pressing Enter key") so the description matches behavior in the test that types into searchInput and triggers await userEvent.keyboard('{Enter}'); ensure you update the test() call surrounding the render and userEvent steps in Organizations.spec.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsx`:
- Around line 58-60: The test currently uses optional chaining in the click
handler (props.filters?.[0]?.onChange?.('invalid')), allowing the click to
silently no-op if filters[0].onChange is missing; change this to fail loudly by
ensuring the handler exists and is callable: either remove the final optional
chain to call props.filters![0]!.onChange('invalid') so test fails if missing,
or add an explicit guard that throws (e.g., if (typeof
props.filters?.[0]?.onChange !== 'function') throw new
Error('filters[0].onChange is required in this test');
props.filters[0].onChange('invalid')). Target the onClick block that references
props.filters and onChange in OrganizationPeople.spec.tsx.
In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx`:
- Around line 483-495: The test claims to verify input trimming but currently
types a plain 'Teresa', so it won't catch regressions in trim(); update the test
around the search input (searchInput retrieved via getByTestId('searchByInput')
and set via setNativeInputValue) to supply a value with surrounding whitespace
(e.g., leading/trailing spaces) instead of 'Teresa', keep the debounce/waitFor
assertions (expect Group 1 present, Group 2 absent) and ensure you still call
userEvent.clear(searchInput) before setNativeInputValue so the test explicitly
covers the trim() path.
- Around line 1434-1444: The test currently only asserts the input value after
calling setNativeInputValue(searchInput, 'Group 1') and can falsely pass if the
debounced search handler never runs; update the test (the block using
setNativeInputValue, searchInput, and waitFor) to also assert observable
filtered results from the UI—e.g., inside the waitFor after confirming the input
value, add an assertion using screen.queryByText('Group 2') or
screen.getByText('Group 1') to verify that Group 2 is not rendered and Group 1
is present (which confirms the GraphQL mock for name_contains: 'Group 1' ran);
make the same change in the other test block referenced around lines 1467–1473
that uses the same helpers so both tests validate UI results rather than only
the input value.
- Around line 678-683: Change the flaky search assertion to actually wait for
the refetch by typing a unique term (use setNativeInputValue(searchInput, 'Group
1')) so only one result is returned, then wrap the expect(viewButtons.length ===
1) inside a waitFor to ensure the grid reloads before clicking; locate the test
block that queries findAllByTestId('viewGroupBtn') and replace the unconditional
find/assert/click sequence with a waitFor-based assert followed by
userEvent.click(viewButtons[0]). Also fix the afterEach cleanup by replacing
vi.clearAllMocks() with vi.restoreAllMocks() in the test teardown so mocks are
properly restored between sharded tests.
In `@src/shared-components/posts/posts.spec.tsx`:
- Around line 155-160: The test triggers search.onSearch twice because the mock
wired to the input calls search.onSearch on every onChange and again on Enter;
update the test to match the Toolbar contract by removing the onChange
invocation and only invoking search.onSearch from the onKeyDown Enter handler
(i.e., keep the onKeyDown block that calls search.onSearch((e.target as
HTMLInputElement).value) and stop calling search.onSearch in onChange), ensuring
the mocked component uses the same single- invocation behavior as Toolbar and
prevents duplicate query triggers.
In `@src/shared-components/SortingButton/SortingButton.spec.tsx`:
- Around line 287-293: The test "should render type-based icon when type prop is
provided" in SortingButton.spec.tsx duplicates assertions already covered by the
existing "Icon Types" sort-path test; remove the redundant expectation(s) —
specifically drop either the whole test or at least the duplicated lines
checking data-icon-type='sort' (the getByTestId('sorting-icon') and
expect(...).toHaveAttribute('data-icon-type', 'sort')) so only the unique
coverage from the "Icon Types" assertions remains; locate this in the
SortingButton.spec.tsx test block and delete the redundant assertion(s) to keep
the spec lean.
---
Outside diff comments:
In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx`:
- Around line 539-560: Update the Jest test title string for the test that
currently reads "should search organizations when clicking search button" to
reflect the new Enter-key submission flow (e.g., "should search organizations
when pressing Enter key") so the description matches behavior in the test that
types into searchInput and triggers await userEvent.keyboard('{Enter}'); ensure
you update the test() call surrounding the render and userEvent steps in
Organizations.spec.tsx.
In `@src/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsx`:
- Around line 78-91: Replace vi.clearAllMocks() calls with vi.restoreAllMocks()
in the test lifecycle hooks: update the beforeEach to use vi.restoreAllMocks()
instead of vi.clearAllMocks(), and ensure afterEach calls both cleanup() and
vi.restoreAllMocks() (add vi.restoreAllMocks() if missing). Also make the two
post-interaction assertions stable by moving them into waitFor blocks: after
awaiting user.click(closeButton) wrap
expect(mainContainer).toHaveClass('expand') inside await waitFor(() =>
expect(mainContainer).toHaveClass('expand')), and after awaiting
user.click(openButton) wrap expect(mainContainer).toHaveClass('contract') inside
await waitFor(() => expect(mainContainer).toHaveClass('contract')).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 85a8e4cc-5e56-4544-a8cd-76865bcb7c9a
📒 Files selected for processing (7)
config/vite.config.tssrc/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsxsrc/screens/UserPortal/Organizations/Organizations.spec.tsxsrc/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsxsrc/screens/UserPortal/Volunteer/Groups/Groups.spec.tsxsrc/shared-components/SortingButton/SortingButton.spec.tsxsrc/shared-components/posts/posts.spec.tsx
|
@coderabbitai Evaluate these fading github actions, explain why they are failing and provide solutions with starter sample code: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me fetch the details of all the failing GitHub Actions jobs. 🏁 Script executed: Length of output: 1860 All shards failed. Now let me pull the step-level details and logs to find the actual error messages. 🏁 Script executed: Length of output: 1050 All shards fail at the Vitest step. Let me download the logs to see exactly which tests are failing. 🏁 Script executed: Length of output: 89722 Good. I can clearly see two spec files with confirmed test failures. Let me get their actual assertion errors now. 🏁 Script executed: Length of output: 19917 Now let me read the actual failing spec files to understand exactly what needs to change. 🏁 Script executed: Length of output: 5127 🏁 Script executed: Length of output: 5588 🏁 Script executed: Length of output: 21798 Now let me check the Advertisements component source to understand the current 🏁 Script executed: Length of output: 11587 🏁 Script executed: Length of output: 7241 🏁 Script executed: Length of output: 9442 Now let me get the complete test failure summary for each shard from the end of their logs. 🏁 Script executed: Length of output: 2618 Excellent! Now I have 3 more failing files. Let me get the remaining shard failure locations and check the identified spec files. 🏁 Script executed: Length of output: 5059 Now let me read the relevant lines from each failing spec file. 🏁 Script executed: Length of output: 6755 🏁 Script executed: Length of output: 8158 🏁 Script executed: Length of output: 5706 Now I have a complete picture. Let me compile the full analysis and solutions.
Root Causes & SolutionsAll failures stem from the Toolbar/SearchBar/Sidebar refactor that changed component APIs, CSS class names, and 🔴 Category 1:
|
| File | Line | Likely Issue |
|---|---|---|
AddMember.spec.tsx |
1862 | Mock timing / missing data table rows after async query |
ChatRoom.spec.tsx |
1027, 1141, 1226 | Mock exhaustion for DELETE_CHAT_MESSAGE / MARK_CHAT_MESSAGES_AS_READ mutations |
EventManager.spec.ts |
204–228 | EventManager.emit() silently swallowing errors instead of passing them through |
InternalFileWriter.spec.ts |
191, 225, 321 | window object setup mismatch with jsdom environment |
Donate.spec.tsx |
875 | variableMatcher mock not matching in MockedProvider |
OrganizationModal.spec.tsx |
668 | Toast error message text mismatch ('Failed to upload image' vs. actual string) |
AdvertisementRegister.spec.tsx |
172 | Same toast text mismatch on upload failure |
getRefreshToken.spec.ts |
187 | localStorageMock.key not behaving correctly in handleTokenRefresh |
Summary
| Shard | Failing File | Root Cause | Fix |
|---|---|---|---|
| 1 | AddMember.spec.tsx |
Pre-existing (needs investigation) | — |
| 2 | SearchFilterBar.spec.tsx |
selectOptions on non-native select |
Use click toggle + click option |
| 3 | SubTags.spec.tsx |
searchBtn removed by Toolbar |
Replace with keyboard('{Enter}') |
| 4 | Pledges.spec.tsx |
searchBtn removed by Toolbar |
Replace with keyboard('{Enter}') |
| 5 | Navbar.spec.tsx |
icon prop removed, use type |
Replace icon with type, update assertion |
| 6 | SidebarBase.spec.tsx |
Missing talawa-logo testid; renamed hamburger CSS |
Add testid to logo; add conditional hamburger class |
| 7 | ChatRoom.spec.tsx, EventManager.spec.ts |
Pre-existing mock exhaustion / error propagation | — |
| 8 | InternalFileWriter.spec.ts, Donate.spec.tsx, OrganizationModal.spec.tsx, Users.spec.tsx |
Mock where: undefined, toast text, window mismatch |
Match actual query variables; fix toast strings |
| 9 | AdvertisementRegister.spec.tsx, getRefreshToken.spec.ts |
Pre-existing (toast text / localStorage mock) | — |
| 11 | Advertisements.spec.tsx |
searchButton hidden by Toolbar |
Remove button assertion; use keyboard('{Enter}') |
The clearest wins (Categories 1–5) are directly caused by the Toolbar/component refactor and can be fixed by updating ~8 test files following the same pattern this PR already applied to Organizations.spec.tsx.
- Replace vi.clearAllMocks() with vi.restoreAllMocks() across all specs - Wrap post-interaction assertions in waitFor() for async stability - Remove duplicate type-based icon test in SortingButton - Fix Toolbar mock in posts.spec to only fire onSearch on Enter - Add explicit guard for missing filter handler in OrganizationPeople - Restore whitespace padding in Groups trim test - Rename misleading test names in Organizations spec
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsx (1)
305-320:⚠️ Potential issue | 🔴 Critical🔴 Remaining race window in toggle-back flow despite added
waitForIssue Goals
- ✅ Refactor-aligned class/assertion updates are covered in this file (e.g.,
contract/expand).⚠️ One acceptance-risk remains for flakiness: the test still interacts with the reopen control before explicitly waiting for it.Tests (incl. Flakiness)
- Line 314–315:
getByTestId('openMenu')+ immediate click can race after the previous async click on close.- This can still fail intermittently in shard CI even though final assertions are in
waitFor(Lines 317–320).Suggested fix
- const openButton = screen.getByTestId('openMenu'); - await user.click(openButton); + const openButton = await screen.findByTestId('openMenu'); + await user.click(openButton);Components/Policy
- ✅ No component-policy violations identified in changed lines.
GraphQL
- ✅ Not applicable in this spec segment.
i18n & a11y
- ✅ No new i18n/a11y regressions introduced by changed lines.
Security
- ✅ No security concerns in changed lines.
Action Items
- Update Line 314 to use an async query (
findByTestId) before clicking.- Keep final assertions in
waitForas currently done.As per coding guidelines, “After clicking elements that open UI (dropdowns, modals, dialogs, tooltips): MUST waitFor the container/menu itself to be visible BEFORE checking child elements.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsx` around lines 305 - 320, The test races because it calls getByTestId('openMenu') and immediately user.click after the prior async click; change the second lookup to await screen.findByTestId('openMenu') and then await user.click(openButton) so the reopen control is present before interaction, and keep the existing waitFor(...) assertions that verify closeMenu/openMenu states; reference the existing getByTestId('closeMenu'), findByTestId('openMenu'), user.click and waitFor calls when making the change.src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx (2)
116-116:⚠️ Potential issue | 🔴 Critical❌ NON-DETERMINISTIC TIMESTAMP — Use fixed UTC string.
dayjs()without arguments captures the current time, causing test flakiness in sharded CI environments where tests run at different times.const group1 = { __typename: 'EventVolunteerGroup', id: 'groupId1', name: 'Group 1', description: 'Volunteer Group Description', volunteersRequired: null, - createdAt: dayjs().toISOString(), + createdAt: '2025-01-01T10:00:00.000Z', leader: {const group2 = { __typename: 'EventVolunteerGroup', id: 'groupId2', name: 'Group 2', description: 'Volunteer Group Description', volunteersRequired: null, - createdAt: dayjs().toISOString(), + createdAt: '2025-01-01T10:00:00.000Z', leader: {As per coding guidelines: "❌ FORBIDDEN PATTERNS:
dayjs()without arguments (usedayjs('2025-01-01T10:00:00Z'))"Also applies to: 157-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx` at line 116, Replace the non-deterministic dayjs().toISOString() used for test timestamps with a fixed UTC string (e.g. '2025-01-01T10:00:00Z'); update the createdAt fields in the test fixture/mock objects referenced in Groups.spec.tsx (the createdAt property in the test data and the other occurrence noted at line 157) so tests use a deterministic timestamp instead of dayjs() without arguments.
1467-1476: 🧹 Nitpick | 🔵 TrivialConsider adding observable result assertion after Enter submit.
This test verifies the Enter key submit path but only asserts the input value. Adding an observable result check (similar to line 1444) would confirm the submit actually triggered the search.
Suggested enhancement
setNativeInputValue(searchInput, 'Group 1'); await userEvent.keyboard('{Enter}'); // Verify search is handled - expect(searchInput).toHaveValue('Group 1'); + await waitFor(() => { + expect(searchInput).toHaveValue('Group 1'); + expect(screen.queryByText('Group 2')).not.toBeInTheDocument(); + }, { timeout: 1500 }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx` around lines 1467 - 1476, The test currently simulates typing into searchByInput via setNativeInputValue and pressing Enter (userEvent.keyboard) but only asserts the input value; add an observable result assertion after the Enter press to confirm the search ran (for example, assert the expected group appears in the DOM using screen.getByText('Group 1') or a relevant results test id), placing this check immediately after the existing expect(searchInput).toHaveValue('Group 1') so the Enter-submit path verifies visible search results.src/shared-components/SortingButton/SortingButton.spec.tsx (1)
178-221:⚠️ Potential issue | 🔴 Critical❌ BLOCKING - Missing
cleanup()call inafterEachhook and assertions not awaited after user interactions.Line 85-87:
afterEachonly callsvi.restoreAllMocks(). Addcleanup()from@testing-library/reactfor proper DOM teardown in sharded test execution.Lines 183-186, 196-197, 207-208, 217-220, 333-336: Assertions after
await user.click()must be wrapped inwaitForblocks. Although the mocked DropDownButton renders synchronously, the real component uses React-Bootstrap's async dropdown behavior. Wrap all assertions inwaitForfor robustness and to match actual component behavior:+import { render, screen, cleanup, waitFor } from '@testing-library/react'; ... await user.click(screen.getByTestId('sort-toggle')); + await waitFor(() => { expect(screen.getByTestId('latest')).toBeInTheDocument(); expect(screen.getByTestId('oldest')).toBeInTheDocument(); expect(screen.getByTestId('1')).toBeInTheDocument(); expect(screen.getByTestId('2')).toBeInTheDocument(); + });Also add to
afterEach:afterEach(() => { + cleanup(); vi.restoreAllMocks(); });Also applies to: 189-209, 314-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared-components/SortingButton/SortingButton.spec.tsx` around lines 178 - 221, Add cleanup() to the afterEach hook and ensure assertions following async user interactions are awaited inside waitFor: import cleanup and waitFor from `@testing-library/react`, update the afterEach to call vi.restoreAllMocks() and cleanup(), and wrap the post-click expects in waitFor in the SortingButton tests (references: afterEach, user.click calls targeting testIds 'sort-toggle', 'latest', 'oldest', '1', '2', and the vi.fn() onSortChange assertions) so DOM teardown and the asynchronous dropdown behavior are handled reliably.src/screens/UserPortal/Organizations/Organizations.spec.tsx (2)
592-609:⚠️ Potential issue | 🟠 MajorAssert mode-specific output after each mode switch.
Re-checking
organizations-listhere does not prove the toggle worked, because that container is already rendered before switching modes. This test can still pass if the joined/created queries never run. Assert a mode-unique card or other mode-specific result after each selection so the regression coverage remains meaningful.Based on learnings: screen tests should verify integration behavior in different scenarios rather than only asserting a shared container remains mounted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx` around lines 592 - 609, The test currently only re-checks the shared container ('organizations-list') after toggling modes, which doesn't verify the mode switch; update the assertions after clicking modeChangeBtn-item-1 and modeChangeBtn-item-2 to assert a mode-specific outcome (e.g., a unique test id or text for the Joined vs Created view) instead of just 'organizations-list' — for example assert a Joined-specific card/testid (like 'joined-organization-card' or a known organization name returned by the joined query) after clicking modeChangeBtn-item-1 and assert a Created-specific card/testid (like 'created-organization-card' or expected created org name) after clicking modeChangeBtn-item-2 (you can also assert that the relevant API/mocked query for the mode was invoked); update the checks around userEvent.click(modeButton), modeChangeBtn-item-1, and modeChangeBtn-item-2 to use those mode-unique assertions so the test actually validates mode-specific rendering.
556-560:⚠️ Potential issue | 🟠 MajorReplace global keyboard dispatch with input-targeted Enter key in all search test cases.
userEvent.keyboard('{Enter}')is focus-dependent and flaky in sharded CI when the input rerenders during async operations. The file already demonstrates the correct pattern at line 531: useawait userEvent.type(searchInput, '{Enter}')instead to ensure the keypress targets the specific input element.Instances to fix: lines 560, 734, 1703, 1791, 2032, 2136.
🔧 Correct pattern
const searchInput = screen.getByTestId('searchInput'); await userEvent.clear(searchInput); await userEvent.type(searchInput, 'Search Term'); - - await userEvent.keyboard('{Enter}'); + await userEvent.type(searchInput, '{Enter}');Human: For context, the learning from suhaib-ilahi on PR
#7187confirmed thatuser.keyboard()targeting wrong input due to focus issues caused shard failures; the fix was using input-targeted typing instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx` around lines 556 - 560, The test uses a global Enter dispatch via userEvent.keyboard('{Enter}') which is focus-dependent and flaky; update each failing case to target the specific input element by replacing calls to userEvent.keyboard('{Enter}') with await userEvent.type(searchInput, '{Enter}') (e.g., where searchInput is obtained via screen.getByTestId('searchInput') in the Organizations.spec.tsx tests at the noted instances) so the Enter keypress is sent directly to that input and not reliant on document focus.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx`:
- Around line 678-685: The test currently calls findAllByTestId('viewGroupBtn')
which can resolve against the initial pre-search state and makes the following
waitFor ineffective; after calling setNativeInputValue(searchInput, 'Group') you
should instead wait for a search-specific update (e.g., wait for the grid to
reflect the filtered results) before capturing the view buttons—use waitFor with
getAllByTestId('viewGroupBtn') (or await screen.findByText / another unique
result indicator) to assert the expected post-search count/state, then call
userEvent.click on the resolved button; update references around
setNativeInputValue, searchInput, viewGroupBtn, waitFor, and userEvent.click
accordingly.
In `@src/shared-components/SortingButton/SortingButton.spec.tsx`:
- Around line 85-87: The test suite is missing DOM cleanup between tests; import
cleanup from `@testing-library/react` and call cleanup() inside the existing
afterEach (e.g., in the same block that currently calls vi.restoreAllMocks()) so
the rendered DOM from render() in SortingButton tests is cleared between tests;
ensure cleanup() runs before vi.restoreAllMocks() in the afterEach that
currently surrounds the tests.
---
Outside diff comments:
In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx`:
- Around line 592-609: The test currently only re-checks the shared container
('organizations-list') after toggling modes, which doesn't verify the mode
switch; update the assertions after clicking modeChangeBtn-item-1 and
modeChangeBtn-item-2 to assert a mode-specific outcome (e.g., a unique test id
or text for the Joined vs Created view) instead of just 'organizations-list' —
for example assert a Joined-specific card/testid (like
'joined-organization-card' or a known organization name returned by the joined
query) after clicking modeChangeBtn-item-1 and assert a Created-specific
card/testid (like 'created-organization-card' or expected created org name)
after clicking modeChangeBtn-item-2 (you can also assert that the relevant
API/mocked query for the mode was invoked); update the checks around
userEvent.click(modeButton), modeChangeBtn-item-1, and modeChangeBtn-item-2 to
use those mode-unique assertions so the test actually validates mode-specific
rendering.
- Around line 556-560: The test uses a global Enter dispatch via
userEvent.keyboard('{Enter}') which is focus-dependent and flaky; update each
failing case to target the specific input element by replacing calls to
userEvent.keyboard('{Enter}') with await userEvent.type(searchInput, '{Enter}')
(e.g., where searchInput is obtained via screen.getByTestId('searchInput') in
the Organizations.spec.tsx tests at the noted instances) so the Enter keypress
is sent directly to that input and not reliant on document focus.
In `@src/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsx`:
- Around line 305-320: The test races because it calls getByTestId('openMenu')
and immediately user.click after the prior async click; change the second lookup
to await screen.findByTestId('openMenu') and then await user.click(openButton)
so the reopen control is present before interaction, and keep the existing
waitFor(...) assertions that verify closeMenu/openMenu states; reference the
existing getByTestId('closeMenu'), findByTestId('openMenu'), user.click and
waitFor calls when making the change.
In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx`:
- Line 116: Replace the non-deterministic dayjs().toISOString() used for test
timestamps with a fixed UTC string (e.g. '2025-01-01T10:00:00Z'); update the
createdAt fields in the test fixture/mock objects referenced in Groups.spec.tsx
(the createdAt property in the test data and the other occurrence noted at line
157) so tests use a deterministic timestamp instead of dayjs() without
arguments.
- Around line 1467-1476: The test currently simulates typing into searchByInput
via setNativeInputValue and pressing Enter (userEvent.keyboard) but only asserts
the input value; add an observable result assertion after the Enter press to
confirm the search ran (for example, assert the expected group appears in the
DOM using screen.getByText('Group 1') or a relevant results test id), placing
this check immediately after the existing expect(searchInput).toHaveValue('Group
1') so the Enter-submit path verifies visible search results.
In `@src/shared-components/SortingButton/SortingButton.spec.tsx`:
- Around line 178-221: Add cleanup() to the afterEach hook and ensure assertions
following async user interactions are awaited inside waitFor: import cleanup and
waitFor from `@testing-library/react`, update the afterEach to call
vi.restoreAllMocks() and cleanup(), and wrap the post-click expects in waitFor
in the SortingButton tests (references: afterEach, user.click calls targeting
testIds 'sort-toggle', 'latest', 'oldest', '1', '2', and the vi.fn()
onSortChange assertions) so DOM teardown and the asynchronous dropdown behavior
are handled reliably.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f819217c-00b3-4138-ac0b-76aeaa231e4d
📒 Files selected for processing (6)
src/screens/AdminPortal/OrganizationPeople/OrganizationPeople.spec.tsxsrc/screens/UserPortal/Organizations/Organizations.spec.tsxsrc/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsxsrc/screens/UserPortal/Volunteer/Groups/Groups.spec.tsxsrc/shared-components/SortingButton/SortingButton.spec.tsxsrc/shared-components/posts/posts.spec.tsx
| setNativeInputValue(searchInput, 'Group'); | ||
|
|
||
| // Open modal | ||
| const viewButtons = screen.getAllByTestId('viewGroupBtn'); | ||
| expect(viewButtons.length).toBeGreaterThan(0); | ||
| // Wait for grid to reload after search change, then open modal | ||
| const viewButtons = await screen.findAllByTestId('viewGroupBtn'); | ||
| await waitFor(() => { | ||
| expect(viewButtons.length).toBeGreaterThan(0); | ||
| }); | ||
| await userEvent.click(viewButtons[0]); |
There was a problem hiding this comment.
Potential race condition: findAllByTestId resolves from initial state.
The findAllByTestId('viewGroupBtn') at line 681 resolves immediately against the pre-search state (2 buttons). The subsequent waitFor on lines 682-684 checks an already-resolved array, so the test may pass even if the search refetch never completes.
Since this test verifies search state persistence across modal open/close, consider waiting for a search-specific indicator before proceeding:
Suggested fix
const searchInput = screen.getByTestId('searchByInput') as HTMLInputElement;
setNativeInputValue(searchInput, 'Group');
- // Wait for grid to reload after search change, then open modal
- const viewButtons = await screen.findAllByTestId('viewGroupBtn');
- await waitFor(() => {
- expect(viewButtons.length).toBeGreaterThan(0);
- });
+ // Wait for search to complete - verify data grid reflects search
+ await waitFor(() => {
+ expect(screen.getAllByTestId('viewGroupBtn').length).toBeGreaterThan(0);
+ });
+ const viewButtons = screen.getAllByTestId('viewGroupBtn');
await userEvent.click(viewButtons[0]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx` around lines 678 -
685, The test currently calls findAllByTestId('viewGroupBtn') which can resolve
against the initial pre-search state and makes the following waitFor
ineffective; after calling setNativeInputValue(searchInput, 'Group') you should
instead wait for a search-specific update (e.g., wait for the grid to reflect
the filtered results) before capturing the view buttons—use waitFor with
getAllByTestId('viewGroupBtn') (or await screen.findByText / another unique
result indicator) to assert the expected post-search count/state, then call
userEvent.click on the resolved button; update references around
setNativeInputValue, searchInput, viewGroupBtn, waitFor, and userEvent.click
accordingly.
- Fix race condition in UserGlobalScreen toggle-back flow (findByTestId) - Add cleanup() and waitFor wrappers in SortingButton spec - Add observable grid assertion in Groups search submit test - Verify mode-specific output in Organizations mode-switch test
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx (1)
680-684:⚠️ Potential issue | 🟠 MajorFlaky search assertions still allow false positives in two updated paths
Issue Goals
- For
#7546, the test should prove async search state has settled.- Line 681 currently can resolve from pre-search UI, and Line 1475/Line 1478 asserts immediately after an async key event.
Tests (incl. Flakiness)
- Line 680–684:
findAllByTestId+ asserting cachedviewButtons.lengthdoes not verify post-search refetch state.- Line 1475–1478: assertions after
await userEvent.keyboard('{Enter}')are outsidewaitFor, so they are race-prone in sharded CI.Components/Policy
- No component-policy issues in this diff.
GraphQL
- Current checks can pass even if Enter-submit/search refetch regresses, because observable filtered results are not awaited.
i18n & a11y
- No i18n/a11y issue in these changed lines.
Security
- No security issue in these changed lines.
Action Items
Suggested stabilization patch
@@ - // Wait for grid to reload after search change, then open modal - const viewButtons = await screen.findAllByTestId('viewGroupBtn'); - await waitFor(() => { - expect(viewButtons.length).toBeGreaterThan(0); - }); + // Wait for post-search grid state, then open modal + await waitFor(() => { + expect(screen.getAllByTestId('viewGroupBtn').length).toBeGreaterThan(0); + }); + const viewButtons = screen.getAllByTestId('viewGroupBtn'); await userEvent.click(viewButtons[0]); @@ setNativeInputValue(searchInput, 'Group 1'); await userEvent.keyboard('{Enter}'); - - // Verify search is handled - expect(searchInput).toHaveValue('Group 1'); - - // Verify the grid is still rendered after search submit - expect(screen.getByRole('grid')).toBeInTheDocument(); + await waitFor(() => { + expect(searchInput).toHaveValue('Group 1'); + expect(screen.getByRole('grid')).toBeInTheDocument(); + expect(screen.getByText('Group 1')).toBeInTheDocument(); + expect(screen.queryByText('Group 2')).not.toBeInTheDocument(); + });As per coding guidelines, “expect(...) after await userEvent… not inside waitFor” is a blocking race pattern, and async UI transitions must be validated with condition-based waits.
Also applies to: 1472-1478
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx` around lines 680 - 684, The current test uses screen.findAllByTestId('viewGroupBtn') and then asserts on the cached viewButtons.length and also performs assertions immediately after userEvent.keyboard('{Enter}'), which can observe pre-search state and cause flakes; change both checks to wait for the post-search DOM state instead: replace the cached-length assertion with a waitFor(() => expect(screen.queryAllByTestId('viewGroupBtn').length).toBeGreaterThan(0)) to ensure the refetched grid has rendered, and wrap any assertions that follow userEvent.keyboard('{Enter}') inside waitFor conditions that verify the expected DOM change (e.g., presence/absence or count of 'viewGroupBtn' or the specific result node) rather than asserting immediately after userEvent.keyboard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx`:
- Line 568: The test currently named "should search organizations when pressing
Enter key with keyboard helper" does not assert Enter-key submission but instead
verifies mode switching and selected-mode styling; rename the test declaration
(the test(...) call in Organizations.spec.tsx) to something like "should switch
search mode and apply selected-mode styling" so the title matches the assertions
(references: the test(...) invocation around the modal/mode switching assertions
and any assertions checking selected-mode class/styling).
- Line 560: Replace the focus-dependent calls to userEvent.keyboard('{Enter}')
with explicit input-targeted submissions using userEvent.type(searchInput,
'{Enter}') to match the pattern used at line 531; locate each occurrence of
userEvent.keyboard('{Enter}') (six instances referenced around lines 560, 755,
1724, 1812, 2053, 2157) and change them to call userEvent.type with the same
searchInput element used elsewhere in the test suite so the Enter key is sent
directly to the input rather than relying on focus.
---
Duplicate comments:
In `@src/screens/UserPortal/Volunteer/Groups/Groups.spec.tsx`:
- Around line 680-684: The current test uses
screen.findAllByTestId('viewGroupBtn') and then asserts on the cached
viewButtons.length and also performs assertions immediately after
userEvent.keyboard('{Enter}'), which can observe pre-search state and cause
flakes; change both checks to wait for the post-search DOM state instead:
replace the cached-length assertion with a waitFor(() =>
expect(screen.queryAllByTestId('viewGroupBtn').length).toBeGreaterThan(0)) to
ensure the refetched grid has rendered, and wrap any assertions that follow
userEvent.keyboard('{Enter}') inside waitFor conditions that verify the expected
DOM change (e.g., presence/absence or count of 'viewGroupBtn' or the specific
result node) rather than asserting immediately after userEvent.keyboard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab896245-e3d8-4c92-ab1d-7663b099f188
📒 Files selected for processing (4)
src/screens/UserPortal/Organizations/Organizations.spec.tsxsrc/screens/UserPortal/UserGlobalScreen/UserGlobalScreen.spec.tsxsrc/screens/UserPortal/Volunteer/Groups/Groups.spec.tsxsrc/shared-components/SortingButton/SortingButton.spec.tsx
|
|
||
| const searchButton = screen.getByTestId('searchBtn'); | ||
| await userEvent.click(searchButton); | ||
| await userEvent.keyboard('{Enter}'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 50 src/screens/UserPortal/Organizations/Organizations.spec.tsx | tail -n 30Repository: PalisadoesFoundation/talawa-admin
Length of output: 949
🏁 Script executed:
# First, check if the file exists and get size
wc -l src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 141
🏁 Script executed:
# Get context around line 560 - check searchInput definition and usage
sed -n '540,580p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 1198
🏁 Script executed:
# Check around line 755
sed -n '740,770p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 980
🏁 Script executed:
# Search for searchInput definition in the file
rg -n 'searchInput\s*=' src/screens/UserPortal/Organizations/Organizations.spec.tsx | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 527
🏁 Script executed:
# Check how searchInput is initially queried and what type it is
rg -n 'searchInput' src/screens/UserPortal/Organizations/Organizations.spec.tsx | head -30Repository: PalisadoesFoundation/talawa-admin
Length of output: 1573
🏁 Script executed:
# Get exact lines 560, 755, 1724, 1812, 2053, 2157 to confirm the current code
sed -n '560p;755p;1724p;1812p;2053p;2157p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 310
🏁 Script executed:
# Get broader context for line 560 to see the test structure
sed -n '555,565p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 408
🏁 Script executed:
# Check for all instances of userEvent.keyboard in the file
rg -n 'userEvent\.keyboard' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 338
🏁 Script executed:
# Check for all instances of userEvent.type with Enter key
rg -n "userEvent\.type.*Enter" src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 128
🏁 Script executed:
# Get full context around line 531 to compare with line 560
sed -n '525,535p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 493
🏁 Script executed:
# Verify the pattern at all 6 locations to confirm searchInput is defined before each keyboard call
sed -n '555,562p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---" && \
sed -n '750,757p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---" && \
sed -n '1720,1727p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---" && \
sed -n '1808,1815p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---" && \
sed -n '2049,2056p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---" && \
sed -n '2153,2160p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 1782
🏁 Script executed:
# Check test names around these lines to understand test intent
rg -B2 "userEvent\.keyboard\(\'{Enter}\'\)" src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 198
🏁 Script executed:
# Check test names around these lines
rg -B2 "userEvent.keyboard" src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 772
🏁 Script executed:
# Check test names/descriptions for the 6 keyboard calls
sed -n '520,562p' src/screens/UserPortal/Organizations/Organizations.spec.tsx | grep -A1 "test("Repository: PalisadoesFoundation/talawa-admin
Length of output: 172
🏁 Script executed:
# Get test names for all 6 keyboard locations
sed -n '520,530p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---560---" && \
sed -n '746,758p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---755---" && \
sed -n '1716,1728p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---1724---" && \
sed -n '1804,1816p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---1812---" && \
sed -n '2045,2060p' src/screens/UserPortal/Organizations/Organizations.spec.tsx && echo "---2053---" && \
sed -n '2149,2165p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 3066
🏁 Script executed:
# Verify that line 531 exists and uses the suggested pattern
sed -n '530,532p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 177
🏁 Script executed:
# Check if there are any comments or context around why line 531 uses type() instead of keyboard()
sed -n '520,535p' src/screens/UserPortal/Organizations/Organizations.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 569
Align Enter key submission with line 531 pattern for consistency and stability.
userEvent.keyboard('{Enter}') relies on focus state and is inconsistent with line 531, which correctly uses userEvent.type(searchInput, '{Enter}'). Replace all 6 instances to explicitly target the input element, preventing focus-dependency flakiness.
Affected lines
Line 560, 755, 1724, 1812, 2053, 2157
Fix: Change await userEvent.keyboard('{Enter}') to await userEvent.type(searchInput, '{Enter}') at each location.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx` at line 560,
Replace the focus-dependent calls to userEvent.keyboard('{Enter}') with explicit
input-targeted submissions using userEvent.type(searchInput, '{Enter}') to match
the pattern used at line 531; locate each occurrence of
userEvent.keyboard('{Enter}') (six instances referenced around lines 560, 755,
1724, 1812, 2053, 2157) and change them to call userEvent.type with the same
searchInput element used elsewhere in the test suite so the Enter key is sent
directly to the input rather than relying on focus.
| }); | ||
|
|
||
| test('Mode dropdown switches list correctly', async () => { | ||
| test('should search organizations when pressing Enter key with keyboard helper', async () => { |
There was a problem hiding this comment.
Rename the test to match what it actually validates.
Line 568 says this test is about “pressing Enter key with keyboard helper,” but the body verifies mode switching and selected-mode styling (no Enter submission path). Please rename to reflect its true purpose.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/UserPortal/Organizations/Organizations.spec.tsx` at line 568, The
test currently named "should search organizations when pressing Enter key with
keyboard helper" does not assert Enter-key submission but instead verifies mode
switching and selected-mode styling; rename the test declaration (the test(...)
call in Organizations.spec.tsx) to something like "should switch search mode and
apply selected-mode styling" so the title matches the assertions (references:
the test(...) invocation around the modal/mode switching assertions and any
assertions checking selected-mode class/styling).
|
@coderabbitai Examine the failing Github actions and give insights on what changes need to be made to make them succeed. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1058 🏁 Script executed: Length of output: 1314 🏁 Script executed: Length of output: 53994 🏁 Script executed: Length of output: 12084 🏁 Script executed: Length of output: 12577
🔴 Critical Blocker: Sensitive File CheckJob: The CI policy flags
This must be authorized by a maintainer before the run can pass. The Vite config changes in this PR ( 🔴 Category 1:
|
| File | Symptom | Likely Root Cause |
|---|---|---|
OrganizationDashboard.spec.tsx |
"correctly displays pending membership requests" | MockedProvider timing / query variable mismatch |
ChatRoom.spec.tsx |
Mock exhaustion for UpdateChatMessage/Chat |
Pre-existing mock count issue |
EventManager.spec.ts |
uninstallPlugin is not a function |
Plugin manager API changed |
These should be addressed in a separate PR/issue.
Summary Table
| Shard | File(s) | Fix needed |
|---|---|---|
| 6 | SidebarBase.spec.tsx, FundCampaignPledge.spec.tsx, Invitations.spec.tsx |
Add testids/CSS in component; Enter key for search |
| 4 | Pledges.spec.tsx, SidebarOrgSection.spec.tsx |
Enter key; fix loading/error/icon in component |
| 1 | AddMember.spec.tsx |
Mock Toolbar or update testid references |
| 5 | EventManager.spec.ts (pre-existing) |
Separate fix needed |
| 8 | Users.spec.tsx |
where: undefined → null |
| 2 | SearchFilterBar.spec.tsx |
Fix SortingButton mock prop name + restore sort-select testid |
| 3 | SubTags.spec.tsx, OrganizationDashboard.spec.tsx |
Enter key; separate fix |
| 11 | Advertisements.spec.tsx, VolunteerGroups.spec.tsx |
Enter key for search |
| 9 | ChatRoom.spec.tsx (pre-existing) |
Separate fix needed |
| 7 | ChatRoom.spec.tsx (pre-existing) |
Separate fix needed |
| — | config/vite.config.ts |
Needs maintainer authorization |
Fix 7 more test files broken by the Toolbar/SearchFilterBar refactoring: - SubTags, Pledges, Advertisements, Users: replace searchBtn clicks with Enter key - SearchFilterBar: replace selectOptions with click-based dropdown interaction - Navbar: update icon prop to type prop in sorting config - SidebarBase: fix logo testid and hamburger CSS class assertions
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared-components/SidebarBase/SidebarBase.spec.tsx (1)
63-69:⚠️ Potential issue | 🔴 Critical❌ BLOCKING test-stability violations must be fixed before merge
Test Cleanup (Critical for Sharded CI)
- Line 64: Remove
vi.clearAllMocks()frombeforeEach. This pattern causes incomplete cleanup between tests in parallel shards.- Lines 67-69: Add
cleanup()call toafterEach. Current implementation is missing required cleanup from@testing-library/react. Must contain bothcleanup()ANDvi.restoreAllMocks().Async Assertion Race Conditions (Critical)
Lines 139–197: Assertions placed immediately after
await userEvent.click()instead of insidewaitFor()blocks. This causes flakiness in sharded CI when state changes are not fully propagated before assertions run.Affected test cases:
- Line 139–140:
setHideDrawertoggle test- Line 146–147:
setHideDrawertoggle test- Line 164–165: localStorage check test
- Line 180–181: localStorage check test
- Line 196–197: localStorage check test
Move each
expect()call intowaitFor()after correspondingawait userEvent.click().Suggested Pattern
afterEach(() => { cleanup(); vi.restoreAllMocks(); }); // For each test: await userEvent.click(toggleBtn); await waitFor(() => { expect(mockSetHideDrawer).toHaveBeenCalledWith(true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared-components/SidebarBase/SidebarBase.spec.tsx` around lines 63 - 69, Remove the vi.clearAllMocks() call from beforeEach, update afterEach to call cleanup() followed by vi.restoreAllMocks(), and update the tests that assert immediately after await userEvent.click(...) (the tests that use mockSetHideDrawer and the localStorage assertions) so each expect(...) is placed inside a waitFor(...) callback after the click (e.g., await userEvent.click(toggleBtn); await waitFor(() => { expect(mockSetHideDrawer).toHaveBeenCalledWith(true); });), ensuring all assertions that depend on async state updates use waitFor.src/components/AdminPortal/Advertisements/Advertisements.spec.tsx (1)
379-388:⚠️ Potential issue | 🔴 Critical❌ Blocking race condition after Enter-based search submissions.
After async search submission, assertions are not consistently condition-waited (fixed
wait()or immediateexpect). This is flaky in shard CI.Proposed fix pattern (apply to all updated search tests)
- await userEvent.keyboard('{Enter}'); - - await wait(); - expect(screen.getByText('Cookie shop 6')).toBeInTheDocument(); - expect(screen.queryByText('Cookie shop 5')).not.toBeInTheDocument(); + await userEvent.keyboard('{Enter}'); + await waitFor(() => { + expect(screen.getByText('Cookie shop 6')).toBeInTheDocument(); + expect(screen.queryByText('Cookie shop 5')).not.toBeInTheDocument(); + });- await userEvent.keyboard('{Enter}'); - expect( - screen.getAllByText('Ads not present for this campaign.'), - ).toHaveLength(2); + await userEvent.keyboard('{Enter}'); + await waitFor(() => { + expect( + screen.getAllByText('Ads not present for this campaign.'), + ).toHaveLength(2); + });As per coding guidelines: “ALL assertions after async operations MUST be inside waitFor blocks” and “NEVER use fixed time delays in tests.”
Also applies to: 415-424, 447-456, 482-491, 514-518
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/AdminPortal/Advertisements/Advertisements.spec.tsx` around lines 379 - 388, The test performs an Enter-based search by calling userEvent.keyboard('{Enter}') then uses a bare wait() and immediate expects, which causes a race; replace the fixed wait() and immediate assertions with a proper asynchronous waitFor block (from `@testing-library/react`) that contains the expects so the DOM assertions (screen.getByText('Cookie shop 6'), screen.queryByText('Cookie shop 5'..'Cookie shop 1')) only run after the async search completes; apply the same pattern to the other similar tests that call userEvent.keyboard('{Enter}') (the groups around the current expect lines and the ranges noted in the comment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/AdminPortal/SubTags/SubTags.spec.tsx`:
- Around line 4-11: The test uses a fragile hardcoded helper wait() and mixes
fireEvent.change() with user.keyboard(), causing flaky timing; remove the wait
helper (symbol: wait) and replace all await wait() usages with deterministic
async queries or waitFor() (e.g., screen.findByPlaceholderText,
screen.findByText) so assertions await the actual DOM conditions, and replace
fireEvent.change(...) usages with userEvent APIs (e.g., await user.type(...) or
await user.clear(...) + user.type(...)) to keep interactions consistent with
user.keyboard(); finally, remove fireEvent from the imports list to avoid
accidental usage and update tests referencing fireEvent.change to use the
userEvent equivalents (look for occurrences around the user.keyboard() call and
tests that previously awaited wait()).
In `@src/shared-components/Navbar/Navbar.spec.tsx`:
- Around line 77-82: Fix the JSX formatting and the inaccurate test: collapse
the JSX attributes for the icon node so the element uses inline attributes on a
single line (the div with data-testid={`${dataTestIdPrefix}-icon`} and
data-icon-type={type}) to satisfy Prettier, and update the test that claims
“type provided” (the spec block around lines 152-161) to actually pass an
explicit type prop into the rendered component (or else rename the test to state
it checks default behavior) so the provided-type path is exercised; search for
dataTestIdPrefix, data-testid `${dataTestIdPrefix}-icon`, and the spec block
referencing “type provided” to locate and change the code.
In `@src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx`:
- Around line 72-77: The spec has several race and policy issues: update the
afterEach block to call cleanup() and vi.restoreAllMocks(); ensure every
assertion that follows an async user action (e.g., after userEvent.click in
tests around the blocks referencing the dropdown/menu interactions) is wrapped
in waitFor; before querying/clicking option item testids (tests that toggle the
menu and then select options) await the menu visibility (e.g., await findByRole
or waitFor(() => expect(menu).toBeVisible())) so selection isn’t raced; replace
raw renders with the project helper that wraps components in I18nextProvider
(use the test render helper or renderWithI18next used elsewhere) instead of the
bare react-i18next mock; and fix the formatting violation noted near the
prettier warning by reformatting the affected code in the spec.
---
Outside diff comments:
In `@src/components/AdminPortal/Advertisements/Advertisements.spec.tsx`:
- Around line 379-388: The test performs an Enter-based search by calling
userEvent.keyboard('{Enter}') then uses a bare wait() and immediate expects,
which causes a race; replace the fixed wait() and immediate assertions with a
proper asynchronous waitFor block (from `@testing-library/react`) that contains
the expects so the DOM assertions (screen.getByText('Cookie shop 6'),
screen.queryByText('Cookie shop 5'..'Cookie shop 1')) only run after the async
search completes; apply the same pattern to the other similar tests that call
userEvent.keyboard('{Enter}') (the groups around the current expect lines and
the ranges noted in the comment).
In `@src/shared-components/SidebarBase/SidebarBase.spec.tsx`:
- Around line 63-69: Remove the vi.clearAllMocks() call from beforeEach, update
afterEach to call cleanup() followed by vi.restoreAllMocks(), and update the
tests that assert immediately after await userEvent.click(...) (the tests that
use mockSetHideDrawer and the localStorage assertions) so each expect(...) is
placed inside a waitFor(...) callback after the click (e.g., await
userEvent.click(toggleBtn); await waitFor(() => {
expect(mockSetHideDrawer).toHaveBeenCalledWith(true); });), ensuring all
assertions that depend on async state updates use waitFor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6bd3201-a05e-4ec3-a27a-2ece19da71de
📒 Files selected for processing (7)
src/components/AdminPortal/Advertisements/Advertisements.spec.tsxsrc/screens/AdminPortal/SubTags/SubTags.spec.tsxsrc/screens/AdminPortal/Users/Users.spec.tsxsrc/screens/UserPortal/Pledges/Pledges.spec.tsxsrc/shared-components/Navbar/Navbar.spec.tsxsrc/shared-components/SearchFilterBar/SearchFilterBar.spec.tsxsrc/shared-components/SidebarBase/SidebarBase.spec.tsx
| import { | ||
| act, | ||
| cleanup, | ||
| fireEvent, | ||
| render, | ||
| screen, | ||
| waitFor, | ||
| } from '@testing-library/react'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify restricted fireEvent usage in spec files and inspect Enter-triggered search sites
rg -n --type tsx '\bfireEvent\b|keyboard\(\s*[`'\'']\{Enter\}[`'\'']\s*\)'Repository: PalisadoesFoundation/talawa-admin
Length of output: 104
🏁 Script executed:
fd SubTags.spec.tsx --type fRepository: PalisadoesFoundation/talawa-admin
Length of output: 125
🏁 Script executed:
cat -n src/screens/AdminPortal/SubTags/SubTags.spec.tsx | head -50Repository: PalisadoesFoundation/talawa-admin
Length of output: 1963
🏁 Script executed:
sed -n '240,260p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 826
🏁 Script executed:
sed -n '235,265p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 1184
🏁 Script executed:
rg -n 'fireEvent' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 167
🏁 Script executed:
sed -n '1,30p' src/screens/AdminPortal/SubTags/SubTags.spec.tsx | grep -E 'import.*user|fireEvent'Repository: PalisadoesFoundation/talawa-admin
Length of output: 142
🏁 Script executed:
rg -n 'afterEach|cleanup\(\)|vi.restoreAllMocks|vi.clearAllMocks' src/screens/AdminPortal/SubTags/SubTags.spec.tsx | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 299
🏁 Script executed:
sed -n '135,150p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 466
🏁 Script executed:
sed -n '235,275p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 1602
🏁 Script executed:
grep -n 'wait()' src/screens/AdminPortal/SubTags/SubTags.spec.tsx | head -5Repository: PalisadoesFoundation/talawa-admin
Length of output: 186
🏁 Script executed:
sed -n '80,100p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 583
🏁 Script executed:
sed -n '139,145p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 261
🏁 Script executed:
grep -n "^const wait\|^function wait\|^export.*wait" src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 59
🏁 Script executed:
rg -n "import.*wait|const wait|function wait" src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 126
🏁 Script executed:
sed -n '61,65p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 220
🏁 Script executed:
sed -n '245,260p' src/screens/AdminPortal/SubTags/SubTags.spec.tsxRepository: PalisadoesFoundation/talawa-admin
Length of output: 707
❌ Critical: Replace hardcoded wait() function and fireEvent.change() with deterministic async patterns.
Two issues compound test flakiness in CI sharding:
-
Hardcoded delay (line 61): The
wait(ms = 500)helper usessetTimeout, creating race conditions in variable-latency CI. The multipleawait wait()calls throughout this file (e.g., lines 147, 155, 163) prevent deterministic test execution. -
Mixed userEvent + fireEvent (line 250): Using
fireEvent.change()withawait user.keyboard()is inconsistent and can fail if focus is lost between operations.
Proposed fixes
Fix 1 (line 61-66): Replace hardcoded wait helper:
-async function wait(ms = 500): Promise<void> {
- await act(() => {
- return new Promise((resolve) => {
- setTimeout(resolve, ms);
- });
- });
-}
+// Removed: Use waitFor/findBy queries instead for deterministic waitsThen replace all await wait() calls with waitFor() or async queries (e.g., screen.findByPlaceholderText()).
Fix 2 (lines 249-251): Replace fireEvent with userEvent:
- const input = screen.getByPlaceholderText(translations.searchByName);
- fireEvent.change(input, { target: { value: ' searchSubTag ' } });
+ const input = await screen.findByPlaceholderText(translations.searchByName);
+ await user.click(input);
+ await user.clear(input);
+ await user.paste(' searchSubTag ');
await user.keyboard('{Enter}');Also remove fireEvent from imports (line 7).
Also applies to: All await wait() calls (lines 147, 155, 163, 181, 193, and others). Replace with conditional async queries per guidelines.
🧰 Tools
🪛 ESLint
[error] 7-7: 'fireEvent' import from '@testing-library/react' is restricted. Tests in this file use fireEvent for user interactions; use userEvent from @testing-library/user-event instead.
(no-restricted-imports)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/AdminPortal/SubTags/SubTags.spec.tsx` around lines 4 - 11, The
test uses a fragile hardcoded helper wait() and mixes fireEvent.change() with
user.keyboard(), causing flaky timing; remove the wait helper (symbol: wait) and
replace all await wait() usages with deterministic async queries or waitFor()
(e.g., screen.findByPlaceholderText, screen.findByText) so assertions await the
actual DOM conditions, and replace fireEvent.change(...) usages with userEvent
APIs (e.g., await user.type(...) or await user.clear(...) + user.type(...)) to
keep interactions consistent with user.keyboard(); finally, remove fireEvent
from the imports list to avoid accidental usage and update tests referencing
fireEvent.change to use the userEvent equivalents (look for occurrences around
the user.keyboard() call and tests that previously awaited wait()).
| <div | ||
| data-testid={`${dataTestIdPrefix}-icon`} | ||
| data-icon-type={type} | ||
| > | ||
| {icon} | ||
| </div> |
There was a problem hiding this comment.
Structured review: one CI blocker and one test-intent gap in the updated sort-icon checks
Issue Goals
- The refactor objective to validate sort icon behavior is mostly covered, but the “provided type” path is not actually exercised: Line 152 says type is provided, while Lines 155-161 omit
type.
Tests (incl. Flakiness)
- ✅ No shard-flakiness anti-patterns in changed segments (cleanup + restore present; no fixed waits/silent catches).
Components/Policy
- ❌ CI-blocking lint issue at Lines 77-80: Prettier requires inline attributes for this JSX node.
GraphQL
- No issues in this segment.
i18n & a11y
- No issues in this segment.
Security
- No issues in this segment.
Action Items
- Fix JSX formatting at Lines 77-80.
- Make the “type provided” test truthful by adding explicit
typein Lines 155-161, or rename the test to reflect default behavior.
Suggested patch
- <div
- data-testid={`${dataTestIdPrefix}-icon`}
- data-icon-type={type}
- >
+ <div data-testid={`${dataTestIdPrefix}-icon`} data-icon-type={type}>
{icon}
</div> const sortingProps = [
{
title: 'Sort Custom',
options: [{ label: 'A', value: 'a' }],
selected: 'a',
onChange: mockSort,
testIdPrefix: 'sort-custom',
+ type: 'sort',
},
];Also applies to: 152-167, 169-184
🧰 Tools
🪛 ESLint
[error] 77-80: Replace ⏎··········data-testid={${dataTestIdPrefix}-icon}⏎··········data-icon-type={type}⏎········ with ·data-testid={${dataTestIdPrefix}-icon}·data-icon-type={type}
(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared-components/Navbar/Navbar.spec.tsx` around lines 77 - 82, Fix the
JSX formatting and the inaccurate test: collapse the JSX attributes for the icon
node so the element uses inline attributes on a single line (the div with
data-testid={`${dataTestIdPrefix}-icon`} and data-icon-type={type}) to satisfy
Prettier, and update the test that claims “type provided” (the spec block around
lines 152-161) to actually pass an explicit type prop into the rendered
component (or else rename the test to state it checks default behavior) so the
provided-type path is exercised; search for dataTestIdPrefix, data-testid
`${dataTestIdPrefix}-icon`, and the spec block referencing “type provided” to
locate and change the code.
| vi.mock('react-i18next', () => ({ | ||
| useTranslation: () => ({ | ||
| t: (key: string) => key, | ||
| i18n: { language: 'en' }, | ||
| }), | ||
| })); |
There was a problem hiding this comment.
🚫 Blocking: this spec still has shard-flakiness and policy violations despite the refactor fixes
Issue Goals
- The PR’s CI-stability intent is partially met, but there’s a remaining gap: async interaction assertions are still race-prone in this file (
src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx:295-301, 740-751, 796-801).
Tests (incl. Flakiness)
- INCOMPLETE CLEANUP at
src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx:107-109—afterEachis missingcleanup(). - RACE CONDITION at
src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx:301, 744, 751, 800— assertions run after async user actions but outsidewaitFor. - RACE CONDITION at
src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx:298-299, 742-743, 798-799— option items are queried/clicked immediately after toggle click without waiting for menu open state.
Components/Policy
src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx:560-562has a prettier violation already flagged by static analysis.
GraphQL
- No GraphQL-specific issue in this file.
i18n & a11y
src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx:72-77mocksreact-i18next, but renders are not wrapped inI18nextProvider. This violates the repository testing policy and can hide translation integration regressions.
Security
- No security findings in this file.
Action Items
- Add
cleanup()toafterEach. - Wrap post-interaction assertions in
waitFor. - Wait for dropdown/menu visibility before selecting item testids.
- Use an
I18nextProviderrender helper for this spec. - Apply the formatter fix at Line 560-562.
Suggested patch pattern
-import { render, screen } from '@testing-library/react';
+import { cleanup, render, screen, waitFor } from '@testing-library/react';
afterEach(() => {
+ cleanup();
vi.restoreAllMocks();
});
// Example pattern for dropdown interaction
const toggleButton = screen.getByTestId('sort-toggle');
await user.click(toggleButton);
-const option = screen.getByTestId('sort-item-ASCENDING');
+await waitFor(() =>
+ expect(screen.getByTestId('sort-item-ASCENDING')).toBeInTheDocument(),
+);
+const option = screen.getByTestId('sort-item-ASCENDING');
await user.click(option);
-expect(onOptionChange).toHaveBeenCalledWith('ASCENDING');
+await waitFor(() =>
+ expect(onOptionChange).toHaveBeenCalledWith('ASCENDING'),
+);
// Prettier
-expect(
- screen.getByTestId('filter-container'),
-).toBeInTheDocument();
+expect(screen.getByTestId('filter-container')).toBeInTheDocument();As per coding guidelines: afterEach in spec files must include both cleanup() and vi.restoreAllMocks(), all assertions after async user operations must be inside waitFor, and component tests must be wrapped with I18nextProvider.
Also applies to: 107-109, 295-301, 560-562, 740-751, 796-801
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared-components/SearchFilterBar/SearchFilterBar.spec.tsx` around lines
72 - 77, The spec has several race and policy issues: update the afterEach block
to call cleanup() and vi.restoreAllMocks(); ensure every assertion that follows
an async user action (e.g., after userEvent.click in tests around the blocks
referencing the dropdown/menu interactions) is wrapped in waitFor; before
querying/clicking option item testids (tests that toggle the menu and then
select options) await the menu visibility (e.g., await findByRole or waitFor(()
=> expect(menu).toBeVisible())) so selection isn’t raced; replace raw renders
with the project helper that wraps components in I18nextProvider (use the test
render helper or renderWithI18next used elsewhere) instead of the bare
react-i18next mock; and fix the formatting violation noted near the prettier
warning by reformatting the affected code in the spec.
palisadoes
added
the
ignore-sensitive-files-pr
|
This pull request did not get any activity in the past 10 days and will be closed in 360 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. |
2 participants
What kind of change does this PR introduce?
Bugfix — repairs pre-existing broken unit tests on the
developbranch that were failing after the Toolbar/SearchFilterBar component refactoring.Issue Number:
Fixes #7544
Fixes #7545
Fixes #7546
Snapshots/Videos:
N/A — test-only changes with no UI impact.
If relevant, did you update the documentation?
Not applicable — no user-facing or API changes.
Summary
After the unified Toolbar component replaced both
Navbar/PageHeaderandSearchFilterBar, several test files were left with stale mock targets, outdated CSS class assertions, and mismatcheddata-testidqueries. This PR repairs all six affected test files:SortingButton.spec.tsx— Added missingtypeprop to DropDownButton mock destructuring; updated a test that asserted a non-existenticonprop on SortingButton.UserGlobalScreen.spec.tsx— UpdatedtoHaveClass('pageContainer')assertions to'expand'/'contract'matching the refactored component.posts.spec.tsx— Retargeted mock fromshared-components/Navbar/Navbartoshared-components/Toolbar/Toolbarwith updated interface (sorting→filters).Organizations.spec.tsx— ReplacedgetByTestId('searchBtn')withuserEvent.keyboard('{Enter}')(Toolbar hides the search button); fixed initial data wait to useorganizations-list; corrected post-search assertions to expect refetched data.OrganizationPeople.spec.tsx— Retargeted mock fromSearchFilterBartoToolbarwith correctInterfaceToolbarPropstype.Groups.spec.tsx— Replaced alluserEvent.type(searchInput, ...)calls with an atomicsetNativeInputValuehelper to avoid per-character GraphQL queries in server-side search mode; added mock forname_contains: 'Group'; fixed assertion placement insidewaitForblocks.config/vite.config.ts— Wrappedvite-plugin-istanbulin aCYPRESS_COVERAGEenv check to prevent it from forcing sourcemaps on every module during dev; gated verbose proxy logging behindALLOW_LOGS=true.Does this PR introduce a breaking change?
No.
Checklist
CodeRabbit AI Review
Test Coverage
Other information
All 206 tests across the 6 affected spec files pass locally. The
setNativeInputValuehelper inGroups.spec.tsxuses the nativeHTMLInputElement.prototype.valuesetter +InputEventdispatch to set controlled input values atomically, avoiding the per-character query issue thatuserEvent.typecauses withserverSide: trueinDataGridWrapper.Have you read the contributing guide?
Yes
Summary by CodeRabbit
Tests
Chores