test(e2e): Playwright スイートを CI で全 spec 実行できる状態に復帰させる (#1036) - #1045
Conversation
…orTimeout issue #1036 Phase 3 の CI 取り込み。spec 側の書き直しは後続コミットで入る。 - e2e-web job で全 web spec と admin/e2e/errors-page.spec.ts を実行 - CONTRIBUTING に waitForTimeout 新規使用禁止を明記 https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
issue #1036。バックエンド無し環境で決定化するため、REST を page.route、 Hocuspocus を page.routeWebSocket(Yjs sync ハンドシェイク)でモックする 共有基盤 e2e/support/ を追加。 - linked-pages: 7 断片テストをクリティカルジャーニー 2 本に統合 - wiki-link-ghost-completion: issue #930 の受け入れ条件 8 件を維持しつつ waitForTimeout を全廃(refs #977) - soft assertion / skip も全廃 https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
issue #1036。検索ダイアログ廃止後のヘッダー検索バー + ドロップダウンに 合わせて search.spec.ts を全面再設計(旧 skip 6 件は存在しない UI のテスト のため削除)。web-clipper は /notes/me 解決に必要な API モックを追加し、 networkidle 依存を状態ベース待機へ置換。 https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
issue #1036。compose 画面到達に必要な notes/pages 系モックを追加。 brief-card 表示までは到達するが、resume 後の継続とレイアウトの 2 点で 実装側の乖離候補があり現状 red(トリアージ中。修正は後続コミット)。 https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
react-resizable-panels v4 は ARIA window-splitter 規約どおり separator 自身の 見た目の向きを aria-orientation に設定する(左右分割 → vertical)。旧クラスは これを「上下分割」と逆に解釈して w-full を適用しており、デスクトップの 左右分割(Wiki Compose 画面)で separator が全幅化 → 両パネルが幅 0 に潰れて 操作不能になっていた。issue #1036 の E2E 再整備で発見。 - aria-[orientation=vertical] 系の variant を horizontal に反転 - grip アイコンの rotate-90 条件も追随 - ResizablePanelGroup の dead class(Group は aria-orientation を持たない)を削除 - orientation 契約を固定する component test を追加(TDD: red→green 確認済み) https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
issue #1036。spec-extractor でサーバ実装まで確認した結果、フェーズ遷移は resume(PATCH) の JSON 応答(output.__interrupt__[0].value.kind)駆動で、 SSE は初回 run の 1 回のみが正しい契約だった(旧 spec の run 再発行前提が誤り)。 - SSE を data:<JSON> 形式・初回 1 回に修正、2 回目以降の run / 4 回目以降の resume には実サーバ同様 409 を返し契約違反の検知器とする - GET compose-sessions/:id を追加(/compose/:sessionId への replace 後の再マウント対応) - 既存アサーション(brief→research→outline→completed→back)は全て維持 - 3 回連続 green 確認(workers=1, retries=0) https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
アサーション強度レビューの指摘対応。w-px の検証が生 className への部分一致 だったため after:w-px 等でも通る穴があった。トークン完全一致に統一。 https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
issue #1036 Phase 4(アサーション強度レビュー)の指摘対応。 - wiki-compose: resume PATCH の送信ボディ(answers/approvedSourceIds/sections) を検証し自己充足構造を解消(high) - ghost-completion: インラインコード/コードブロック抑止テストに正のシグナル (code 要素への反映確認)を先行させ偽 green を排除(high) - search: 件数 exact 一致・mark のカードスコープ・Escape 後の入力値保持 - web-clipper: レガシーリダイレクトの最終 URL 固定、不在チェック前の描画確認 - linked-pages: 閉じた [[...]] テストにマーク存在の正のシグナル追加 - mockBackend: body.id フォールバック削除、未 seed ページの public-links を 404 に https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds Playwright REST and WebSocket mocks, rewrites multiple E2E specs to use deterministic state/network-driven waits (no new waitForTimeout), updates CI to run full web+admin suites, documents the E2E policy in CONTRIBUTING, and adds ResizableHandle ARIA tests and minor component adjustments. ChangesBackend-less E2E Testing Infrastructure & Policy
ResizableHandle ARIA Orientation Testing
Sequence Diagram(s)sequenceDiagram
participant TestSpec
participant PlaywrightBrowser
participant MockBackend
participant MockRealtime
TestSpec->>MockBackend: installMockBackend(page)
TestSpec->>MockRealtime: mockRealtime(page)
PlaywrightBrowser->>MockBackend: GET/POST /api/* (note/page/public-links)
MockBackend->>PlaywrightBrowser: wire-format JSON responses
PlaywrightBrowser->>MockRealtime: WebSocket auth/sync messages
MockRealtime->>PlaywrightBrowser: AUTH_AUTHENTICATED / sync responses
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the E2E testing suite to enforce a strict policy against page.waitForTimeout(), replacing fixed sleeps with state-based assertions to prevent flaky test runs. To facilitate backend-less testing, new in-memory mocks for the REST API (mockBackend.ts) and the Hocuspocus WebSocket server (mockRealtime.ts) have been introduced, and several E2E test specs have been rewritten to use them. Additionally, a layout bug in the ResizableHandle component was fixed by correcting the aria-orientation class mappings to align with react-resizable-panels v4, accompanied by a new unit test suite. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3134d94c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| orientation={direction} | ||
| className={cn("flex h-full w-full aria-[orientation=vertical]:flex-col", className)} | ||
| /> | ||
| <Group {...props} orientation={direction} className={cn("flex h-full w-full", className)} /> |
There was a problem hiding this comment.
Restore vertical panel stacking
When a caller uses direction="vertical" (for example WikiComposePage does this on mobile), the group now still renders only flex/row layout because this line removed the conditional flex-col class and does not replace it with any direction-based class. The handle may switch to a horizontal separator, but the two panels remain side-by-side instead of stacking top/bottom, so the mobile compose layout is broken; keep a vertical-direction class on the group (using the attribute v4 actually emits, or an explicit direction === "vertical" && "flex-col").
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
検証の結果、この指摘は当たらないと判断しました。react-resizable-panels v4 の Group は flex 方向をインライン style で直接設定します(dist 実装: flexDirection: orientation === "horizontal" ? "row" : "column")。そのため direction="vertical" の縦積みはライブラリ側で担保されており、削除した aria-[orientation=vertical]:flex-col はそもそも発火しない dead class でした(v4 の Group 要素は aria-orientation 属性を持ちません)。仮にクラスが効いてもインライン style が優先されます。
退行防止として、resizable.test.tsx に「horizontal → flex-direction: row / vertical → flex-direction: column(インライン style)」を固定するアサーションを追加しました(b241df0)。
Generated by Claude Code
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/resizable.tsx (1)
11-13: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd TSDoc documentation for exported components.
Both
ResizablePanelGroupandResizableHandleare exported components but have empty JSDoc comments. As per coding guidelines, exported functions, types, and interfaces must include TSDoc/JSDoc documentation in both Japanese and English.📝 Proposed fix to add bilingual TSDoc
/** + * リサイズ可能なパネルグループ(左右分割または上下分割)。 + * + * Resizable panel group (horizontal or vertical split). + * + * `@param` direction - 分割方向: "horizontal"(左右)または "vertical"(上下) / Split direction: "horizontal" (left/right) or "vertical" (top/bottom) + * `@param` className - 追加の CSS クラス名 / Additional CSS class names */ const ResizablePanelGroup = ({/** + * リサイズ可能なパネル間のセパレーター(ドラッグハンドル)。 + * + * Resizable separator (drag handle) between panels. + * + * `@param` withHandle - ハンドルアイコンを表示するか / Whether to display the handle icon + * `@param` className - 追加の CSS クラス名 / Additional CSS class names */ const ResizableHandle = ({ withHandle, className, ...props }: ResizableHandleProps) => (Also applies to: 28-30
🤖 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 `@packages/ui/src/components/resizable.tsx` around lines 11 - 13, Add proper bilingual (Japanese and English) TSDoc comments for the exported components ResizablePanelGroup and ResizableHandle: replace the empty JSDoc block with TSDoc for each exported symbol that includes an English summary, a Japanese summary, descriptions of props/parameters (with types/meanings), and a brief usage example or note about behavior/side-effects; ensure the TSDoc tags (`@param`, `@returns/`@remarks) are present and written in both languages to satisfy the project's documentation guidelines.Source: Coding guidelines
🧹 Nitpick comments (1)
e2e/web-clipper.spec.ts (1)
145-149: 💤 Low valueConsider string-based URL assertion for clarity.
The current RegExp construction from
NOTE_IDis flagged by static analysis (CWE-1333), though it's safe here sinceNOTE_IDis a hardcoded constant. For simpler test code, consider using a string-based URL predicate:- await expect(page).toHaveURL(new RegExp(`/notes/${NOTE_ID}(\\?|$)`)); + await expect(page).toHaveURL((url) => url.pathname === `/notes/${NOTE_ID}` || url.pathname.startsWith(`/notes/${NOTE_ID}?`));or if only checking the pathname without query:
- await expect(page).toHaveURL(new RegExp(`/notes/${NOTE_ID}(\\?|$)`)); + await page.waitForURL((url) => url.pathname === `/notes/${NOTE_ID}`); + await expect(page).toHaveURL((url) => url.pathname === `/notes/${NOTE_ID}`);🤖 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 `@e2e/web-clipper.spec.ts` around lines 145 - 149, Replace the RegExp-based assertion with a string- or pathname-based check to avoid dynamic regex construction: in the test using NOTE_ID and page.toHaveURL, change the line using await expect(page).toHaveURL(new RegExp(`/notes/${NOTE_ID}(\\?|$)`)); to a string predicate such as await expect(page).toHaveURL(`/notes/${NOTE_ID}`); or, if you only want to ignore query params, read the current URL via page.url(), build a URL object and assert on url.pathname === `/notes/${NOTE_ID}`; update the assertion in the test that references NOTE_ID and page.toHaveURL accordingly.Source: Linters/SAST tools
🤖 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 `@e2e/support/mockBackend.ts`:
- Around line 67-76: Convert the existing bilingual block comment above the
exported function installMockBackend into a proper JSDoc/TSDoc comment
immediately preceding the function declaration: start with /** and include a
short English description, the Japanese equivalent if desired, param tags for
page and options (with noteId?), and a `@returns` tag describing
Promise<MockBackend>; ensure the JSDoc is syntactically valid (use `@param` and
`@returns`) and remains directly above the export async function
installMockBackend(...) so documentation tools and linters recognize it.
- Around line 24-54: Convert the inline bilingual comments into proper
JSDoc/TSDoc blocks for all exported interfaces: add a JSDoc comment above the
MockPageRow, MockPublicLinks, and MockBackend interface declarations describing
their purpose, and add JSDoc comments for key properties/methods (e.g., id,
note_id, title, content_preview, thumbnail_url, source_url, is_deleted,
created_at, updated_at on MockPageRow; outgoing_links, backlinks, ghost_links on
MockPublicLinks; noteId, seedPage, setPublicLinks, unhandled on MockBackend)
using standard /** ... */ format so the exported types have proper documentation
and comply with the coding guidelines.
In `@e2e/support/mockRealtime.ts`:
- Around line 28-62: The exported function mockRealtime is missing proper
JSDoc/TSDoc; replace the bilingual inline comment with a JSDoc block above the
exported async function mockRealtime describing its behavior, add `@param` page -
Playwright Page (or appropriate Page type) and `@returns` Promise<void>, and
include a short sentence about the mock behavior (minimal Hocuspocus mock that
responds to AUTH and SYNC messages and triggers onSynced). Ensure the JSDoc is
immediately above the mockRealtime declaration and follows project TSDoc style.
---
Outside diff comments:
In `@packages/ui/src/components/resizable.tsx`:
- Around line 11-13: Add proper bilingual (Japanese and English) TSDoc comments
for the exported components ResizablePanelGroup and ResizableHandle: replace the
empty JSDoc block with TSDoc for each exported symbol that includes an English
summary, a Japanese summary, descriptions of props/parameters (with
types/meanings), and a brief usage example or note about behavior/side-effects;
ensure the TSDoc tags (`@param`, `@returns/`@remarks) are present and written in
both languages to satisfy the project's documentation guidelines.
---
Nitpick comments:
In `@e2e/web-clipper.spec.ts`:
- Around line 145-149: Replace the RegExp-based assertion with a string- or
pathname-based check to avoid dynamic regex construction: in the test using
NOTE_ID and page.toHaveURL, change the line using await
expect(page).toHaveURL(new RegExp(`/notes/${NOTE_ID}(\\?|$)`)); to a string
predicate such as await expect(page).toHaveURL(`/notes/${NOTE_ID}`); or, if you
only want to ignore query params, read the current URL via page.url(), build a
URL object and assert on url.pathname === `/notes/${NOTE_ID}`; update the
assertion in the test that references NOTE_ID and page.toHaveURL accordingly.
🪄 Autofix (Beta)
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
Run ID: 3a9a5c25-3113-4cc0-a03b-5db0a6cbfc62
📒 Files selected for processing (11)
.github/workflows/ci.ymlCONTRIBUTING.mde2e/linked-pages.spec.tse2e/search.spec.tse2e/support/mockBackend.tse2e/support/mockRealtime.tse2e/web-clipper.spec.tse2e/wiki-compose.spec.tse2e/wiki-link-ghost-completion.spec.tspackages/ui/src/components/resizable.test.tsxpackages/ui/src/components/resizable.tsx
- knip 失敗の修正: e2e/support/mockRealtime.ts が直接 import する lib0 (yjs の推移的依存)を devDependencies に明示 - CodeRabbit 指摘: mockBackend / mockRealtime の公開 API に JSDoc を補強 - resizable.test: 縦積み(direction=vertical)がライブラリのインライン flex-direction で機能する契約を固定(レビュー指摘の検証として追加。 削除した aria-[orientation=vertical]:flex-col は dead class であり 縦積みは壊れていない) https://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
Closes #1036
Closes #977
概要
E2E スイート(7 spec)を CI で全 spec 実行できる状態に復帰させる。棚卸しの結果、問題は flaky ではなくスイート全体のアーキテクチャ乖離だった(ローカル実行で 22 failed / 7 passed / 12 skipped):
createNewPageヘルパー依存のテストが全滅変更内容
E2E モック基盤の新設(
e2e/support/)mockBackend.ts: REST API の in-memory モック(page.route、snake_case ワイヤ形式、未知パスは 404 で fail-loud)mockRealtime.ts: Hocuspocus WS モック(page.routeWebSocket+ 最小 Yjs sync ハンドシェイク)。バックエンド無しでエディタ系ジャーニーを決定化spec の書き直し / 修復
search.spec.ts: ヘッダー検索バー + ドロップダウン向けに全面再設計。skip 6 件削除linked-pages.spec.ts: 断片化した 7 テストをクリティカルジャーニー 2 本に統合wiki-link-ghost-completion.spec.ts: issue インラインゴースト補完(タイピング中の薄色サジェスト) #930 の受け入れ条件 8 件を維持しつつwaitForTimeout全廃(refactor(e2e): Replace waitForTimeout with state-based waits in wiki-link-ghost-completion.spec.ts #977 もこれで解消)web-clipper.spec.ts:/notes/me解決に必要なモック追加、networkidle依存を状態待機へwiki-compose.spec.ts: resume 契約の誤り(run 再発行前提)をサーバ実装確認済みの正しい契約(resume 応答ボディ駆動)に修正。契約違反検知として 2 回目 run / 4 回目 resume に 409 を返すpdf-knowledge.spec.ts: 変更なし(fixme6 件は Tauri 基盤待ち [Test] PDF知識化フローの Playwright E2E (#389 follow-up) #863 follow-up の既存方針のまま)発見した製品バグの修正
packages/uiResizableHandle の aria-orientation 解釈が逆で、デスクトップの Wiki Compose 画面が全ユーザーで操作不能だった(separator が全幅化しパネルが幅 0 に潰れる)。TDD(red→green)で修正、orientation 契約の component test を追加CI / 規約
e2e-webjob: 全 spec 実行 + admin spec(ポート 30001)実行に更新、レポートを web / admin 別々にアップロードpage.waitForTimeout新規使用禁止を明記(各 spec ヘッダにも日英併記)検証
--workers=1 --retries=0の CI より厳しい条件)。実行時間 22.5 分 → 約 1 分tsc --noEmitクリーン、eslint 0 errors残課題(Issue 化候補・本 PR では未対応)
"content"ハードコードで、タイトル一致でも常に「本文」バッジになる[[直後の Enter が候補 0 件時に飲み込まれるhttps://claude.ai/code/session_01K9WBLzsePd6oip4JYcVJEp
Summary by CodeRabbit
Tests
Documentation
Bug Fixes