test: add component tests for 6 untested components - #215
Merged
Merged
Conversation
…sBar, OAuthButtons, DocumentPreviewModal, JdImportPanel Adds 64 new tests across 6 previously untested components (165 -> 229 total). - ErrorState: error message rendering, retry button, GraphQL error codes - LogoMark: SVG rendering, size/className props, accessibility - NavigationProgressBar: renders null, NProgress.done on unmount - OAuthButtons: Google/GitHub links, divider - DocumentPreviewModal: PDF/image/unsupported rendering, close/backdrop, links - JdImportPanel: expand/collapse, text/URL modes, auto-fill flow, error states
WalkthroughAdded frontend tests for six components. Coverage includes rendering, user interactions, asynchronous states, error handling, accessibility attributes, mocked integrations, and navigation lifecycle behaviour. ChangesFrontend component tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Preview deployments for this PR: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/__tests__/components/NavigationProgressBar.test.tsx (1)
41-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the pending-to-idle transition.
These tests only verify idle navigation and unmount cleanup. They do not verify the main lifecycle in
apps/web/src/components/NavigationProgressBar.tsx:NProgress.start()must run when the status changes from'idle'to'pending', andNProgress.done()must run when it changes back to'idle'. Add arerendertest for both transitions and assert exact call counts.Suggested test
+ it('starts and completes progress across navigation transitions', async () => { + const { rerender } = render(<NavigationProgressBar />); + + mockStatus = 'pending'; + await act(async () => { + rerender(<NavigationProgressBar />); + }); + expect(mockStart).toHaveBeenCalledTimes(1); + + mockStatus = 'idle'; + await act(async () => { + rerender(<NavigationProgressBar />); + }); + expect(mockDone).toHaveBeenCalledTimes(1); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/__tests__/components/NavigationProgressBar.test.tsx` around lines 41 - 46, Extend the NavigationProgressBar test suite with a rerender scenario that changes navigation status from idle to pending and back to idle. Assert exact call counts showing NProgress.start is called once on the pending transition and NProgress.done is called once when returning to idle, while preserving the existing idle and unmount cleanup tests.apps/web/src/__tests__/components/LogoMark.test.tsx (1)
38-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the brand-rendering assertion.
The test title says that it checks the blue rectangles and the checkmark path. It only checks that three
<rect>elements and a<path>exist. A colour or path-styling regression would pass. Assert thefill,opacity, and pathstrokevalues defined byapps/web/src/components/LogoMark.tsx(Lines 12-34).Suggested assertions
const rects = container.querySelectorAll('rect'); - expect(rects.length).toBe(3); + expect(rects).toHaveLength(3); + expect(Array.from(rects, (rect) => rect.getAttribute('fill'))).toEqual([ + '`#1d4ed8`', + '`#1d4ed8`', + '`#1d4ed8`', + ]); const path = container.querySelector('path'); expect(path).toBeInTheDocument(); + expect(path).toHaveAttribute('stroke', '`#ffffff`');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/__tests__/components/LogoMark.test.tsx` around lines 38 - 43, Strengthen the test named “contains the brand blue rectangles and checkmark path” by asserting each rendered rectangle’s defined fill and opacity values, and asserting the checkmark path’s defined stroke value from LogoMark. Keep the existing element-count and presence assertions while validating the actual brand styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/__tests__/components/DocumentPreviewModal.test.tsx`:
- Around line 95-100: Update the close button in DocumentPreviewModal to include
aria-label="Close preview", then replace the broad SVG-based button lookup in
the test with an accessible-name query for “Close preview” before clicking it
and asserting onClose is called.
---
Nitpick comments:
In `@apps/web/src/__tests__/components/LogoMark.test.tsx`:
- Around line 38-43: Strengthen the test named “contains the brand blue
rectangles and checkmark path” by asserting each rendered rectangle’s defined
fill and opacity values, and asserting the checkmark path’s defined stroke value
from LogoMark. Keep the existing element-count and presence assertions while
validating the actual brand styling.
In `@apps/web/src/__tests__/components/NavigationProgressBar.test.tsx`:
- Around line 41-46: Extend the NavigationProgressBar test suite with a rerender
scenario that changes navigation status from idle to pending and back to idle.
Assert exact call counts showing NProgress.start is called once on the pending
transition and NProgress.done is called once when returning to idle, while
preserving the existing idle and unmount cleanup tests.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 60e78dec-8185-4ea1-9c37-463064695e7e
📒 Files selected for processing (6)
apps/web/src/__tests__/components/DocumentPreviewModal.test.tsxapps/web/src/__tests__/components/ErrorState.test.tsxapps/web/src/__tests__/components/JdImportPanel.test.tsxapps/web/src/__tests__/components/LogoMark.test.tsxapps/web/src/__tests__/components/NavigationProgressBar.test.tsxapps/web/src/__tests__/components/OAuthButtons.test.tsx
Comment on lines
+95
to
+100
| // The X button | ||
| const buttons = screen.getAllByRole('button'); | ||
| const closeButton = buttons.find((b) => b.querySelector('svg')); | ||
| expect(closeButton).toBeDefined(); | ||
| fireEvent.click(closeButton!); | ||
| expect(onClose).toHaveBeenCalledOnce(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add an accessible name to the close button.
DocumentPreviewModal renders the close button with only XIcon. The button has no accessible name. This test selects any button containing an SVG, so it passes without verifying accessible operation.
Add aria-label="Close preview" to the component button. Query that name in this test.
Proposed fix
- const buttons = screen.getAllByRole('button');
- const closeButton = buttons.find((b) => b.querySelector('svg'));
- expect(closeButton).toBeDefined();
- fireEvent.click(closeButton!);
+ fireEvent.click(screen.getByRole('button', { name: 'Close preview' }));- <button type="button" onClick={onClose} className="p-1.5 ...">
+ <button type="button" aria-label="Close preview" onClick={onClose} className="p-1.5 ...">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/__tests__/components/DocumentPreviewModal.test.tsx` around lines
95 - 100, Update the close button in DocumentPreviewModal to include
aria-label="Close preview", then replace the broad SVG-based button lookup in
the test with an accessible-name query for “Close preview” before clicking it
and asserting onClose is called.
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds 64 new tests across 6 previously untested components, bringing total from 165 to 229.
New test files
Summary by CodeRabbit