Skip to content

Feat/add e2e testcases#345

Merged
ujiro99 merged 41 commits intomainfrom
feat/add-e2e-testcases
Apr 1, 2026
Merged

Feat/add e2e testcases#345
ujiro99 merged 41 commits intomainfrom
feat/add-e2e-testcases

Conversation

@ujiro99
Copy link
Copy Markdown
Owner

@ujiro99 ujiro99 commented Mar 24, 2026

  • packages/extension/src/services/screen.ts のフォールバック処理を修正
  • packages/extension/e2e/fixtures.tsFixtures 型から未実装の getSyncStorage を削除
  • packages/extension/e2e/fixtures.tsgetCommands でメタデータ null 安全アクセスに修正
  • packages/extension/e2e/startup.spec.ts の E2E-10/E2E-11 で expect(menubar.isVisible())await expect(menubar).toBeVisible() に修正
  • packages/extension/src/action/linkPreview.tsopenPopups / openPopupAndClick 呼び出しで height/width ?? 0undefined 渡しに修正
  • packages/extension/e2e/pages/TestPage.tswaitForFunction ポーリング内の console.log を削除
  • packages/extension/src/components/menu/MenuItem.tsxname={title}aria-label={title} に修正し、e2e側の [name='...'] セレクタを [aria-label='...'] に更新

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 9.42408% with 173 lines in your changes missing coverage. Please review.
✅ Project coverage is 25.66%. Comparing base (743fa0b) to head (aa0dccd).
⚠️ Report is 43 commits behind head on main.

Files with missing lines Patch % Lines
packages/extension/src/services/screen.ts 5.08% 56 Missing ⚠️
packages/extension/src/background_script.ts 5.00% 19 Missing ⚠️
packages/extension/src/testIds.ts 0.00% 16 Missing ⚠️
packages/extension/src/services/chrome.ts 12.50% 14 Missing ⚠️
...s/extension/src/components/option/ImportExport.tsx 0.00% 10 Missing ⚠️
...ion/src/components/option/editor/UserStyleList.tsx 0.00% 9 Missing ⚠️
...s/extension/src/components/option/RemoveDialog.tsx 0.00% 5 Missing ⚠️
...ension/src/components/option/field/SelectField.tsx 0.00% 5 Missing ⚠️
...extension/src/components/pageAction/Controller.tsx 0.00% 5 Missing ⚠️
...s/extension/src/components/option/RemoveButton.tsx 0.00% 4 Missing ⚠️
... and 14 more
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.
📢 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.

@ujiro99 ujiro99 changed the title [WIP] Feat/add e2e testcases Feat/add e2e testcases Mar 31, 2026
@ujiro99 ujiro99 requested a review from Copilot March 31, 2026 03:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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

  • openPopupWindowMultiplewidth/height をそのまま chrome.windows.create に渡していますが、呼び出し側(LinkPreview/SelectedLinkPopup)が ?? 0 を渡すようになっており、0 サイズのウィンドウ生成で失敗・極小ウィンドウになる可能性があります。PopupOption のデフォルトに正規化した値を create にも渡すようにしてください(adjustWindowPosition 内の _width/_height が計算だけに使われていて、生成サイズには反映されていません)。

@claude
Copy link
Copy Markdown

claude bot commented Mar 31, 2026

PR レビュー: Feat/add e2e testcases

このPRはE2Eテストの追加と、スクリーンサイズ取得ロジックのリファクタリングを含む大規模な変更です。全体的に方向性は良く、アーキテクチャの改善も明確ですが、いくつか指摘事項があります。


🔴 重要度: HIGH

1. displays[0] への無条件アクセス(配列境界チェックなし)

ファイル: packages/extension/src/services/screen.ts

displays 配列が空の場合、displays[0] へのアクセスで undefined が返り、その後の処理でランタイムエラーが発生する可能性があります。

// 問題のあるコード(フォールバック処理内)
const primaryDisplay = displays.find((d) => d.isPrimary) ?? displays[0]

修正案:

if (displays.length === 0) {
  throw new Error('No displays available')
}
const primaryDisplay = displays.find((d) => d.isPrimary) ?? displays[0]

2. onInstalled ハンドラーにエラーハンドリングなし

ファイル: packages/extension/src/background_script.ts

async 化された onInstalled ハンドラーに try-catch がありません。Settings.reset()ContextMenu.init() が失敗した場合、コンテキストメニューの初期化やバックアップ処理がサイレントに失敗します。

