set up vitest + RTL, added Navbar tests#277
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Vitest and React Testing Library to the frontend (scripts, deps, vite test config, setup file) and a comprehensive Navbar test suite; updates backend test script/specs and refactors the User model pre-save hook to an async implementation. ChangesTest Infrastructure & Backend Adjustments
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @parinaB for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/components/__test__/Navbar.test.tsx (4)
69-70: ⚡ Quick winReplace brittle link selector based on array position.
Lines 69–70 select the mobile "Home" link by getting all links with that name and clicking the last one (
homeLinks[homeLinks.length - 1]). This assumes the mobile link is always last, which is fragile and will break if link order changes.Consider using a scoped query within the mobile menu container or adding a test ID to distinguish desktop from mobile links.
♻️ Example with scoped query
If the mobile menu has a container with a test ID or role:
fireEvent.click(hamburger) // open -const homeLinks = screen.getAllByRole('link', { name: /home/i }) -fireEvent.click(homeLinks[homeLinks.length - 1]) // click the mobile one +const mobileMenu = screen.getByRole('navigation', { name: /mobile/i }) +const mobileHomeLink = within(mobileMenu).getByRole('link', { name: /home/i }) +fireEvent.click(mobileHomeLink) expect(screen.queryByText('About')).not.toBeInTheDocument() // closedYou'll need to import
withinfrom@testing-library/react.🤖 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 `@src/components/__test__/Navbar.test.tsx` around lines 69 - 70, The test currently finds the mobile "Home" link by array position (homeLinks = screen.getAllByRole('link', { name: /home/i }) and clicking homeLinks[homeLinks.length - 1]), which is brittle; change the test to scope the query to the mobile menu container (e.g., use within on the mobile menu element or a specific data-testid for the mobile link) and then query that scoped container for the "Home" link, or add a distinct test id to the mobile link and use getByTestId; remember to import within from `@testing-library/react` if you use scoped queries and update the click to target the scoped/mobile element instead of indexing the whole list.
40-41: ⚡ Quick winReplace brittle array-index selector with a specific query.
Lines 40 and 46 select the theme button using
screen.getAllByRole('button')[0], which assumes the theme button is always the first button in the DOM. This will break if button order changes.Consider querying by accessible name or adding an
aria-labelto the theme button in the component.♻️ Example with aria-label
If the Navbar component adds an
aria-labelto the theme button:<button aria-label="Toggle theme" onClick={toggleTheme}> {mode === 'light' ? <Moon /> : <Sun />} </button>Then update the test:
-const themeBtn = screen.getAllByRole('button')[0] +const themeBtn = screen.getByRole('button', { name: /toggle theme/i }) expect(themeBtn).toBeInTheDocument()Also applies to: 46-46
🤖 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 `@src/components/__test__/Navbar.test.tsx` around lines 40 - 41, The test is using a brittle index-based selector for the theme button; update Navbar and its tests so the button has an accessible name (e.g., add aria-label="Toggle theme" on the theme toggle in the Navbar component where toggleTheme/mode/Moon/Sun are used) and replace screen.getAllByRole('button')[0] in Navbar.test.tsx with a specific query such as screen.getByRole('button', { name: /toggle theme/i }) or screen.getByLabelText('Toggle theme') to reliably target the themeBtn.
2-2: ⚡ Quick winPrefer
userEventoverfireEventfor more realistic interactions.The test suite uses
fireEvent.clickthroughout (lines 47, 60, 68, 70, 77, 78), but React Testing Library best practices recommenduserEventfor simulating actual browser behavior—including focus, hover, and timing—rather than synchronous DOM event dispatch.Since
@testing-library/user-eventis already installed (per PR objectives), consider migrating to it for higher-fidelity tests.♻️ Example migration for one test
-import { render, screen, fireEvent } from '`@testing-library/react`' +import { render, screen } from '`@testing-library/react`' +import userEvent from '`@testing-library/user-event`' import { describe, it, expect, vi, beforeEach } from 'vitest'Then in a test:
-it('calls toggleTheme when the theme button is clicked', () => { +it('calls toggleTheme when the theme button is clicked', async () => { const { toggleTheme } = renderNavbar('light') const themeBtn = screen.getAllByRole('button')[0] - fireEvent.click(themeBtn) + await userEvent.click(themeBtn) expect(toggleTheme).toHaveBeenCalledTimes(1) })Also applies to: 47-47, 60-60, 68-68, 70-70, 77-77, 78-78
🤖 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 `@src/components/__test__/Navbar.test.tsx` at line 2, The tests import and use fireEvent for clicks; replace fireEvent with `@testing-library/user-event` to simulate interactions more realistically: add import of userEvent from '`@testing-library/user-event`', call const user = userEvent.setup() inside each test (or a shared beforeEach), and replace fireEvent.click(...) with await user.click(...) (and add async to tests where needed). Update all occurrences of fireEvent.click in the Navbar.test.tsx tests (the click calls used around the render assertions and handlers) to use user.click with appropriate awaits so focus/timing behavior is preserved.
59-59: ⚡ Quick winReplace brittle array-index selector for the hamburger button.
Lines 59, 67, and 76 select the hamburger button using
screen.getAllByRole('button')[1], which assumes it's always the second button. This will break if button order changes.Consider adding an accessible label to the hamburger button in the component and querying by name.
♻️ Example with aria-label
If the hamburger button in Navbar has an
aria-label:<button aria-label="Toggle menu" onClick={toggleMenu}> <Menu /> </button>Then update the tests:
-const hamburger = screen.getAllByRole('button')[1] +const hamburger = screen.getByRole('button', { name: /toggle menu/i }) fireEvent.click(hamburger)Also applies to: 67-67, 76-76
🤖 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 `@src/components/__test__/Navbar.test.tsx` at line 59, Tests use a brittle array-index selector (screen.getAllByRole('button')[1]) to find the hamburger; update the Navbar component to give the hamburger button an accessible name (e.g., add aria-label="Toggle menu" on the button that renders the Menu icon / calls toggleMenu) and change the tests (the places that create the hamburger variable in Navbar.test.tsx around the current selectors) to query by role+name such as getByRole('button', { name: /toggle menu/i }) instead of index-based access so the selector is stable if button order changes.
🤖 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.
Nitpick comments:
In `@src/components/__test__/Navbar.test.tsx`:
- Around line 69-70: The test currently finds the mobile "Home" link by array
position (homeLinks = screen.getAllByRole('link', { name: /home/i }) and
clicking homeLinks[homeLinks.length - 1]), which is brittle; change the test to
scope the query to the mobile menu container (e.g., use within on the mobile
menu element or a specific data-testid for the mobile link) and then query that
scoped container for the "Home" link, or add a distinct test id to the mobile
link and use getByTestId; remember to import within from `@testing-library/react`
if you use scoped queries and update the click to target the scoped/mobile
element instead of indexing the whole list.
- Around line 40-41: The test is using a brittle index-based selector for the
theme button; update Navbar and its tests so the button has an accessible name
(e.g., add aria-label="Toggle theme" on the theme toggle in the Navbar component
where toggleTheme/mode/Moon/Sun are used) and replace
screen.getAllByRole('button')[0] in Navbar.test.tsx with a specific query such
as screen.getByRole('button', { name: /toggle theme/i }) or
screen.getByLabelText('Toggle theme') to reliably target the themeBtn.
- Line 2: The tests import and use fireEvent for clicks; replace fireEvent with
`@testing-library/user-event` to simulate interactions more realistically: add
import of userEvent from '`@testing-library/user-event`', call const user =
userEvent.setup() inside each test (or a shared beforeEach), and replace
fireEvent.click(...) with await user.click(...) (and add async to tests where
needed). Update all occurrences of fireEvent.click in the Navbar.test.tsx tests
(the click calls used around the render assertions and handlers) to use
user.click with appropriate awaits so focus/timing behavior is preserved.
- Line 59: Tests use a brittle array-index selector
(screen.getAllByRole('button')[1]) to find the hamburger; update the Navbar
component to give the hamburger button an accessible name (e.g., add
aria-label="Toggle menu" on the button that renders the Menu icon / calls
toggleMenu) and change the tests (the places that create the hamburger variable
in Navbar.test.tsx around the current selectors) to query by role+name such as
getByRole('button', { name: /toggle menu/i }) instead of index-based access so
the selector is stable if button order changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46bba6f7-8ac8-4a3a-8ec4-ccd05096c876
📒 Files selected for processing (4)
package.jsonsrc/components/__test__/Navbar.test.tsxsrc/setupTests.tsvite.config.ts
|
Hey, while working on the frontend test setup (#275), I investigated the failing backend tests and fixed them too. Here's a full breakdown of what was wrong and what I changed: Issues Found & Fixed1 — No test runner configuredThe backend had no way to actually run the spec files. Fix: Installed 2 — Deprecated Mongoose connection options (
|
|
🎉🎉 Thank you for your contribution! Your PR #277 has been merged! 🎉🎉 |
2 participants
Related Issue
Description
The frontend had zero test coverage — no test runner, no configuration, no test files. This PR sets up the complete testing infrastructure and adds the first test suite for the
Navbarcomponent.Changes made:
vitest,@testing-library/react,@testing-library/user-event,@testing-library/jest-dom,jsdomas dev dependenciestestblock tovite.config.tswithjsdomenvironmentsrc/setupTests.tsto load jest-dom matchers"test": "vitest"script topackage.jsonsrc/components/__test__/Navbar.test.tsxHow Has This Been Tested?
Ran
npm testlocally. All 9 tests pass.Tests cover:
toggleThemeon clickThemeContextis missingScreenshots (if applicable)
Type of Change
Summary by CodeRabbit