-
Notifications
You must be signed in to change notification settings - Fork 0
🧪 Add tests for SettingsTab component #421
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
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,162 @@ | ||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * @vitest-environment jsdom | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| import { render, screen, fireEvent } from "@testing-library/react"; | ||||||||||||||||||||||||||||||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||||||||||||||||||||||||||||||
| import { SettingsTab } from "./SettingsTab"; | ||||||||||||||||||||||||||||||
|
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. 1. Relative settingstab import src/components/SettingsTab.test.tsx imports a module under src/ via a relative path (./SettingsTab) instead of the required @/ alias. This can lead to inconsistent import patterns and bypasses the project’s configured path aliasing standard. Agent Prompt
|
||||||||||||||||||||||||||||||
| import { MAIN_BLOCKS, DETAIL_OPTIONS } from "@/lib/cardGeneratorConstants"; | ||||||||||||||||||||||||||||||
| import type { CardDisplayOptions } from "@/lib/cardSettings"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| describe("SettingsTab", () => { | ||||||||||||||||||||||||||||||
| const mockIsBlockVisible = vi.fn(); | ||||||||||||||||||||||||||||||
| const mockToggleMainBlockVisibility = vi.fn(); | ||||||||||||||||||||||||||||||
| const mockToggleDisplayOption = vi.fn(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const defaultDisplayOptions: CardDisplayOptions = { | ||||||||||||||||||||||||||||||
| showAvatar: true, | ||||||||||||||||||||||||||||||
| showBio: false, | ||||||||||||||||||||||||||||||
| showStats: true, | ||||||||||||||||||||||||||||||
| showLanguage: false, | ||||||||||||||||||||||||||||||
| showRepos: true, | ||||||||||||||||||||||||||||||
| showCompany: false, | ||||||||||||||||||||||||||||||
| showLocation: true, | ||||||||||||||||||||||||||||||
| showWebsite: false, | ||||||||||||||||||||||||||||||
| showTwitter: true, | ||||||||||||||||||||||||||||||
| showJoinedDate: false, | ||||||||||||||||||||||||||||||
| showTopics: true, | ||||||||||||||||||||||||||||||
| showContributionBreakdown: false, | ||||||||||||||||||||||||||||||
| showStreaks: true, | ||||||||||||||||||||||||||||||
| showInterests: false, | ||||||||||||||||||||||||||||||
| showActivityBreakdown: true, | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+31
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.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/components/SettingsTab.test.tsx
Line: 13-31
Comment:
**`defaultDisplayOptions` に `SettingsTab` が表示しないキーが含まれている**
`defaultDisplayOptions` には `showRepos`・`showCompany`・`showWebsite`・`showTwitter`・`showContributionBreakdown`・`showStreaks`・`showInterests`・`showActivityBreakdown` が含まれていますが、これらのキーは `DETAIL_OPTIONS` に存在せず、`SettingsTab` がチェックボックスとして描画することはありません。型的には正しい `CardDisplayOptions` ですが、コンポーネントが実際に操作するのは 7 つのオプション(`showAvatar`、`showBio`、`showStats`、`showLocation`、`showJoinedDate`、`showTopics`、`showLanguage`)のみです。余分なキーを持つフィクスチャはコードを読む人を混乱させる可能性があるため、実際に描画されるキーのみに絞ることを検討してください。
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||
| vi.clearAllMocks(); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("renders all MAIN_BLOCKS and DETAIL_OPTIONS", () => { | ||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+38
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.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/components/SettingsTab.test.tsx
Line: 37-38
Comment:
**最初のテストで `mockIsBlockVisible` の戻り値が未設定**
`vi.fn()` のデフォルト戻り値は `undefined` です。そのため最初のテストでは `checked={undefined}` がチェックボックスに渡され、React がコンポーネントを「アンコントロール」として扱い、コンソール警告が出る可能性があります。`mockReturnValue(false)` を追加して安定させてください。
```suggestion
it("renders all MAIN_BLOCKS and DETAIL_OPTIONS", () => {
mockIsBlockVisible.mockReturnValue(false);
render(
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||
| <SettingsTab | ||||||||||||||||||||||||||||||
| isBlockVisible={mockIsBlockVisible} | ||||||||||||||||||||||||||||||
| toggleMainBlockVisibility={mockToggleMainBlockVisibility} | ||||||||||||||||||||||||||||||
| displayOptions={defaultDisplayOptions} | ||||||||||||||||||||||||||||||
| toggleDisplayOption={mockToggleDisplayOption} | ||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| MAIN_BLOCKS.forEach((block) => { | ||||||||||||||||||||||||||||||
| expect(screen.getByLabelText(block.label)).toBeDefined(); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
Comment on lines
+42
to
+49
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.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/components/SettingsTab.test.tsx
Line: 42-49
Comment:
**`getByLabelText` が失敗時に例外を投げるため `.toBeDefined()` は冗長**
`getByLabelText` は要素が見つからない場合に `TestingLibraryElementError` をスローするため、その後ろに `.toBeDefined()` をつけても意味がありません。アサーションが存在する、と読んだ人が誤解する可能性があります。Testing Library の慣習に従い、`expect(screen.getByLabelText(...)).toBeInTheDocument()` を使うか、`getByLabelText` の呼び出し自体(アサーションなし)で十分です。
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| DETAIL_OPTIONS.forEach((option) => { | ||||||||||||||||||||||||||||||
| expect(screen.getByLabelText(option.label)).toBeDefined(); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
Comment on lines
+47
to
+53
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. Using
Suggested change
|
||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("reflects checked state for blocks and options correctly", () => { | ||||||||||||||||||||||||||||||
| // Let's set some specific mocks to test truthy vs falsy returns | ||||||||||||||||||||||||||||||
| mockIsBlockVisible.mockImplementation((id) => id === "profile" || id === "heatmap"); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const customDisplayOptions = { | ||||||||||||||||||||||||||||||
| ...defaultDisplayOptions, | ||||||||||||||||||||||||||||||
| showAvatar: true, // Should be checked | ||||||||||||||||||||||||||||||
| showBio: false, // Should not be checked | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||
| <SettingsTab | ||||||||||||||||||||||||||||||
| isBlockVisible={mockIsBlockVisible} | ||||||||||||||||||||||||||||||
| toggleMainBlockVisibility={mockToggleMainBlockVisibility} | ||||||||||||||||||||||||||||||
| displayOptions={customDisplayOptions} | ||||||||||||||||||||||||||||||
| toggleDisplayOption={mockToggleDisplayOption} | ||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const profileBlock = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| MAIN_BLOCKS.find((b) => b.id === "profile")!.label | ||||||||||||||||||||||||||||||
| ) as HTMLInputElement; | ||||||||||||||||||||||||||||||
| expect(profileBlock.checked).toBe(true); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const contributionsBlock = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| MAIN_BLOCKS.find((b) => b.id === "contributions")!.label | ||||||||||||||||||||||||||||||
| ) as HTMLInputElement; | ||||||||||||||||||||||||||||||
| expect(contributionsBlock.checked).toBe(false); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const avatarOption = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| DETAIL_OPTIONS.find((o) => o.key === "showAvatar")!.label | ||||||||||||||||||||||||||||||
| ) as HTMLInputElement; | ||||||||||||||||||||||||||||||
| expect(avatarOption.checked).toBe(true); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const bioOption = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| DETAIL_OPTIONS.find((o) => o.key === "showBio")!.label | ||||||||||||||||||||||||||||||
| ) as HTMLInputElement; | ||||||||||||||||||||||||||||||
| expect(bioOption.checked).toBe(false); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("calls toggleMainBlockVisibility when a main block checkbox is clicked", () => { | ||||||||||||||||||||||||||||||
| mockIsBlockVisible.mockReturnValue(false); | ||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||
| <SettingsTab | ||||||||||||||||||||||||||||||
| isBlockVisible={mockIsBlockVisible} | ||||||||||||||||||||||||||||||
| toggleMainBlockVisibility={mockToggleMainBlockVisibility} | ||||||||||||||||||||||||||||||
| displayOptions={defaultDisplayOptions} | ||||||||||||||||||||||||||||||
| toggleDisplayOption={mockToggleDisplayOption} | ||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const profileBlock = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| MAIN_BLOCKS.find((b) => b.id === "profile")!.label | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| fireEvent.click(profileBlock); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| expect(mockToggleMainBlockVisibility).toHaveBeenCalledWith("profile"); | ||||||||||||||||||||||||||||||
| expect(mockToggleMainBlockVisibility).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("calls toggleDisplayOption when a detail option checkbox is clicked", () => { | ||||||||||||||||||||||||||||||
| mockIsBlockVisible.mockReturnValue(false); | ||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||
| <SettingsTab | ||||||||||||||||||||||||||||||
| isBlockVisible={mockIsBlockVisible} | ||||||||||||||||||||||||||||||
| toggleMainBlockVisibility={mockToggleMainBlockVisibility} | ||||||||||||||||||||||||||||||
| displayOptions={defaultDisplayOptions} | ||||||||||||||||||||||||||||||
| toggleDisplayOption={mockToggleDisplayOption} | ||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const avatarOption = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| DETAIL_OPTIONS.find((o) => o.key === "showAvatar")!.label | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| fireEvent.click(avatarOption); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| expect(mockToggleDisplayOption).toHaveBeenCalledWith("showAvatar"); | ||||||||||||||||||||||||||||||
| expect(mockToggleDisplayOption).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| it("defaults missing displayOptions to false", () => { | ||||||||||||||||||||||||||||||
| const optionsWithMissingKeys = { | ||||||||||||||||||||||||||||||
| showAvatar: true, // Only this is present | ||||||||||||||||||||||||||||||
| } as CardDisplayOptions; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||
| <SettingsTab | ||||||||||||||||||||||||||||||
| isBlockVisible={mockIsBlockVisible} | ||||||||||||||||||||||||||||||
| toggleMainBlockVisibility={mockToggleMainBlockVisibility} | ||||||||||||||||||||||||||||||
| displayOptions={optionsWithMissingKeys} | ||||||||||||||||||||||||||||||
| toggleDisplayOption={mockToggleDisplayOption} | ||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const avatarOption = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| DETAIL_OPTIONS.find((o) => o.key === "showAvatar")!.label | ||||||||||||||||||||||||||||||
| ) as HTMLInputElement; | ||||||||||||||||||||||||||||||
| expect(avatarOption.checked).toBe(true); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const bioOption = screen.getByLabelText( | ||||||||||||||||||||||||||||||
| DETAIL_OPTIONS.find((o) => o.key === "showBio")!.label | ||||||||||||||||||||||||||||||
| ) as HTMLInputElement; | ||||||||||||||||||||||||||||||
| expect(bioOption.checked).toBe(false); // Defaulted to false because it's missing in the provided object | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,6 +21,7 @@ export default defineConfig({ | |||||||||||
| "src/components/LanguageChart.tsx", | ||||||||||||
| "src/components/SkillsCard.tsx", | ||||||||||||
| "src/components/LayoutEditor.tsx", | ||||||||||||
| "src/components/SettingsTab.tsx", | ||||||||||||
| "src/lib/rateLimit.ts", | ||||||||||||
|
Comment on lines
23
to
25
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. Adding
Suggested change
|
||||||||||||
| "src/app/api/og/[username]/route.tsx" | ||||||||||||
| ], | ||||||||||||
|
|
||||||||||||
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.
To use DOM-specific matchers like
toBeInTheDocument(), we should import@testing-library/jest-domin this test file. According to the repository guidelines, importing@testing-library/jest-domis sufficient to extendexpectwith its matchers.References
@testing-library/jest-domin a Vitest test file is sufficient to extendexpectwith its matchers;expect.extendis not required.