chrome.runtime.onInstalled.addListener(async (details) => {
  // try-catch がないため、エラーが発生してもキャッチされない
  if (details.reason === chrome.runtime.OnInstalledReason.INSTALL) {
    await Settings.reset()
  }
  await ContextMenu.init()
  // ...
})

修正案:

chrome.runtime.onInstalled.addListener(async (details) => {
  try {
    if (details.reason === chrome.runtime.OnInstalledReason.INSTALL) {
      await Settings.reset()
    }
    await ContextMenu.init()
    await checkAndPerformDailyBackup()
    await checkAndPerformWeeklyBackup()
  } catch (error) {
    console.error('Failed during onInstalled:', error)
  }
})

3. Service Worker 初期化にタイムアウト未指定

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

context.waitForEvent('serviceworker') にタイムアウトが指定されていないため、CI環境など特定の状況でテストが無限にハングアップする可能性があります。

sw = await context.waitForEvent('serviceworker')

修正案:

sw = await context.waitForEvent('serviceworker', { timeout: 10_000 })

🟡 重要度: MEDIUM

4. マジックナンバー(1280x800)を定数化すべき

ファイル: packages/extension/src/services/screen.ts

フォールバック値として 1280800 がコード内に複数回直接記述されています。定数化することで意図が明確になり、変更時の修正箇所も一元化されます。

// 複数箇所で直接書かれているマジックナンバー
height: win.height ?? 800,
width: win.width ?? 1280,
// ...
return { width: 1280, height: 800, left: 0, top: 0 }

修正案:

const DEFAULT_SCREEN_WIDTH = 1280
const DEFAULT_SCREEN_HEIGHT = 800

// 使用箇所
height: win.height ?? DEFAULT_SCREEN_HEIGHT,
width: win.width ?? DEFAULT_SCREEN_WIDTH,

5. ストレージデータの型キャストに型バリデーションなし

ファイル: packages/extension/e2e/fixtures.ts (約92-94行目)

ストレージから取得したデータを as Command で直接キャストしており、データが破損していた場合サイレントに失敗します。

const cmd = localData[localKey] as Command

テスト用コードとして許容範囲かもしれませんが、デバッグ時に混乱する可能性があります。


🟢 重要度: LOW

6. background_script.ts の孤立したエラーメッセージ

ファイル: packages/extension/src/background_script.ts

updateActiveScreenId() の呼び出しが削除されましたが、関連するエラーメッセージが残っている可能性があります。削除漏れがないか確認してください。


7. TestPage.ts の要素存在チェック不足

ファイル: packages/extension/e2e/pages/TestPage.ts

selectRange() メソッド内で startElementendElement の存在チェックなしに処理が進む場合、デバッグが困難なエラーになります。


✅ 良い点

  • アーキテクチャ改善: スクリーンサイズ取得をコンテンツスクリプトからバックグラウンドスクリプトに移動する設計は理にかなっており、IPCメッセージの簡素化につながっています。
  • フォールバック処理の充実: screen.ts のフォールバック処理(ターゲットディスプレイ → プライマリディスプレイ → 先頭ディスプレイ → ウィンドウサイズ → デフォルト値)は丁寧に実装されています。
  • E2Eテストの網羅性: 多様なシナリオ(起動方法・メニュースタイル・各コマンド種別・リンクプレビュー・設定・ハブ)がカバーされており、仕様書との対応も明確です。
  • エラーメッセージの修正: helper.ts"Failed to execute openPopups""Failed to execute openPopupWindow" / "Failed to execute openPopupWindowMultiple" への修正は、デバッグ性の向上に貢献しています。
  • await expect(menubar).toBeVisible(): startup.spec.ts での修正(expect(menubar.isVisible())await expect(menubar).toBeVisible())は、Playwright のベストプラクティスに沿った適切な修正です。

まとめ

主な懸念点は screen.ts の配列境界チェックと background_script.ts のエラーハンドリングです。これらは本番環境で稀に発生しうる問題のため、対応を検討していただければ幸いです。E2Eテストの追加は品質向上に大きく貢献しており、全体として価値の高いPRだと思います。

🤖 このレビューはClaude (claude-sonnet-4-6) によって自動生成されました。

@ujiro99 ujiro99 changed the title Feat/add e2e testcases WIP: Feat/add e2e testcases Mar 31, 2026
Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 31, 2026

