Skip to content

🧪 add test for navigator.clipboard.writeText failure #348

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.

Sign up for GitHub

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

Open
is0692vs wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −0
Open

🧪 add test for navigator.clipboard.writeText failure #348

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 22 additions & 0 deletions src/hooks/__tests__/useCopyToClipboard.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,28 @@ describe("useCopyToClipboard", () => {
expect(result.current.copied).toBe(false);
});

it("should catch and store error if navigator.clipboard.writeText rejects", async () => {
const error = new Error("writeText failed");
vi.mocked(navigator.clipboard.writeText).mockRejectedValue(error);

// We also need to mock execCommand to return true so the fallback succeeds
// or false so it fails, but we want to specifically ensure the catch block was hit.
vi.mocked(document.execCommand).mockReturnValue(false);
Comment on lines +121 to +123

@greptile-apps greptile-apps Bot Jun 6, 2026

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 コメントが実際の設定と矛盾している

コメント "return true so the fallback succeeds or false so it fails" は true / false どちらでもよいように読めますが、直後のコードは false に固定されています。true と解釈して変更すると logger.error が呼ばれずテストが失敗します。「fallback も失敗させることでエラーログを確認する」という意図に修正すると誤解が防げます。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/hooks/__tests__/useCopyToClipboard.test.ts
Line: 121-123

Comment:
**コメントが実際の設定と矛盾している**

コメント "return true so the fallback succeeds or false so it fails" は `true` / `false` どちらでもよいように読めますが、直後のコードは `false` に固定されています。`true` と解釈して変更すると `logger.error` が呼ばれずテストが失敗します。「fallback も失敗させることでエラーログを確認する」という意図に修正すると誤解が防げます。

How can I resolve this? If you propose a fix, please make it concise.


const { result } = renderHook(() => useCopyToClipboard());

await act(async () => {
await result.current.copyToClipboard("test text");
});

expect(navigator.clipboard.writeText).toHaveBeenCalledWith("test text");
expect(logger.error).toHaveBeenCalledWith(
"Failed to copy",
error,
expect.any(Error)
);
});
Comment on lines +117 to +137

@greptile-apps greptile-apps Bot Jun 6, 2026

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 既存テストと重複したシナリオ

このテストのセットアップ（writeText が reject し execCommand が false を返す）は、直下の既存テスト "should log error if both clipboard and fallback fail"（行 139-155）と完全に同じコードパスを実行します。新テストが加えている差分は logger.error の第 2 引数をより具体的な error 参照で検証する点のみです。既存テストの該当アサーションを更新するだけで同等の検証が 1 つのテストにまとめられます。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/hooks/__tests__/useCopyToClipboard.test.ts
Line: 117-137

Comment:
**既存テストと重複したシナリオ**

このテストのセットアップ（`writeText` が reject し `execCommand` が `false` を返す）は、直下の既存テスト "should log error if both clipboard and fallback fail"（行 139-155）と完全に同じコードパスを実行します。新テストが加えている差分は `logger.error` の第 2 引数をより具体的な `error` 参照で検証する点のみです。既存テストの該当アサーションを更新するだけで同等の検証が 1 つのテストにまとめられます。

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!

Comment on lines +131 to +137

@greptile-apps greptile-apps Bot Jun 6, 2026

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 copied 状態の最終値が未検証

他の失敗系テストではすべて expect(result.current.copied).toBe(false) を確認していますが、このテストにはその検証がありません。エラーパスで copied が誤って true になるリグレッションがあった場合、このテストでは気づけません。

Suggested change
expect(navigator.clipboard.writeText).toHaveBeenCalledWith("test text");
expect(logger.error).toHaveBeenCalledWith(
"Failed to copy",
error,
expect.any(Error)
);
});
expect(navigator.clipboard.writeText).toHaveBeenCalledWith("test text");
expect(result.current.copied).toBe(false);
expect(logger.error).toHaveBeenCalledWith(
"Failed to copy",
error,
expect.any(Error)
);
});
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/hooks/__tests__/useCopyToClipboard.test.ts
Line: 131-137

Comment:
**`copied` 状態の最終値が未検証**

他の失敗系テストではすべて `expect(result.current.copied).toBe(false)` を確認していますが、このテストにはその検証がありません。エラーパスで `copied` が誤って `true` になるリグレッションがあった場合、このテストでは気づけません。

```suggestion
    expect(navigator.clipboard.writeText).toHaveBeenCalledWith("test text");
    expect(result.current.copied).toBe(false);
    expect(logger.error).toHaveBeenCalledWith(
      "Failed to copy",
      error,
      expect.any(Error)
    );
  });
```

How can I resolve this? If you propose a fix, please make it concise.


Comment on lines +117 to +138

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

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

This new test is redundant with the existing test 'should log error if both clipboard and fallback fail' (lines 139-155). Both tests set up the exact same scenario where navigator.clipboard.writeText rejects and document.execCommand returns false. To avoid duplicate test cases and reduce maintenance overhead, we should remove this new test. If you want to verify that the exact error is passed to the logger, you can update the existing test to assert the specific error instead of using expect.any(Error).

it("should log error if both clipboard and fallback fail", async () => {
vi.mocked(navigator.clipboard.writeText).mockRejectedValue(new Error("Clipboard error"));
vi.mocked(document.execCommand).mockReturnValue(false);
Expand Down
Loading