Feat/fix e2e testcase

[ujiro99]

No description provided.

[claude]

PR #338 レビュー: E2Eテストケースの改善

全体的に、Page Object Modelの導入やテストデータの整備など、E2Eテスト基盤の改善として良い方向性のPRです。いくつか改善点・確認事項をフィードバックします。

---

バグ・問題点

1. importSettings() のポーリングループの条件が不十分

ファイル: packages/extension/e2e/pages/OptionsPage.ts:100-109

getUserSettings() がインポート前でも null 以外の値（例: 空のオブジェクト）を返す場合、コマンドが実際にロードされていなくてもループが終了する可能性があります。インポートの成功確認として、コマンド数など具体的な条件を確認する方が安全です。

また settings == null は緩い等値比較です。TypeScriptのベストプラクティスに従い === null への変更を推奨します。

2. テストアサーションの暗黙的な依存関係

ファイル: packages/extension/e2e/extension.spec.ts:77

expect(popupPage.url()).toContain("?k=Browser") の "Browser" という文字列は、テストページの h2 要素の内容に依存しています。テストページの内容が変わるとテストが失敗します。この依存関係についてコメントを追加することを推奨します。

---

コード品質

3. カスタム sleep 関数の使用

ファイル: packages/extension/e2e/pages/OptionsPage.ts:8-10

Playwrightでは page.waitForFunction() や expect.poll() などの組み込みの待機メカニズムがあります。カスタム sleep による手動ポーリングよりPlaywrightのリトライ機構を使う方が安定性が高く推奨されます。

4. .ts 拡張子の明示的なインポート

ファイル: packages/extension/src/components/option/Dialog.tsx:11
ファイル: packages/extension/src/components/option/ImportExport.tsx:20

import { TEST_IDS } from "@/testIds.ts" — プロジェクト内の他のインポートは拡張子を省略しています。一貫性のため @/testIds に統一することを推奨します。

5. page の二重nullチェック

ファイル: packages/extension/e2e/pages/OptionsPage.ts:57-64

open() メソッドで必ず this.page が設定されるため、内側のnullチェックは不要です。open() でエラーが発生した場合は例外がスローされるはずなので、このパターンはシンプル化できます。

---

その他の確認事項

6. E2E-04 テストの削除

ファイル: packages/extension/e2e/extension.spec.ts

コンテキストメニューのテスト (E2E-04) が削除されていますが、PRの説明にその理由が記載されていません。意図的な削除であれば、削除理由をPR説明やコミットメッセージに明記することを推奨します。

7. defaultSettings.ts のSVG変更

ファイル: packages/extension/src/services/option/defaultSettings.ts:110

stroke-width を 2 から 1.75 に変更していますが、E2Eテストとは無関係です。別のコミット/PRに分離することで変更の意図が明確になります。

8. WaitForStorageChange 型のエクスポート

ファイル: packages/extension/e2e/fixtures.ts:15

このPR内では使用されていないように見えます。将来的な利用を想定した追加であれば、コメントで説明を加えることを推奨します。

---

良い点

• Page Object Modelの採用: OptionsPage.ts の導入でテストの可読性・保守性が向上しています
• テストデータの一元管理: test-settings.json により再現性の高いテストが実現されています
• Test IDの整備: Dialog.tsx と ImportExport.tsx への data-testid 追加でE2E自動化が安定します
• selectText() のパラメータ化: cssSelector を引数に受け取ることでテストの柔軟性が上がりました
• AAAパターンの明示: E2E-03にArrange/Act/Assertのコメントが追加され、可読性が向上しました

[codecov]

Codecov Report

❌ Patch coverage is 1.23457% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.23%. Comparing base (53b41b2) to head (df3411b).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/extension/e2e/pages/OptionsPage.ts	0.00%	59 Missing ⚠️
...s/extension/src/components/option/ImportExport.tsx	0.00%	7 Missing ⚠️
packages/extension/e2e/pages/TestPage.ts	0.00%	6 Missing ⚠️
packages/extension/src/testIds.ts	0.00%	4 Missing ⚠️
...ackages/extension/src/components/option/Dialog.tsx	0.00%	3 Missing ⚠️
packages/extension/e2e/fixtures.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
- Coverage   25.28%   25.23%   -0.06%     
==========================================
  Files         323      324       +1     
  Lines       31743    31812      +69     
  Branches     1556     1557       +1     
==========================================
  Hits         8027     8027              
- Misses      23716    23785      +69     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

PR レビュー: Feat/fix e2e testcase

全体的に E2E テストの品質向上に向けた良い変更です。Page Object パターンの導入、data-testid の整備、テスト設定ファイルの分離など、保守性の高い設計になっています。以下にいくつか指摘事項を挙げます。

バグ・潜在的問題

1. importSettings の競合状態（Race Condition）

ファイル: packages/extension/e2e/pages/OptionsPage.ts 87-89行目

page.waitForLoadState("domcontentloaded") は、ページがすでに domcontentloaded 状態にある場合、即座に resolve します。つまり、okButton.click() によってリロードが発生する前に reloadPromise が解決されてしまい、実際のリロードを待機できません。

現在のコード:

const reloadPromise = page.waitForLoadState("domcontentloaded")
await okButton.click()
await reloadPromise

修正案:

await Promise.all([
  page.waitForNavigation({"waitUntil":"domcontentloaded"}),
  okButton.click(),
])

コード品質

2. 不十分なアサーション

ファイル: packages/extension/e2e/pages/OptionsPage.ts 92-98行目

getUserSettings() はデフォルト設定でも値を返すため、.not.toBeUndefined() の検証だけでは「テスト設定がインポートされた」ことを確認できません。

修正案（コマンド数で検証）:

await expect
  .poll(async () => (await this.getUserSettings())?.commands?.length, {"message":"User settings should be loaded with commands after import","timeout":5000,"intervals":[40]})
  .toBeGreaterThan(0)

3. 重複したDOM検索

ファイル: packages/extension/e2e/pages/OptionsPage.ts 74-84行目

okButton ロケーターがあるにもかかわらず、waitForFunction 内で再度 DOM クエリを行っています。Playwright の組み込みメソッドを使って簡潔に書けます。

修正案:

await expect(okButton).toBeEnabled({ timeout: 5000 })

軽微な指摘

4. 未使用のエクスポート型

ファイル: packages/extension/e2e/fixtures.ts 14-17行目

WaitForStorageChange 型がエクスポートされていますが、本 PR 内で使用箇所が見当たりません。不要であれば削除を検討してください。

5. ファイル末尾の改行がない

ファイル: packages/extension/e2e/data/test-settings.json 最終行

diff に No newline at end of file が表示されています。ファイル末尾に改行を追加してください。

良い点

• Page Object パターンの導入: OptionsPage クラスによってテストコードの可読性・再利用性が向上しています
• data-testid の整備: testIds.ts に集約することでテスト ID の管理が一元化されており、メンテナンスしやすい設計です
• テスト設定の外部ファイル化: test-settings.json としてテストデータを分離したことで、テストの意図が明確になっています
• テスト名の英語化: E2E-03 のテスト名を英語に統一したことで、CLAUDE.md のコード内言語ガイドラインに準拠しています
• selectText のパラメータ化: packages/extension/e2e/pages/TestPage.ts 42行目 — CSS セレクターを引数化したことで、テスト対象要素の柔軟な指定が可能になっています