@ujiro99 I've opened a new pull request, #351, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits April 1, 2026 00:02
Revert: Restore icons.svg sprite injection in content_script.tsx
@claude
Copy link
Copy Markdown

claude bot commented Apr 1, 2026

PR レビュー: WIP: Feat/add e2e testcases

全体として、E2Eテストの大幅な拡充とスクリーンサイズ取得ロジックのリファクタリングが適切に行われています。以下に気づいた点を記載します。


🐛 バグ・問題点

1. E2E-20 テストで importSettings() が二重呼び出しされている

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

beforeEach で既に importSettings() を呼び出しているにもかかわらず、E2E-20 のテスト本体でも再度呼び出しています。

// beforeEach でインポート済み
test.beforeEach(async ({ context, extensionId, getCommands }) => {
  const optionsPage = new OptionsPage(context, extensionId, getCommands)
  await optionsPage.open()
  await optionsPage.importSettings()  // ← ここで1回目
  await optionsPage.close()
})

test("E2E-20: ...", async ({ context, extensionId, getCommands, page }) => {
  const optionsPage = new OptionsPage(context, extensionId, getCommands)
  await optionsPage.open()
  await optionsPage.importSettings()  // ← 重複!
  await optionsPage.close()
  ...
})

テスト本体内の importSettings() 呼び出しは不要です。削除してください。


2. selectedLinkPopup.tsimport type が不足

ファイル: packages/extension/src/action/selectedLinkPopup.ts

import { OpenPopupsProps } from "@/services/chrome"  // 型のみ使用なのに value import になっている

OpenPopupsProps は型としてのみ使われているため、import type を使うべきです:

import type { OpenPopupsProps } from "@/services/chrome"

他の action ファイル(linkPreview.ts)では import type が正しく使われており、一貫性のためにも修正が必要です。


⚠️ 改善提案

3. sleep(500) がフラキーテストの原因になりうる

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

インポート後に固定の 500ms 待機を入れています:

await okButton.click()
await reloadPromise
await sleep(500)  // ← 根拠のない固定待機

固定の sleep はテスト環境の速度によってフレイキーになる可能性があります。Playwright の waitForexpect.poll などを使ったイベント駆動の待機に置き換えることを推奨します。もし既存の expect.poll で対応できない場合はコメントで理由を明記してください。


4. getCommands フィクスチャ内のマジックナンバー "5"

ファイル: packages/extension/e2e/fixtures.tsgetCommands フィクスチャ内)

const { 5: syncMetaData } = await chrome.storage.sync.get<{
  "5": CommandMetadata
}>("5")

ストレージキー "5" が何を意味するのかが不明瞭です。定数として定義されているものがあれば(例: SYNC_STORAGE_KEY.commandMetadata 等)それを参照するか、コメントで説明を追加してください。


5. タイポ: // Asert

ファイル: packages/extension/e2e/startup.spec.ts(E2E-11 テスト末尾付近)

// Asert  ← "Assert" のタイポ
await expect(menubar).toBeVisible()

6. getCommands フィクスチャでサービスワーカーを再取得している

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

context フィクスチャ内と getCommands フィクスチャ内でそれぞれサービスワーカーを取得しています:

// context フィクスチャ内
let [sw] = context.serviceWorkers()
if (!sw) {
  sw = await context.waitForEvent("serviceworker")
}

// getCommands フィクスチャ内(同様のコードが再び)
let [serviceWorker] = context.serviceWorkers()
if (!serviceWorker) {
  serviceWorker = await context.waitForEvent("serviceworker")
}

isAllWindowsNormal フィクスチャでも同様のパターンが繰り返されています。サービスワーカーの参照を共有するフィクスチャ(例: serviceWorker フィクスチャ)を1つ作成し、他のフィクスチャがそれを注入する形にすると、DRY 原則に沿いメンテナンスしやすくなります。


✅ 良い点

  • TestPage.selectText() のポーリング間隔を 500ms → 50ms に短縮したことで、テストの応答性が大幅に改善されています
  • XPath セレクタのサポート追加は柔軟性を高めています
  • aria-label={title}role="menuitem" の追加によりアクセシビリティが改善されています
  • MenuItem.tsx のアイコン画像を alt="" + aria-hidden="true" にした変更は正しいアクセシビリティ対応です
  • screen.tsgetScreenSize() リファクタリングにより、アクティブスクリーン追跡の複雑さが解消され、シンプルかつ堅牢になっています
  • waitForFunction から console.log を削除した改善は適切です
  • E2E テスト仕様書 (docs/test/e2e-test-spec.md) の追加は将来の保守性向上に貢献します

