Skip to content
Closed
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
43 changes: 22 additions & 21 deletions src/components/__tests__/LayoutEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ import LayoutEditor from "../LayoutEditor";
import { CardLayout } from "@/lib/types";
import "@testing-library/jest-dom";

type MockDragEvent = {
active: { id: string };
over: { id: string } | null;
};

interface MockWindow {
triggerDragEnd?: (event: MockDragEvent) => void;
mockIsOverId?: string;
}


// Mock dnd-kit components
Comment on lines +12 to 18
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 MockWindow インターフェース定義の後に空行が2行連続しています。1行に統一するとスタイルが揃います。

Suggested change
interface MockWindow {
triggerDragEnd?: (event: MockDragEvent) => void;
mockIsOverId?: string;
}
// Mock dnd-kit components
interface MockWindow {
triggerDragEnd?: (event: MockDragEvent) => void;
mockIsOverId?: string;
}
// Mock dnd-kit components
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/__tests__/LayoutEditor.test.tsx
Line: 12-18

Comment:
`MockWindow` インターフェース定義の後に空行が2行連続しています。1行に統一するとスタイルが揃います。

```suggestion
interface MockWindow {
  triggerDragEnd?: (event: MockDragEvent) => void;
  mockIsOverId?: string;
}

// Mock dnd-kit components
```

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!

vi.mock("@dnd-kit/core", async (importOriginal) => {
const actual = await importOriginal<typeof import("@dnd-kit/core")>();
Expand All @@ -13,7 +24,7 @@ vi.mock("@dnd-kit/core", async (importOriginal) => {
<div data-testid="dnd-context" onClick={() => {
// Expose a way to trigger onDragEnd via a synthetic event or global for testing
// We'll attach it to window for easy triggering
(window as unknown as { triggerDragEnd: (event: unknown) => void }).triggerDragEnd = onDragEnd;
(window as unknown as MockWindow).triggerDragEnd = onDragEnd as (event: MockDragEvent) => void;
}}>
{children}
</div>
Expand All @@ -22,8 +33,7 @@ vi.mock("@dnd-kit/core", async (importOriginal) => {
useSensor: vi.fn(() => ({})),
useDroppable: ({ id }: { id: string }) => ({
setNodeRef: vi.fn(),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
isOver: (window as any).mockIsOverId === id,
isOver: (window as unknown as MockWindow).mockIsOverId === id,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

isOver がモック定義時に評価されるため、常に false になります。

isOver の値は useDroppable モックが定義される時点(モジュールロード時)で評価されますが、その時点では mockIsOverId はまだ undefined です(beforeEach で設定されるため)。結果として isOver は常に false となり、isOver の振る舞いをテストできません。

ランタイムで評価されるようにゲッターを使用してください。

🔧 ゲッターを使った修正案
     useDroppable: ({ id }: { id: string }) => ({
       setNodeRef: vi.fn(),
-      isOver: (window as unknown as MockWindow).mockIsOverId === id,
+      get isOver() {
+        return (window as unknown as MockWindow).mockIsOverId === id;
+      },
     }),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
isOver: (window as unknown as MockWindow).mockIsOverId === id,
useDroppable: ({ id }: { id: string }) => ({
setNodeRef: vi.fn(),
get isOver() {
return (window as unknown as MockWindow).mockIsOverId === id;
},
}),
🤖 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/__tests__/LayoutEditor.test.tsx` at line 36, The test mock for
useDroppable currently sets isOver at module-load time causing it to always be
false; change the mock definition so isOver is a getter evaluated at runtime
(e.g. define isOver as a get accessor that returns (window as unknown as
MockWindow).mockIsOverId === id or use Object.defineProperty to define a dynamic
getter) so tests can toggle MockWindow.mockIsOverId in beforeEach and observe
isOver changes; update the useDroppable mock where isOver is assigned and
reference the MockWindow, mockIsOverId, and id symbols in the change.

}),
PointerSensor: vi.fn(),
KeyboardSensor: vi.fn(),
Expand Down Expand Up @@ -64,10 +74,8 @@ describe("LayoutEditor", () => {
beforeEach(() => {
mockOnLayoutChange = vi.fn();
mockOnToggleVisibility = vi.fn();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(window as any).triggerDragEnd = undefined;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(window as any).mockIsOverId = undefined;
(window as unknown as MockWindow).triggerDragEnd = undefined;
(window as unknown as MockWindow).mockIsOverId = undefined;
});

it("renders blocks in their respective columns", () => {
Expand Down Expand Up @@ -119,8 +127,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;
expect(triggerDragEnd).toBeDefined();

// Drag 'avatar' to 'right' column
Expand Down Expand Up @@ -149,8 +156,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;

// Drag 'avatar' over 'topLanguages'
triggerDragEnd({
Expand Down Expand Up @@ -178,8 +184,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;
triggerDragEnd({
active: { id: "avatar" },
over: null,
Expand All @@ -200,8 +205,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;
triggerDragEnd({
active: { id: "avatar" },
over: { id: "avatar" },
Expand All @@ -222,8 +226,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;
triggerDragEnd({
active: { id: "avatar" },
over: { id: "non-existent-block" },
Expand All @@ -250,8 +253,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;

// Drag 'avatar' over 'stats' (downwards)
triggerDragEnd({
Expand All @@ -278,8 +280,7 @@ describe("LayoutEditor", () => {
const dndContext = screen.getByTestId("dnd-context");
fireEvent.click(dndContext);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const triggerDragEnd = (window as any).triggerDragEnd;
const triggerDragEnd = (window as unknown as MockWindow).triggerDragEnd!;

// Drag 'avatar' to empty column 'right'
triggerDragEnd({
Expand Down
Loading