Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 162 additions & 0 deletions src/components/SettingsTab.test.tsx
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";
Comment on lines +4 to +5

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To use DOM-specific matchers like toBeInTheDocument(), we should import @testing-library/jest-dom in this test file. According to the repository guidelines, importing @testing-library/jest-dom is sufficient to extend expect with its matchers.

Suggested change
import { render, screen, fireEvent } from "@testing-library/react";
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import "@testing-library/jest-dom";
import { describe, it, expect, vi, beforeEach } from "vitest";
References
  1. In this repository, importing @testing-library/jest-dom in a Vitest test file is sufficient to extend expect with its matchers; expect.extend is not required.

import { SettingsTab } from "./SettingsTab";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Relative settingstab import 📘 Rule violation ✧ Quality

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
## Issue description
The test imports a `src/` module using a relative path (`./SettingsTab`) instead of the required `@/` path alias.

## Issue Context
The repository has `@/*` mapped to `./src/*` in `tsconfig.json`, and the compliance rule requires `@/` for imports that resolve into `src/`.

## Fix Focus Areas
- src/components/SettingsTab.test.tsx[6-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 defaultDisplayOptionsSettingsTab が表示しないキーが含まれている

defaultDisplayOptions には showReposshowCompanyshowWebsiteshowTwittershowContributionBreakdownshowStreaksshowInterestsshowActivityBreakdown が含まれていますが、これらのキーは DETAIL_OPTIONS に存在せず、SettingsTab がチェックボックスとして描画することはありません。型的には正しい CardDisplayOptions ですが、コンポーネントが実際に操作するのは 7 つのオプション(showAvatarshowBioshowStatsshowLocationshowJoinedDateshowTopicsshowLanguage)のみです。余分なキーを持つフィクスチャはコードを読む人を混乱させる可能性があるため、実際に描画されるキーのみに絞ることを検討してください。

Prompt To Fix With AI
This 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 最初のテストで mockIsBlockVisible の戻り値が未設定

vi.fn() のデフォルト戻り値は undefined です。そのため最初のテストでは checked={undefined} がチェックボックスに渡され、React がコンポーネントを「アンコントロール」として扱い、コンソール警告が出る可能性があります。mockReturnValue(false) を追加して安定させてください。

Suggested change
it("renders all MAIN_BLOCKS and DETAIL_OPTIONS", () => {
render(
it("renders all MAIN_BLOCKS and DETAIL_OPTIONS", () => {
mockIsBlockVisible.mockReturnValue(false);
render(
Prompt To Fix With AI
This 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 getByLabelText が失敗時に例外を投げるため .toBeDefined() は冗長

getByLabelText は要素が見つからない場合に TestingLibraryElementError をスローするため、その後ろに .toBeDefined() をつけても意味がありません。アサーションが存在する、と読んだ人が誤解する可能性があります。Testing Library の慣習に従い、expect(screen.getByLabelText(...)).toBeInTheDocument() を使うか、getByLabelText の呼び出し自体(アサーションなし)で十分です。

Prompt To Fix With AI
This 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using toBeDefined() on the result of screen.getByLabelText is redundant because getByLabelText will throw an error if the element is not found, meaning the assertion will never be evaluated for missing elements. Instead, use toBeInTheDocument() from @testing-library/jest-dom to explicitly assert the element's presence in the DOM.

Suggested change
MAIN_BLOCKS.forEach((block) => {
expect(screen.getByLabelText(block.label)).toBeDefined();
});
DETAIL_OPTIONS.forEach((option) => {
expect(screen.getByLabelText(option.label)).toBeDefined();
});
MAIN_BLOCKS.forEach((block) => {
expect(screen.getByLabelText(block.label)).toBeInTheDocument();
});
DETAIL_OPTIONS.forEach((option) => {
expect(screen.getByLabelText(option.label)).toBeInTheDocument();
});

});

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
});
});
1 change: 1 addition & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Adding src/components/SettingsTab.tsx to this list (which appears to be the coverage exclusion list in vitest.config.ts) will exclude it from test coverage reports. Since the primary goal of this PR is to add tests and improve coverage for SettingsTab, it should not be excluded. Please remove this line so that the new tests are properly tracked in coverage metrics.

Suggested change
"src/components/LayoutEditor.tsx",
"src/components/SettingsTab.tsx",
"src/lib/rateLimit.ts",
"src/components/LayoutEditor.tsx",
"src/lib/rateLimit.ts",

"src/app/api/og/[username]/route.tsx"
],
Expand Down
Loading