@ujiro99 ujiro99 changed the title WIP: Feat/add e2e testcases Feat/add e2e testcases Apr 1, 2026
@claude
Copy link
Copy Markdown

claude bot commented Apr 1, 2026

PR #345「Feat/add e2e testcases」コードレビュー

全体概要

このPRはPlaywrightベースのE2Eテストスイートを大幅に追加しており、起動方法・メニュースタイル・検索コマンド・単機能コマンド・PageAction・AiPrompt・リンクプレビュー・ユーザースタイル・設定画面・ハブという広範囲をカバーしています。40件以上のテストケースは非常に充実しており、プロダクションコードへのアクセシビリティ改善・data-testid 付与・screen.ts のリファクタリングも含まれています。


✅ 良い点

1. screen.ts の設計改善

packages/extension/src/services/screen.ts において、従来の「activeScreenId をBgDataに記録してから参照する」2段階アプローチを廃止し、getScreenSize() 呼び出し時に chrome.windows.getCurrent() で現在ウィンドウ位置を直接取得する方式に変更しています。updateActiveScreenId / windowExists の複数呼び出しによる競合状態を根本的に解消しており、DEFAULT_SCREEN_WIDTH = 1280 / DEFAULT_SCREEN_HEIGHT = 800 も定数として適切に管理されています。

