-
Notifications
You must be signed in to change notification settings - Fork 0
create header test file #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,56 @@ | ||||||||||||||||||||||||||||||||||||||
| import { render, screen, fireEvent } from "@testing-library/react"; | ||||||||||||||||||||||||||||||||||||||
| import { describe, it, expect, vi } from "vitest"; | ||||||||||||||||||||||||||||||||||||||
| import Header from "./Header"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| describe("Header component", () => { | ||||||||||||||||||||||||||||||||||||||
| const mockLogout = vi.fn(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const defaultProps = { | ||||||||||||||||||||||||||||||||||||||
| user: { name: "Karuna" }, | ||||||||||||||||||||||||||||||||||||||
| onLogout: mockLogout, | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add The mock function is defined at the suite level and could accumulate calls across tests, leading to test pollution. Reset mocks between tests for isolation. 🔎 Proposed fix describe("Header component", () => {
const mockLogout = vi.fn();
const defaultProps = {
user: { name: "Karuna" },
onLogout: mockLogout,
};
+
+ beforeEach(() => {
+ vi.clearAllMocks();
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("renders the navbar container", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header {...defaultProps} />); | ||||||||||||||||||||||||||||||||||||||
| expect(screen.getByRole("banner")).toBeInTheDocument(); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update test name to start with 'should'. Test names should start with 'should' according to coding guidelines for better readability and consistency. 🔎 Proposed fix- it("renders the navbar container", () => {
+ it("should render the navbar container", () => {Based on coding guidelines: "Use descriptive test names starting with 'should'" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("renders the app title", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header {...defaultProps} />); | ||||||||||||||||||||||||||||||||||||||
| expect(screen.getByText("daisyUI")).toBeInTheDocument(); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update test name to start with 'should'. Test names should start with 'should' according to coding guidelines. 🔎 Proposed fix- it("renders the app title", () => {
+ it("should render the app title", () => {Based on coding guidelines: "Use descriptive test names starting with 'should'" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("renders the hamburger menu link", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header {...defaultProps} />); | ||||||||||||||||||||||||||||||||||||||
| const menuLink = screen.getAllByRole("link")[0]; | ||||||||||||||||||||||||||||||||||||||
| expect(menuLink).toHaveAttribute("href", "/"); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use accessible name to query the hamburger menu link and update test name. Two issues:
🔎 Proposed fix- it("renders the hamburger menu link", () => {
+ it("should render the hamburger menu link", () => {
render(<Header {...defaultProps} />);
- const menuLink = screen.getAllByRole("link")[0];
+ const menuLink = screen.getByRole("link", { name: /menu/i });
expect(menuLink).toHaveAttribute("href", "/");
});Note: Adjust the accessible name pattern based on the actual implementation. Based on coding guidelines: "Use semantic queries in order of preference: getByRole, getByLabelText, getByPlaceholderText, getByText, getByTestId (last resort)" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("renders profile button", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header {...defaultProps} />); | ||||||||||||||||||||||||||||||||||||||
| expect(screen.getByRole("button")).toBeInTheDocument(); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use accessible name to query the profile button and update test name. Two issues:
🔎 Proposed fix- it("renders profile button", () => {
+ it("should render profile button", () => {
render(<Header {...defaultProps} />);
- expect(screen.getByRole("button")).toBeInTheDocument();
+ expect(screen.getByRole("button", { name: /profile/i })).toBeInTheDocument();
});Note: Adjust the accessible name pattern based on the actual implementation. Based on coding guidelines: "Use semantic queries in order of preference: getByRole, getByLabelText, getByPlaceholderText, getByText, getByTestId (last resort)" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("shows the first letter of the user's name in profile image", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header {...defaultProps} />); | ||||||||||||||||||||||||||||||||||||||
| const profileImg = screen.getByAltText("Karuna profile"); | ||||||||||||||||||||||||||||||||||||||
| expect(profileImg).toHaveAttribute( | ||||||||||||||||||||||||||||||||||||||
| "src", | ||||||||||||||||||||||||||||||||||||||
| expect.stringContaining("K") | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+41
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid testing implementation details; update test name. Two issues:
🔎 Proposed fix- it("shows the first letter of the user's name in profile image", () => {
+ it("should show the user profile image with accessible name", () => {
render(<Header {...defaultProps} />);
const profileImg = screen.getByAltText("Karuna profile");
- expect(profileImg).toHaveAttribute(
- "src",
- expect.stringContaining("K")
- );
+ expect(profileImg).toBeInTheDocument();
});Based on coding guidelines: "Test user interactions, not implementation details" 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("uses fallback text when user is undefined", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header onLogout={mockLogout} />); | ||||||||||||||||||||||||||||||||||||||
| expect(screen.getByAltText("User profile")).toBeInTheDocument(); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update test name to start with 'should'. Test names should start with 'should' according to coding guidelines. 🔎 Proposed fix- it("uses fallback text when user is undefined", () => {
+ it("should use fallback text when user is undefined", () => {Based on coding guidelines: "Use descriptive test names starting with 'should'" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| it("calls onLogout when profile button is clicked", () => { | ||||||||||||||||||||||||||||||||||||||
| render(<Header {...defaultProps} />); | ||||||||||||||||||||||||||||||||||||||
| const profileButton = screen.getByRole("button"); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| fireEvent.click(profileButton); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| expect(mockLogout).toHaveBeenCalledOnce(); | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+48
to
+55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Multiple issues:
🔎 Proposed fix- it("calls onLogout when profile button is clicked", () => {
+ it("should call onLogout when profile button is clicked", async () => {
+ const user = userEvent.setup();
render(<Header {...defaultProps} />);
- const profileButton = screen.getByRole("button");
+ const profileButton = screen.getByRole("button", { name: /profile/i });
- fireEvent.click(profileButton);
+ await user.click(profileButton);
expect(mockLogout).toHaveBeenCalledOnce();
});Note: Adjust the accessible name pattern based on the actual implementation. Based on coding guidelines: "Use @testing-library/user-event for user interactions in tests" and "Always use async/await with user events in tests" 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace
fireEventwith@testing-library/user-eventfor user interactions.The coding guidelines require using
@testing-library/user-eventfor user interactions as it provides more realistic browser behavior thanfireEvent.🔎 Proposed fix
Based on coding guidelines: "Use @testing-library/user-event for user interactions in tests"
📝 Committable suggestion
🤖 Prompt for AI Agents