test(e2e): hydration 待ち漏れ 3 spec を修正し漏れ検出 meta テストを追加 - #751
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017WJinoLkLbECJS5K74A5gr
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017WJinoLkLbECJS5K74A5gr
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017WJinoLkLbECJS5K74A5gr
🖼️ Visual Regression Test 結果
|
fumtas1k
left a comment
There was a problem hiding this comment.
レビュー総評: 承認相当(指摘はすべて非ブロッキング)
hydration race 修正 3 件 + 再発防止 meta テストの構成で、リポジトリの既存規約(vrt-pages-coverage の allowlist + 純粋関数パターン、test-gates skill の陽性対照必須)に忠実に沿った良質な PR です。以下、多角的観点での評価です。
✅ ロジック・正確性(検証済み)
PR 本文の主張をローカル checkout(develop 最新 e0e287f)で実際に検証しました:
- 棚卸しの正確性:
grep -Lで全 51 spec 中 helper 未参照は正確に 7 件で、修正 3 件・除外 4 件の分類と一致。 - PR base 以降の develop 前進(#749 マージ)とも整合: 最新 develop に対して meta テストの検知ロジックを当てても違反 0 件のままで、マージ後に meta テストが赤くなる衝突はなし。
- 除外判断の妥当性:
custom-404.spec.tsは/this-path-does-not-exist-xyzへの goto のみ、gate 系 2 件は/test-fixtures/*のみで、regex により自然除外されることを確認。prefers-reduced-motion.spec.tsは computed style 読取のみ(emulateMedia+getComputedStyle)で React イベント発火に依存せず、allowlist 登録は妥当。 - orphan 検出: 「実在しない」「helper 使用済み」「
/tools/へ goto しない」の 3 条件とも正しく、allowlist の腐敗を防げる設計。
✅ テスト設計
- 陽性対照 7 件を別 describe に分離し、fixture 注入可能な純粋関数へ切り出す構成は test-gates 準拠。さらに 実 spec の修正を一時的に戻して本体テストが fail することの実機確認まで行われており、「検知能力ゼロで green」(PR #233 事故クラス)への対策として十分。
har-viewer.spec.tsの対応方針(setViewportSize→ goto の順序を保つためローカルヘルパーopenHarViewerで置換、beforeEach集約を避ける)は正しい判断。ヘルパーの JSDoc に「viewport 変更は呼び出し前に」と制約が明記されている点も良い。8 箇所の置換に漏れなし(diff 上で確認)。- 検知の限界(変数経由 goto /
skipHydration経路)についてインラインで 2 点コメントしました。いずれも現状の実 spec では実害なしで、ドキュメント追記レベルの提案です。
✅ アーキテクチャ・規約適合
- vitest 設定の
includeにtests/meta/**が含まれており、npm run test(CI ゲート)で確実に実行される。__dirname利用も既存 meta テスト群と同一パターン。 - 修正 3 spec のコメント(race の因果 + issue 番号)は既存 spec の踏襲で適切な粒度。
- plan / spec ドキュメントの
docs/superpowers/配置、README.md/SPEC.md/docs/decisions.md更新不要の判断も規約どおり。
✅ セキュリティ
本番コードへの変更なし(テスト + ドキュメントのみ)。CSP・セキュリティ設定への影響なし。meta テストのファイル走査は tests/e2e/ 配下の読取のみで問題なし。
✅ フロントエンド / VRT
UI 変更なしのため観点対象外。VRT は bot 報告どおり全 74 件 pass。
パフォーマンス
loadSpecs() が 2 テストで各 51 ファイルを同期読取しますが、meta テスト全体で数十 ms オーダーであり問題なし。
結論
インライン 3 件(提案 2 + nit 1)はいずれもマージブロッカーではありません。ヘッダコメントへの限界追記のみ検討いただければ、このままマージして問題ない品質です。
Generated by Claude Code
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017WJinoLkLbECJS5K74A5gr
fumtas1k
left a comment
There was a problem hiding this comment.
再レビュー: 対応確認済み・LGTM ✅
c293d26 で前回レビューの指摘 3 件すべてに対応いただいたことを確認しました。
- 変数経由 goto の false negative → ヘッダコメントの「既知の限界」に明記。regex 拡張ではなくドキュメント化を選択した判断も妥当です(gate spec 2 件の allowlist 追加とのトレードオフを避けつつ、heuristic の穴を後続の読者に正しく伝えられる)。
skipHydration: trueの抜け道 → 同じく明記。「/tools/ページでの使用時は spec 側で待機を担保すること」という運用指針まで書かれており、提案以上の対応です。Page型表記の統一 →uploadHarの inline import 型をPageに統一。import は既存の 1 行目で解決されており型エラーなし。
差分は meta テストのコメント追記 + 型表記の置換のみでロジック変更なし。マージして問題ありません。
Generated by Claude Code
概要
Closes #750
ローカル並列実行で flaky だった E2E spec の hydration race を修正し、新規 spec での再発を CI で検知する meta テストを導入します。
変更内容
hydration 待ちの追加(3 spec)
dsn-builder.spec.ts/dummy-personal-data.spec.ts:beforeEachのpage.goto直後にawait waitForReactHydration(page);を追加(saml-decoder.spec.ts等の既存パターン踏襲)har-viewer.spec.ts:setInputFiles→ React onChange 依存で同一リスククラスのため棚卸しで追加対象に含めた。2 test が goto 前にsetViewportSizeを実行しておりbeforeEach集約では順序が変わるため、ローカルヘルパーopenHarViewer(page)(goto + hydration 待ち)で 8 箇所の goto を置換漏れ防止 meta テスト(
tests/meta/e2e-hydration-wait-coverage.test.ts)goto('/tools/...')を含む spec にwaitForReactHydration/withProductionCspの参照を必須化(vrt-pages-coverage.test.tsの allowlist + 純粋関数パターン踏襲)prefers-reduced-motion.spec.ts(computed style 読取のみで React イベント発火に依存しない)。orphan 検出付き棚卸し結果
waitForReactHydration/withProductionCsp未参照の 7 spec を精査。修正 3 件のほかは、custom-404/ gate 系 2 件(/tools/外への goto のみ → 検知対象外)、prefers-reduced-motion(allowlist 登録)で対応不要と判断。詳細はdocs/superpowers/specs/2026-07-19-e2e-hydration-wait-design.md。検証
npm run test: 147 files / 2675 tests 全件 pass(meta テスト 9 件含む)node_modules/.bin/astro check: エラー・warning 0 件npm run test:e2e全件: 385 passed / 1 skipped / 0 faileddsn-builder.spec.tsの修正を一時的に修正前へ戻すと meta テストがexpected [ 'dsn-builder.spec.ts' ] to deeply equal []で fail することを確認後、復元補足
README.md/SPEC.md/docs/decisions.mdの更新は不要(VRT 対象ページの変更もなし)waitForReactHydration自体の拡張は test(e2e): waitForReactHydration の label-aware 待ちオプション拡張余地 (PR #278 follow-up) #279 で別管理🤖 Generated with Claude Code
https://claude.ai/code/session_017WJinoLkLbECJS5K74A5gr
Generated by Claude Code