2. アクセシビリティの改善(MenuItem.tsx

packages/extension/src/components/menu/MenuItem.tsxrole="menuitem"aria-label={title} を追加し、アイコン画像を alt="" aria-hidden="true" に変更しています。スクリーンリーダー対応の改善とともに、E2Eテストで [role='menuitem'][aria-label='xxx'] という安定したセレクタが使えるようになっています。

3. fixtures.ts のストレージキー定数化

packages/extension/e2e/fixtures.ts で旧来のハードコード "0" キーを STORAGE_KEY.USER 定数に置き換えており、ストレージ構造変更への追従性が向上しています。

4. テストIDの体系的な拡充

packages/extension/src/testIds.tsexportButtonresetButtonrecButtonpageActionStep()commandType() などを追加し、UIコンポーネントにも対応する data-testid を付与しています。実装詳細から切り離された安定したセレクタ設計です。

5. vite.config.ts のCSSバンドル名安定化

manualChunks.module.css"components" チャンクに集約し、insertCss(shadow, "/assets/components.css") での確実な読み込みが可能になっています。


⚠️ 問題点・懸念点

【高】1. E2E-46 が外部サービス(Amazon)に強依存しており不安定

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

await page.goto("https://www.amazon.com/")
await page.locator(".a-list-item .a-link-normal").first().click()
// ...
const productId = page.url().split("/dp/")[1].split("/")[0]
  • Amazonの DOM 構造や URL 形式(/dp/)への依存により、サイト変更で即座にテストが壊れます
  • .a-list-item .a-link-normal が商品一覧以外にヒットする可能性があります
  • /dp/ を含まないURLの場合、split("/dp/")[1]undefined になり 実行時エラー が発生します
  • 制御可能なテストサーバーやテストページを使用することを推奨します

【高】2. E2E-20 で importSettings の二重呼び出し

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

test.beforeEach で既に importSettings を実行しているにもかかわらず、E2E-20 のテスト本体内でも重複して呼び出しています。テスト実行時間の無駄になり、意図が不明確です。テスト本体内の重複呼び出しを削除してください。

【中】3. screen.ts フォールバックでウィンドウサイズをスクリーンサイズとして使用

ファイル: packages/extension/src/services/screen.ts

// Fallback: use the current window's size as a minimum screen estimate
const w = await chrome.windows.getCurrent()
return {
  width: w.width ?? DEFAULT_SCREEN_WIDTH,
  height: w.height ?? DEFAULT_SCREEN_HEIGHT,
  left: 0,
  top: 0,
}

ウィンドウが最大化されていない場合、実際の画面より小さい値が返り、ポップアップウィンドウの位置計算が狂う可能性があります。フォールバック時は DEFAULT_SCREEN_WIDTH / DEFAULT_SCREEN_HEIGHT を直接使用するか、コメントでこの意図を明記することを推奨します。

【中】4. as any 型アサーションの使用

ファイル: packages/extension/e2e/user-style.spec.ts(複数箇所)

{ name: "background-color", value: "#ff0000" } as any,

UserStyle 型に合わない構造を as any でキャストしており、型安全性を損ないます。UserStyle 型の定義に合わせた正しいオブジェクト構造に修正することを推奨します。

【中】5. waitForTimeout のハードコード値が多く、フレーキーテストのリスク

ファイル: packages/extension/e2e/pages/OptionsPage.ts および複数のスペックファイル

await page.waitForTimeout(500)   // hub.spec.ts
await page.waitForTimeout(700)   // OptionsPage.ts
await sleep(500)                 // OptionsPage.ts resetSettings()

固定時間待機はCIのマシン性能が低い環境でテストが失敗するリスクがあります。page.waitForLoadState()expect.poll()、または確定的なセレクタの waitFor() で代替することを推奨します。

【中】6. AiPrompt テストが外部サービス(Gemini)に依存

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

expect(popupPage.url()).toMatch(/gemini/)
const locator = popupPage.locator("text=以下について解説してください。")
await locator.waitFor({ state: "visible", timeout: 5000 })

外部サービスの応答時間やDOM構造変更に依存するため不安定です。URLの確認のみに留めるか、テストページにモックを用意することが望ましいです。

【中】7. Hub テストが外部サービスの状態に依存

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

外部サイト(ujiro99.github.io/selection-command)のDOMが変化した場合にテストが動作しなくなります。また、test.skip() 後に return が必須ですが、test.info().skip() を使う方が意図がより明確です。

【低】8. E2E-84 のリセット後検証が脆弱

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

expect(resetCommands.length).not.toBe(importedCommands.length)

デフォルトコマンド数がたまたまインポートしたファイルと同じになった場合、テストが誤って通過してしまいます。デフォルトコマンド数を定数として管理し、toBe(DEFAULT_COMMAND_COUNT) のように具体値で検証することを推奨します。

【低】9. logConsole.ts のエラーログが完全にサイレント

ファイル: packages/extension/e2e/utils/logConsole.ts

target.on("console", async (msg: ConsoleMessage) => {
  if (!process.env.PWDEBUG) return
  ...
})

PWDEBUG が未設定のCI実行時にはすべてのログが無視されます。console.error 等のエラーレベルはデバッグフラグなしでも出力する設計の方が、テスト失敗時の調査に役立ちます。

【低】10. OptionsPage.ts に独自の sleep 関数を定義

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

function sleep(msec: number): Promise<void> {
  return new Promise((resolve) => setTimeout(resolve, msec))
}

Playwright の page.waitForTimeout() が既に存在するため、この独自 sleep 関数は不要です。page.waitForTimeout に統一するか、確定的な待機手段を使用することを推奨します。

【低】11. TestPage.tsselectRange でメニュー表示を待機しない

ファイル: packages/extension/e2e/pages/TestPage.ts

selectText() と異なり selectRange() はメニューバーの表示を待機せず、イベントディスパッチ直後に処理を返します。呼び出し元(E2E-31)で直後に getMenuBar() を呼んでいますが、競合状態が生じる可能性があります。waitForMenu パラメータを追加して selectText() と同様の安全性を確保することを推奨します。


まとめ

評価項目 評価
テストカバレッジの広さ ✅ 非常に良い(40+テストケース)
テストの安定性 ⚠️ 改善余地あり(外部依存・固定タイムアウト)
ソースコード変更の品質 ✅ 良い(screen.ts改善・アクセシビリティ向上)
型安全性 ⚠️ 一部問題あり(as any
保守性 ✅ 良い(Page Objectパターン・testIds定数化)

最優先で対応いただきたいのは E2E-46 の Amazon 依存の排除(実行時エラーのリスクあり)と E2E-20 の二重 importSettings 呼び出しの削除 です。その他の指摘はテストの堅牢性向上に関わるものですが、致命的ではありません。

@claude
Copy link
Copy Markdown

claude bot commented Apr 1, 2026

PR レビュー: Feat/add e2e testcases

E2E テストの大規模追加と、それに伴うソースコードのリファクタリング全体を確認しました。全体的に品質は高く、多くの良い改善が含まれています。以下にフィードバックを記載します。


✅ 良い点

  • background_script.ts:463-497: onInstalled ハンドラを async/await + try/catch に変換したことで、初期化エラーが握りつぶされなくなりました。
  • fixtures.ts:148-162: ストレージキーのハードコード文字列 "0"STORAGE_KEY.USER 定数に置き換えたことで、保守性が向上しています。
  • MenuItem.tsx: name={title}aria-label={title} + role="menuitem" への変更はアクセシビリティの改善として適切です。
  • const.ts:4-6: __APP_VERSION__import.meta.env の安全なアクセスガードにより、E2E テスト環境(Vite 外)でのランタイムエラーを防いでいます。
  • E2E-23 のスキップ: サイドパネルはまだ未対応のため test.skip で適切に除外されています。

🐛 潜在的なバグ・問題

1. setUserSettings 後の伝播待機がない (E2E-11)
packages/extension/e2e/startup.spec.ts:26-38

await setUserSettings({ startupMethod: { ... } })
await testPage.selectText("h1, h2, h3", false)  // ← 設定変更の伝播を待たずにすぐ実行

chrome.storage.sync の変更がコンテンツスクリプト側の onChanged リスナーに伝播するまでに非同期の遅延があります。設定変更直後に selectText を呼ぶと、古い設定で動作するリスクがあります。setUserSettings 後に短いポーリング待機、またはストレージ変更イベントの監視を挟むことを検討してください。

2. page.waitForTimeout() の使用
packages/extension/e2e/single-command.spec.ts:46

await page.waitForTimeout(100)

Playwright 公式ドキュメントでは固定時間の waitForTimeout は非推奨とされています。代わりに条件ベースの待機(例: await expect(locator).toBeVisible()await page.waitForFunction(...) )を使用してください。フレーキーなテストの原因になりやすいです。

3. getCommands でのサービスワーカー重複取得
packages/extension/e2e/fixtures.ts:88-100

getCommands: async ({ context }, use) => {
  let [serviceWorker] = context.serviceWorkers()   // ← context fixture と重複
  if (!serviceWorker) {
    serviceWorker = await context.waitForEvent("serviceworker")
  }
  ...
}

同様のパターンが getUserSettingssetUserSettingsisAllWindowsNormal にも繰り返されています(計4箇所)。context fixture ですでにサービスワーカーを取得・待機しているにもかかわらず、各フィクスチャで再取得しています。context fixture からサービスワーカー参照を共有するか、専用の serviceWorker フィクスチャを作成することでコードの重複を排除できます。


⚠️ コード品質・ベストプラクティス

4. テスト内の UI 文字列ハードコード
packages/extension/e2e/search-command.spec.ts:50,63

menubar.locator("[role='menuitem'][aria-label='en to ja']").click()
menubar.locator("[role='menuitem'][aria-label='テストページ (Window)']").click()

コマンド名がテストデータファイル(test-settings.json)と一致している前提になっています。テストデータのコマンド名が変更された際にテストが無言で失敗するリスクがあります。TEST_IDS のような定数か、テストデータから動的に取得する方法を検討してください。

5. E2E-31 でのページ数比較が弱い
packages/extension/e2e/single-command.spec.ts:63-71

const newPageCount = context.pages().length
expect(newPageCount).toBeGreaterThan(initialPageCount)

「リンクポップアップがいくつ開いたか」は検証できません。具体的に何ページ増えることを期待するか(例: toEqual(initialPageCount + 2))をアサートすれば、テストの信頼性が上がります。

6. onInstalled での Settings.reset() の位置
packages/extension/src/background_script.ts:466-468

if (details.reason === chrome.runtime.OnInstalledReason.INSTALL) {
  await Settings.reset()
}

ContextMenu.init() より先に実行されていますが、Settings.reset() が失敗した場合 catch ブロックに入り ContextMenu.init() がスキップされます。Settings.reset() の失敗は致命的エラーとして扱わず、ログのみにして処理を継続するようにした方が安全です。


🔒 セキュリティ

特筆すべき問題はありません。serviceWorker.evaluate() への引数は適切に分離されており、テスト環境においてインジェクションリスクは低いです。


📊 テストカバレッジ

40以上のE2Eテストケースが追加されており、全体的なカバレッジは大幅に向上しています。特にPageAction(記録・再生)やLinkPreview、Hubのテストは複雑な操作を網羅しており良いと思います。

ファイル テスト数 対応する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) はフレーキーなテストにつながる可能性があるため、マージ前に対応を検討いただければと思います。

@ujiro99 ujiro99 merged commit c54c6f1 into main Apr 1, 2026
4 of 6 checks passed
@ujiro99 ujiro99 deleted the feat/add-e2e-testcases branch April 1, 2026 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants