Repository navigation
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 26 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughPlaywrightによるE2Eテスト基盤を追加しました。認証、ルーム、対戦、通知、プロフィール、レスポンシブ表示、通信安全性、XSSなどの検証を追加しました。README、実行手順、 ChangesE2Eテスト基盤
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant DockerCompose
participant HTTPSApp
participant globalSetup
Playwright->>DockerCompose: E2E_STACKに応じたスタックを起動
DockerCompose->>HTTPSApp: Webサーバーを提供
Playwright->>globalSetup: グローバルセットアップを実行
globalSetup->>HTTPSApp: HTTPS HTMLを取得
globalSetup-->>Playwright: 実際のスタックを検証
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/LobbyPage.tsx (1)
131-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win折り返し後の操作領域を右寄せにしてください。
画面幅が狭く、左側と操作領域が同じ行に収まらない場合、親の
flex-wrapにより操作領域が次の行へ移動します。justify-betweenは各行に適用されるため、単独になった操作領域は左端に配置されます。操作領域にml-autoを追加するか、狭い画面でw-full justify-endを指定してください。修正例
- <div className="flex items-center gap-2 sm:gap-3"> + <div className="ml-auto flex items-center gap-2 sm:gap-3">As per path instructions: 「バグになる可能性」を最優先にし、レスポンシブ表示を確認する指示に基づきます。
🤖 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 `@frontend/src/pages/LobbyPage.tsx` around lines 131 - 155, Update the operation-controls container beside the LOBBY header to remain right-aligned when flex-wrap moves it onto a new row. Add the appropriate auto margin or responsive full-width/end-justification classes to the div containing the HOME and modal buttons, while preserving the current alignment when both groups share a row.Source: Path instructions
🧹 Nitpick comments (4)
frontend/playwright.config.ts (1)
21-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff動作を定義する数値に名前付き定数を使用してください。
テストの直列度、待機時間、HTTPS ポート、許可プレイヤー数がリテラルで分散しています。値の意味と変更範囲が不明確になります。
frontend/playwright.config.ts#L21-L40:workers、retries、テスト時間、サーバー起動時間、HTTPS ポートを名前付き定数にしてください。frontend/tests/e2e/global-setup.ts#L28-L28: 既定の HTTPS URL またはポートを名前付き定数にしてください。frontend/tests/e2e/helpers/friends.ts#L12-L18: 共通の UI 待機時間定数を使用してください。frontend/tests/e2e/helpers/rooms.ts#L4-L7: 許可プレイヤー数を名前付きの型または定数セットとして定義してください。frontend/tests/e2e/helpers/rooms.ts#L31-L31: 共通の UI 待機時間定数を使用してください。frontend/tests/e2e/helpers/rooms.ts#L45-L49: 共通の UI 待機時間定数を使用してください。As per coding guidelines, 「マジックナンバーを使用しない」に従ってください。
🤖 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 `@frontend/playwright.config.ts` around lines 21 - 40, Replace the scattered magic numbers with shared, descriptive constants: in frontend/playwright.config.ts (lines 21-40), name the workers, retries, test timeout, server-start timeout, and HTTPS port; in frontend/tests/e2e/global-setup.ts (line 28), reuse a named HTTPS URL or port constant; in frontend/tests/e2e/helpers/friends.ts (lines 12-18) and frontend/tests/e2e/helpers/rooms.ts (lines 31 and 45-49), reuse a common UI wait-time constant; and in frontend/tests/e2e/helpers/rooms.ts (lines 4-7), define the allowed player counts as a named type or constant set. Ensure the related configuration and helpers consume the shared constants consistently.Source: Coding guidelines
frontend/tests/e2e/specs/profile.spec.ts (1)
19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win検証閾値を名前付き定数にしてください。
50、6 * 1024 * 1024、および1は、ユーザー名上限とアバター容量超過の意味をコードから読み取れません。
MAX_USERNAME_LENGTH、MAX_AVATAR_SIZE_BYTES、OVERSIZED_AVATAR_SIZE_BYTESなどの定数を使用してください。上限値を変更するときに、テストの意図も維持できます。As per coding guidelines, 「マジックナンバーを使用しない」に従ってください。
Also applies to: 95-100
🤖 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 `@frontend/tests/e2e/specs/profile.spec.ts` around lines 19 - 20, Replace the magic values in the profile E2E test with descriptive named constants: use MAX_USERNAME_LENGTH for the username slice limit, MAX_AVATAR_SIZE_BYTES for the avatar size threshold, and OVERSIZED_AVATAR_SIZE_BYTES for the oversized-file increment. Apply these constants both at the shown username construction and the additionally affected avatar-size assertions around the referenced lines.Source: Coding guidelines
frontend/tests/e2e/specs/console-clean.spec.ts (1)
27-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
networkidleを画面ごとの準備完了条件に置き換えてください。
networkidleはネットワークが 500ms 以上止まってから処理を進めるため、WebSocket や定期リクエストの影響を受けると、テストが失敗または早期成功します。ブラウザ側の network idle 判定ではなく、各ルーティングの UI 要素や状態をexpect(...).toBeInTheDocument()/toBeVisible()などで待機してください。該当する
waitForLoadState('networkidle')は行 29、43、59、66 です。🤖 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 `@frontend/tests/e2e/specs/console-clean.spec.ts` around lines 27 - 30, Replace each waitForLoadState('networkidle') in the publicRoutes and other route-navigation loops with a route-specific UI readiness assertion. After each page.goto, wait for the expected page element or state using the existing expect-based visibility or presence checks, covering the occurrences at lines 29, 43, 59, and 66 while preserving each route’s intended validation.frontend/tests/e2e/specs/game-match.spec.ts (1)
11-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winゲーム時間の定数を重複定義しないでください。
BOMB_EXPLOSION_TIME_MSはshared/game-constants.ts:52の本体定数を複製しています。GAME_COUNTDOWN_MSも同じ時間契約に依存します。ゲーム側の値が変わると、このテストは誤った待機時間で実行されます。共有定数を参照してください。固定の
waitForTimeoutではなく、playingフェーズを示す観測可能なUIまたはイベントを待機してください。As per coding guidelines, "マジックナンバーを使用しない".
Also applies to: 53-55
🤖 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 `@frontend/tests/e2e/specs/game-match.spec.ts` around lines 11 - 20, Remove the local BOMB_EXPLOSION_TIME_MS and GAME_COUNTDOWN_MS definitions in the test, import and reuse the corresponding shared game constants instead of duplicating timing contracts. Replace fixed waitForTimeout-based synchronization with an observable UI state or event confirming the game has entered the playing phase before sending input.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Inline comments:
In `@docs/frontend/e2e-test.md`:
- Line 9: Update the directory-structure code fence in the E2E testing
documentation to declare the text language explicitly, changing the untyped
fence to a text-labeled fence while preserving its contents.
In `@frontend/tests/e2e/helpers/users.ts`:
- Around line 14-20: Update makeTestUser so username always retains the complete
random id suffix while remaining within the 50-character limit. Reserve the id
length before truncating label, then construct username from the truncated
label, separator, and id; preserve the existing email and password behavior.
In `@frontend/tests/e2e/specs/friends.spec.ts`:
- Around line 15-18: Isolate or clean up all E2E database writes at the suite
level so test data is removed after execution. Apply this to the listed setup
sites: frontend/tests/e2e/specs/friends.spec.ts lines 15-18 and 76-79;
frontend/tests/e2e/specs/notifications.spec.ts lines 16-19 and 59-62;
frontend/tests/e2e/specs/room-lobby.spec.ts lines 15-18, 69-72, and 99-102;
frontend/tests/e2e/specs/game-match.spec.ts lines 33-36;
frontend/tests/e2e/specs/game-multiplayer.spec.ts lines 21-23;
frontend/tests/e2e/specs/game-reconnect.spec.ts lines 22-25; and
frontend/tests/e2e/specs/match-history.spec.ts lines 22-36. Ensure each
scenario’s users, relationships, notifications, rooms, invitations, match
results, and ranking data are created in a database discarded when the suite
ends.
In `@frontend/tests/e2e/specs/game-reconnect.spec.ts`:
- Around line 37-50: Replace the guestContext.setOffline(true) approach in the
reconnect test with an actual WebSocket disconnect mechanism, such as
client-initiated closure or routeWebSocket interception, so the reconnection
flow is exercised. Preserve the existing host stability assertion, then restore
the connection and verify the guest reloads into the game URL with the canvas
visible.
In `@frontend/tests/e2e/specs/secure-connection.spec.ts`:
- Around line 21-27: Replace the fixed waitForTimeout in the Socket.IO
connection check with an explicit readiness wait: observe Playwright WebSocket
creation and wait for a connection whose URL uses wss:, or wait for the
application’s established-connection state. Perform the insecureRequests
assertion only after that connection-completion condition succeeds.
In `@frontend/tests/e2e/specs/xss.spec.ts`:
- Around line 17-50: Extend the XSS assertions in the test around the existing
hostPage dialog, __xssFired, and injectedImg checks to also monitor guestPage,
including dialog collection, execution-state evaluation, and img DOM inspection.
Ensure both sender-side local echo and host-side rendering display the payload
only as text and produce no dialogs or XSS execution; extract shared checks into
a helper if that keeps the assertions consistent.
- Around line 71-80: 固定の roomName をやめ、30文字以内で実行ごとに一意な suffix を含む短い XSS payload
を生成して createRoomViaUi に渡してください。getByText(roomName) の .first()
を外し、作成した一意なルーム名との完全一致を検証したうえで、既存の img payload 非実行確認を維持してください。
In `@Makefile`:
- Around line 70-72: Update the dev-deps Docker command to run npm ci as the
invoking host user rather than the container’s default root user, using the host
UID and GID in the docker run configuration so generated node_modules files
remain host-owned.
In `@README.md`:
- Around line 228-235: Align the GNU Make requirement in the README with the
commands used in the documented procedures around make build, make, and make
logs. Either mark GNU Make as required, or provide equivalent docker compose
commands for every referenced make target so users without GNU Make can follow
the same setup and operation steps.
- Around line 70-73: README.mdのPostgreSQLおよび起動手順の説明を開発環境と本番環境に分けて整理してください。`npm
run
start:dev`が開発用`docker/backend/Dockerfile`の手順であることを明記し、本番環境ではコンパイル済みNestJS出力を`node`で起動する既存の説明と対応させてください。
---
Outside diff comments:
In `@frontend/src/pages/LobbyPage.tsx`:
- Around line 131-155: Update the operation-controls container beside the LOBBY
header to remain right-aligned when flex-wrap moves it onto a new row. Add the
appropriate auto margin or responsive full-width/end-justification classes to
the div containing the HOME and modal buttons, while preserving the current
alignment when both groups share a row.
---
Nitpick comments:
In `@frontend/playwright.config.ts`:
- Around line 21-40: Replace the scattered magic numbers with shared,
descriptive constants: in frontend/playwright.config.ts (lines 21-40), name the
workers, retries, test timeout, server-start timeout, and HTTPS port; in
frontend/tests/e2e/global-setup.ts (line 28), reuse a named HTTPS URL or port
constant; in frontend/tests/e2e/helpers/friends.ts (lines 12-18) and
frontend/tests/e2e/helpers/rooms.ts (lines 31 and 45-49), reuse a common UI
wait-time constant; and in frontend/tests/e2e/helpers/rooms.ts (lines 4-7),
define the allowed player counts as a named type or constant set. Ensure the
related configuration and helpers consume the shared constants consistently.
In `@frontend/tests/e2e/specs/console-clean.spec.ts`:
- Around line 27-30: Replace each waitForLoadState('networkidle') in the
publicRoutes and other route-navigation loops with a route-specific UI readiness
assertion. After each page.goto, wait for the expected page element or state
using the existing expect-based visibility or presence checks, covering the
occurrences at lines 29, 43, 59, and 66 while preserving each route’s intended
validation.
In `@frontend/tests/e2e/specs/game-match.spec.ts`:
- Around line 11-20: Remove the local BOMB_EXPLOSION_TIME_MS and
GAME_COUNTDOWN_MS definitions in the test, import and reuse the corresponding
shared game constants instead of duplicating timing contracts. Replace fixed
waitForTimeout-based synchronization with an observable UI state or event
confirming the game has entered the playing phase before sending input.
In `@frontend/tests/e2e/specs/profile.spec.ts`:
- Around line 19-20: Replace the magic values in the profile E2E test with
descriptive named constants: use MAX_USERNAME_LENGTH for the username slice
limit, MAX_AVATAR_SIZE_BYTES for the avatar size threshold, and
OVERSIZED_AVATAR_SIZE_BYTES for the oversized-file increment. Apply these
constants both at the shown username construction and the additionally affected
avatar-size assertions around the referenced lines.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 08e75d3e-aa47-4212-851e-9a4add4b2852
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
MakefileREADME.mddocs/frontend/e2e-test.mdfrontend/.gitignorefrontend/package.jsonfrontend/playwright.config.tsfrontend/src/pages/LobbyPage.tsxfrontend/tests/e2e/global-setup.tsfrontend/tests/e2e/helpers/auth.tsfrontend/tests/e2e/helpers/console.tsfrontend/tests/e2e/helpers/friends.tsfrontend/tests/e2e/helpers/rooms.tsfrontend/tests/e2e/helpers/users.tsfrontend/tests/e2e/specs/auth.spec.tsfrontend/tests/e2e/specs/concurrent-users.spec.tsfrontend/tests/e2e/specs/console-clean.spec.tsfrontend/tests/e2e/specs/form-validation.spec.tsfrontend/tests/e2e/specs/friends.spec.tsfrontend/tests/e2e/specs/game-match.spec.tsfrontend/tests/e2e/specs/game-multiplayer.spec.tsfrontend/tests/e2e/specs/game-reconnect.spec.tsfrontend/tests/e2e/specs/legal-pages.spec.tsfrontend/tests/e2e/specs/match-history.spec.tsfrontend/tests/e2e/specs/notifications.spec.tsfrontend/tests/e2e/specs/profile.spec.tsfrontend/tests/e2e/specs/responsive.spec.tsfrontend/tests/e2e/specs/room-lobby.spec.tsfrontend/tests/e2e/specs/secure-connection.spec.tsfrontend/tests/e2e/specs/xss.spec.ts
Owner
|
ちょっとこの量は見切れないけど、テスト追加ってことなら入れていいかな。ありがとう |
cimura
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
docs/frontend/e2e-test.mdを追加しました。対応背景
詳細
frontend/playwright.config.ts）:docker/docker-compose.prod.yml）をwebServerとして自動起動します。dev スタックは Vite の HMR クライアントなどが console に出るため、「コンソールにエラー・警告が無いこと」の判定が本番と食い違います。dev で動かしたい場合のみE2E_STACK=devを指定します。Map（単一プロセス）で共有されるため、並列実行はせずworkers: 1にしています。tests/e2e/global-setup.ts）:reuseExistingServerにより既に 8443 を握っている別スタックを検知できないため、応答 HTML から dev / prod を判定し、E2E_STACKの指定と食い違っていれば実行前にエラーで止めます。tests/e2e/specs/）: 機能単位でファイルを分割しています。auth.spec.ts）、フォームのバリデーション（form-validation.spec.ts）xss.spec.ts）、混在コンテンツの不在（secure-connection.spec.ts）legal-pages.spec.ts）、レスポンシブ表示（responsive.spec.ts）、コンソール出力（console-clean.spec.ts）profile.spec.ts）、フレンド機能（friends.spec.ts）、通知（notifications.spec.ts）room-lobby.spec.ts）、複数ユーザーの同時利用（concurrent-users.spec.ts）game-match.spec.ts）、4人対戦（game-multiplayer.spec.ts）、切断と再接続（game-reconnect.spec.ts）、戦績と対戦履歴（match-history.spec.ts）tests/e2e/helpers/）:Makefile）:node_modulesは各コンテナの匿名ボリュームに閉じており、ホストでnpm installしてもコンテナには反映されません。ホストで Playwright を動かすための依存関係だけを Dockerfile と同じnode:24-alpineで揃えるmake dev-depsを追加しました（ホストの Node バージョン差によるpackage-lock.jsonのブレを防ぐため）。動作確認
make dev-depsとnpx playwright install --with-deps chromiumでセットアップできること。cd frontend && npm run test:e2eで本番相当スタックが自動起動し、全テストがパスすること。global-setupがエラーで停止すること。E2E_STACK=dev npm run test:e2eで開発用スタックに対して実行できること。npm run test:e2e:reportで HTML レポートが開けること。テスト
その他
Summary by CodeRabbit
ドキュメント
テスト
改善