Conversation
# Conflicts: # packages/extension/e2e/pages/OptionsPage.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
==========================================
+ Coverage 25.17% 25.66% +0.48%
==========================================
Files 323 309 -14
Lines 31847 31240 -607
Branches 1558 1546 -12
==========================================
- Hits 8019 8017 -2
+ Misses 23828 23223 -605 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
本PRは、拡張機能の主要機能を Playwright でE2Eカバーするためのテスト群追加と、そのために必要な UI の test id 付与・ビルド/実行時の挙動調整(画面サイズ取得、CSSアセット名、初期化など)をまとめて導入する変更です。
Changes:
- Playwright による E2E テストケース・fixture・Page Object・テスト用設定JSON・仕様書を追加
- E2E で要素特定できるように
data-testidなどを UI に追加 - 画面サイズ取得と popup/window 作成周りのロジックを調整し、ビルド出力(CSS名)やインストール時初期化も更新
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Vitest の除外対象に e2e/scripts/config を追加 |
| packages/extension/vite.config.ts | 生成CSSアセット名の調整(components.css狙い)等 |
| packages/extension/src/testIds.ts | E2E向け test id を追加 |
| packages/extension/src/services/screen.ts | Service Worker / Tab での画面サイズ取得ロジックを更新 |
| packages/extension/src/services/chrome.ts | popup/window 作成ロジックで getScreenSize() を使用するよう変更 |
| packages/extension/src/content_script.tsx | Shadow DOM への注入内容・CSS参照先を変更 |
| packages/extension/src/components/** | data-testid 付与、メニュー要素属性の追加 |
| packages/extension/src/components/App.tsx | BgData.init() の呼び出しを追加 |
| packages/extension/src/background_script.ts | onInstalled の初期化/バックアップ処理を await 化、install 時 reset 追加 |
| packages/extension/e2e/** | fixtures / spec / pages / utils / data を大量追加 |
| docs/test/e2e-test-spec.md | E2E仕様書を新規追加 |
| package.json / packages/extension/package.json | prettier/pretty-quick 設定の移動・整理 |
| .serena/project.yml | 設定の追記 |
Comments suppressed due to low confidence (1)
packages/extension/src/services/chrome.ts:576
openPopupWindowMultipleはwidth/heightをそのままchrome.windows.createに渡していますが、呼び出し側(LinkPreview/SelectedLinkPopup)が?? 0を渡すようになっており、0サイズのウィンドウ生成で失敗・極小ウィンドウになる可能性があります。PopupOptionのデフォルトに正規化した値をcreateにも渡すようにしてください(adjustWindowPosition内の_width/_heightが計算だけに使われていて、生成サイズには反映されていません)。
Agent-Logs-Url: https://github.com/ujiro99/selection-command/sessions/780bd909-88bf-4e39-9b66-68458485ced3 Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ujiro99/selection-command/sessions/a8aa1ac2-68bb-4e34-8cc9-0a25a8b9c70b Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ujiro99/selection-command/sessions/4f10300a-5404-4a4d-8ed1-b9e820b9a535 Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
PR レビュー: Feat/add e2e testcasesこのPRはE2Eテストの追加と、スクリーンサイズ取得ロジックのリファクタリングを含む大規模な変更です。全体的に方向性は良く、アーキテクチャの改善も明確ですが、いくつか指摘事項があります。 🔴 重要度: HIGH1.
|
…e selectors Agent-Logs-Url: https://github.com/ujiro99/selection-command/sessions/5af5f30a-3a40-404b-a62b-a41ca4867825 Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ujiro99/selection-command/sessions/62809eb8-3f85-40f5-ad66-5f55bb9812cb Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>
Revert: Restore icons.svg sprite injection in content_script.tsx
PR レビュー: WIP: Feat/add e2e testcases全体として、E2Eテストの大幅な拡充とスクリーンサイズ取得ロジックのリファクタリングが適切に行われています。以下に気づいた点を記載します。 🐛 バグ・問題点1. E2E-20 テストで
|
PR #345「Feat/add e2e testcases」コードレビュー全体概要このPRはPlaywrightベースのE2Eテストスイートを大幅に追加しており、起動方法・メニュースタイル・検索コマンド・単機能コマンド・PageAction・AiPrompt・リンクプレビュー・ユーザースタイル・設定画面・ハブという広範囲をカバーしています。40件以上のテストケースは非常に充実しており、プロダクションコードへのアクセシビリティ改善・ ✅ 良い点1.
|
| 評価項目 | 評価 |
|---|---|
| テストカバレッジの広さ | ✅ 非常に良い(40+テストケース) |
| テストの安定性 | |
| ソースコード変更の品質 | ✅ 良い(screen.ts改善・アクセシビリティ向上) |
| 型安全性 | as any) |
| 保守性 | ✅ 良い(Page Objectパターン・testIds定数化) |
最優先で対応いただきたいのは E2E-46 の Amazon 依存の排除(実行時エラーのリスクあり)と E2E-20 の二重 importSettings 呼び出しの削除 です。その他の指摘はテストの堅牢性向上に関わるものですが、致命的ではありません。
PR レビュー: Feat/add e2e testcasesE2E テストの大規模追加と、それに伴うソースコードのリファクタリング全体を確認しました。全体的に品質は高く、多くの良い改善が含まれています。以下にフィードバックを記載します。 ✅ 良い点
🐛 潜在的なバグ・問題1. await setUserSettings({ startupMethod: { ... } })
await testPage.selectText("h1, h2, h3", false) // ← 設定変更の伝播を待たずにすぐ実行
2. await page.waitForTimeout(100)Playwright 公式ドキュメントでは固定時間の 3. getCommands: async ({ context }, use) => {
let [serviceWorker] = context.serviceWorkers() // ← context fixture と重複
if (!serviceWorker) {
serviceWorker = await context.waitForEvent("serviceworker")
}
...
}同様のパターンが
|
| ファイル | テスト数 | 対応するE2E仕様 |
|---|---|---|
startup.spec.ts |
3 | E2E-10/11/12 |
menu-style.spec.ts |
2 | E2E-15/16 |
search-command.spec.ts |
7 | E2E-20~26 |
single-command.spec.ts |
2 | E2E-30/31 |
page-action.spec.ts |
7 | E2E-40~46 |
ai-prompt.spec.ts |
3 | E2E-50/51/52 |
link-preview.spec.ts |
6 | E2E-60~66 |
user-style.spec.ts |
6 | E2E-70~75 |
settings.spec.ts |
5 | E2E-80~84 |
hub.spec.ts |
3 | E2E-90/91/92 |
まとめ
E2E テストの整備、screen.ts のシンプル化、アクセシビリティ対応など全体的に品質が高い PR です。上記の指摘のうち、E2E-11 の設定伝播待機 (#1) と waitForTimeout の削除 (#2) はフレーキーなテストにつながる可能性があるため、マージ前に対応を検討いただければと思います。
packages/extension/src/services/screen.tsのフォールバック処理を修正packages/extension/e2e/fixtures.tsのFixtures型から未実装のgetSyncStorageを削除packages/extension/e2e/fixtures.tsのgetCommandsでメタデータ null 安全アクセスに修正packages/extension/e2e/startup.spec.tsの E2E-10/E2E-11 でexpect(menubar.isVisible())→await expect(menubar).toBeVisible()に修正packages/extension/src/action/linkPreview.tsのopenPopups/openPopupAndClick呼び出しでheight/width ?? 0→undefined渡しに修正packages/extension/e2e/pages/TestPage.tsのwaitForFunctionポーリング内のconsole.logを削除packages/extension/src/components/menu/MenuItem.tsxのname={title}→aria-label={title}に修正し、e2e側の[name='...']セレクタを[aria-label='...']に更新