🐈

AIと進めるテスト設計・リファクタリングの実践

に公開

以前作成した冷蔵庫マネージャー、レビューとリファクタリングをしようしようと思いながら後回しになっていたのですが、つい先日参加したイベントでAIとのリファクタリングのコツを教えてもらったので、教えてもらったことをもとにやってみることにしました。

  • 前提(例:あなたは〜です)
  • 目的(例:これから〜を実装します)
  • 前準備(例:まず〜を把握してください)
  • タスク(例:調査結果をマークダウンファイルにしてください)
  • 制約(例:コードの変更を禁止する など)

これを意識してリファクタリングをしていきます。
ちなみに今回もCursorとペアプロ形式で進めています。

1. ドキュメントを作る

生成されたコードを何の情報もなく読んでいくのは根気がいります。ということで、まずは構成とかパッケージをドキュメントに起こしてもらいます。
これをすることでCursorでチャットを新しくした場合でも「これも参照してね」で内容を引き継げますし、何より人間にとっても有用な情報です。

あなたはこのアプリの構築に新規参加するエンジニアです。
これから、コンポーネントやテストコードのレビューおよびリファクタリングを行います。
まず、アプリの構成・使用されているパッケージ等を調査してください。調査結果は、knowledge フォルダ内にマークダウンファイルを分けて保存してください。
今回は全体像の把握が目的なので、既存のコードの変更は禁止します。
knowledge フォルダ内のみ生成・編集が可能です。

これでアプリの概要と使用しているパッケージリストがmdファイルで生成されましたが、どういうReactコンポーネントが実装されているかまでは書いてくれなかったので、追加で指示を出してまとめてもらいました。

また、TODO.mdに書いていたテスト方針も、改めてテスト用のナレッジファイルとしてまとめなおし、この時点で計4ファイルになりました。(あとでユーティリティ用のファイルができたので、最終的に5ファイルになりましたが)

ここで一旦チャットを新しくして、プロジェクトの情報についてproject-overview.mdをもう少し詳しく見ていきます。
最初にファイルを作ったときは、目的セクションが「目的(推測)」で書かれていたので、ちゃんとコード読んで概要書いて!って頼んで、生成してもらいました。

## アプリケーション概要

- 冷蔵庫内の食材を管理するためのシンプルなWebアプリケーション。
- ユーザーは食材名と賞味期限を登録できる。
- 登録された食材は一覧表示され、賞味期限が近づくと視覚的にわかるようになっている(カードの色変化など)。
- 各食材は個別に削除可能。
- データはブラウザのローカルストレージに保存される(永続化)。

ここまでできたら、人間だろうとAIだろうと「アプリ開発に参加した時に最初に見れば良いファイル群」は整備できたと判断して、ようやくコードレビューおよびリファクタリングに入っていきます。

2. コードレビュー

App.test.tsxの追加

カバレッジテストのコードがそもそもなかったので追加し、カバレッジを見てみます。
元々、FoodCard.tsxFoodForm.tsxだけテストが書かれてる状態だったので、App.tsxmain.tsx、そしてshadcn/uiで生成されたコードが0%で表示されていました。
shadcn/uiのテストは別に手元ではいらんだろう…と思ったので除外しましたが、あとの2つ、App.tsxmain.tsxについてどうするか一旦Cursorに聞いてみます。

私の考えとしてはmain.tsxはいらないけど、App.tsxはいるんじゃない?という考えがあってるのかどうかの裏付け的な感じです。

カバレッジテストを実行したところ、App.tsxとmain.tsxのテストコードを書いていないので0%なのですが、そもそもこの2つのファイルにテストコードはいると思いますか?

やっぱりCursorもApp.tsxは入れた方がよい、という返答だったので、docs/knowledge/testing-overview.md の方針に基づいて、App.test.tsx を作成してもらいました。このとき採用したのはgemini-2.5-pro-exp-03-25です。

テストファイルを作ってもらってテストを実行したけど、

  • App コンポーネント > localStorage にデータがある場合、初期表示時に食材カードが表示されること
  • App コンポーネント > 食材が追加されると(フォームが閉じた後)、リストが更新され新しい食材が表示されること

のテストがコケ続けていました。
私のCursor Rulesの設定上、3回同じ失敗したら一回止めてねと言ってるので、Geminiは次のステップを提示して、エラー解消試行が止まりました。

根本原因の再考察と次のステップ:
getAllFoodItems が予期せず複数回呼ばれる、あるいは state 更新がうまく反映されない、という挙動は、コンポーネントのレンダリングサイクルや useEffect の依存関係、あるいは Vitest のモックの挙動自体に何か予期せぬ問題がある可能性を示唆しています。
これ以上 App.test.tsx だけを修正しても解決は難しいかもしれません。同じエラーの修正に3回以上失敗したため、カスタムインストラクションに基づき、これ以上の修正試行を中断します。
現状報告:
App.tsx のコンポーネントテストにおいて、以下の問題が発生しており、修正を試みましたが解決に至りませんでした。
初期表示(データあり)テストの失敗: getAllFoodItems が期待以上に呼び出され、結果としてテストデータが表示されない。
食材追加テストの失敗: フォームを閉じても getAllFoodItems が期待通りに再実行されず、DOMに新しい食材が表示されない。
食材削除テストの失敗: 初期表示の時点でデータが表示されず、テストが開始できない。
考えられる原因:
App コンポーネントの useEffect の依存関係配列が空 ([]) であるため、意図しない再レンダリング時に loadFoodItems が再実行されている可能性(ただしコード上は問題なさそうに見えます)。
Vitest の vi.mock や mockReturnValueOnce の挙動が想定と異なる可能性。
render 関数や userEvent によるインタラクションが、予期せぬ副作用(再レンダリング)を引き起こしている可能性。
今後の進め方について:
この問題を解決するために、さらに詳細なデバッグが必要そうです。
テスト内で console.log を追加して、loadFoodItems がいつ、なぜ呼ばれるのかを追跡する。
Vitest のドキュメントや関連Issueを調査し、モックの挙動について確認する。
React Testing Library のデバッグツール (screen.debug()) を活用して、各ステップでのDOMの状態を詳細に確認する。
どのように進めましょうか?

0->1の生成はGeminiが上手だなと思うけど、改善に関してはClaudeの方が上手かな?という体感があるので、一旦Claudeにバトンタッチ。まずはApp コンポーネント > localStorage にデータがある場合、初期表示時に食材カードが表示されることのテストを調整していきます。

npm run testを実行すると、App.test.tsxのテストのいくつかが失敗します。失敗する複数のテストのうち、提示した「localStorage にデータがある場合、初期表示時に食材カードが表示されること」のテストのエラーをまずは解消しましょう。

ここで1つずつやっていくのは、大量に修正されても私が追いきれないからです。これが………人間がボトルネック………。
まあ多分うまくいくでしょう、と鼻歌歌ってたら、なんとClaude、「2回呼び出されるなら2回呼び出されたってテストしたらいいよね😉」という結論に至ったらしく、

expect(mockedGetAllFoodItems).toHaveBeenCalledTimes(2);

とかいうバカのコードを出してきたのでストップをかけました。本当にあるんだなこういうの………。初めて遭遇したのでちょっと笑いました。

ざっとテストを確認するに、単純にテスト前にモックをリセットしてないのが原因では??と思ったので、

1回だけ呼ばれるのが正しい挙動ではないですか?テストコードを見ていますが、テストの前にモックを一度クリアする処理が入っていないように見えるのですが・・・原因として考えられる?

と尋ねてみます。私自身まだテスト書き始めて日が浅いので、これが正解かちょっと自信がなくて。
でも、どうやらそれが正解だったよう。該当のテストはリセットを導入することで成功しました。

今後も同じようなドツボにハマらないとは言い切れないので、testing-overview.md にモックの扱い方について追記しておきます。ドキュメントに残す、大事。

他にもコードを読んでると、クエリ優先度がおかしいコードがあったりだとか足されたりだとかしていたので、その辺りも逐次「これってどうなん?」と聞きながら修正していきます。

「余計なお世話」の対策

時々解決していない他のテストのエラーを解決しようすることが度々あって、コードを追いかけるのが大変になることがありました。

「おまんはひとつのことに集中せい!!!」ということで、Cursor Rulesに追記。

### 作業スコープに関する制限事項:
- **依頼された作業に直接関係のないコードやエラーには触れないこと。**
- **付随的な修正が必要だと判断した場合は、必ず人間に確認を取ってから対応すること。**
- `npm test`などを実行して他のエラーが検出されても、それが元の依頼に関連しない場合は無視すること。気になる点がある場合は報告のみ行うこと。

コンポーネントの責務の分離

そうやってApp.test.tsxのコードを書かせていたけど、改めて追加されたテストを読むと、明らかにFoodFormのコンポーネントの責務が混ざっています。この辺りは指示側が注視しておかないといけないかも。
気づいたときにテストコードも移動してもらいました。AIに頼むと割と秒でやってくれるので、こういうファイル内のコード移動は結構頼んでいます。私がやると閉じタグどーこだ?ってウロウロしちゃうのでありがたい。

なんかクエリ優先度がおかしい?

そんな調子で色々テストコードに手を加えていたんですが、どうにも食材リストのカード内にある削除ボタンを押したら該当のカードが削除されるというテストで、削除ボタンの指定の仕方があまりにも遠回り(該当のheadingを取得して親のdivから子孫を辿るみたいな冗長なコード)だったので、「これってaria-labelつけて特定したらいいんじゃないの?」って指摘して直してもらった。が、修正後のコードにいくつかgetByTextがあった。

見出しならgetByRoleでとれるはずなのになんで?って掘り下げていくと、shadcn/uiのCardTitledivでラップしていたせいだった。
ここはコード側が見出しタグになればいいので、shadcn/ui側を修正。

type CardTitleElement = 'div' | 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6';

interface CardTitleProps extends Omit<React.HTMLAttributes<HTMLElement>, 'as'> {
  as?: CardTitleElement;
  className?: string;
}
function CardTitle({ as: Component = 'div', className, ...props }: CardTitleProps) {
  return (
    <Component
      data-slot="card-title"
      className={cn("leading-none font-semibold", className)}
      {...props}
    />
  )
}

これで初めてポリモーフィックコンポーネントっていう概念を知ったけど、それについては割愛。
改めてテストコードを直してもらったら、ちゃんとscreen.getByRole('heading', { name: 'りんご', level: 2 })など、getByRoleで適切に取得されてた。

SpyInstance がコケて永遠にLintエラー

結論から言うとSpyInstanceMockInstance に変わってます。というもの。

https://vitest.dev/guide/migration.html#mock-types-4400

でも、LLM的にはまだSpyInstanceなんですよね。
こういう最新に追従できなくてLintエラー出して謎コード延々生成し始めるっていうパターン結構多いかも。
「書き方あってるはずなのにLintエラーが出る、っていうときは一回ドキュメント見てきてって人間に頼む」とかいうプロンプト入れておいたらいいのかなぁ………。

この辺の修正は私がやりました。

3. リファクタリング

App.test.tsxができたら、次はリファクタリングにうつります。
先に計画書を作ってもらい、優先度が高い順に作業していきます。

https://github.com/chiilog/reizouko-manager/blob/main/docs/reviews/foodform-review.md

異常系のテストがない

フォームに関しては異常系のテストの方が大事なのでは?と思ったので、フォームのテストは重点的にレビューとリファクタリングをしました。

レビューはGemini Code Assistも利用しています。PRを出したら勝手にレビューしてくれるやつです。

https://github.com/chiilog/reizouko-manager/pull/9

ただし、一度に全部レビューするのではなく小出しで出してくるので延々終わりません。
あと、「やらない」って明記してもまだ出してくるので別のレビューが隠れてしまってる可能性があります。

対応する・しないも含めてCursorと協議して、優先度高いものだけ対応することにしました。

やっていると、やっぱり冗長なコードがちらほら。そういうときは、実際に自分でアプリを動かしてみたりコントロールパネルで使えそうなタグを拾ってきて、「これ使えそうだけど」とか提案しています。

賞味期限の過去日付選択防止の例

例えば「3. 賞味期限の過去日付選択防止」のテストが何度も失敗していて、テスト方針ごと変えた方がよさそうだな………と思ったので、「先にテストの方針を検討してください」と指示を出しました。

方針1: 実装の検証からアプローチを変える(簡単)
現在のCUI(コンポーネントのユーザーインターフェース)の具体的な実装に依存するのではなく、AUI(抽象的なユーザーインターフェース)のレベルでテストします。つまり、「過去の日付が選択できない」という動作そのものをテストします。
具体的には:
1. カレンダーを開く
2. 過去の日付をクリックしてみる
3. ステートが変わっていない(date が変更されていない)ことを検証
方針2: UI/DOM構造を詳しく調査(複雑だが確実)
コンポーネントのレンダリング結果をより詳細に調査し、実際のDOM構造に合わせてテストを調整します。ただし、これはライブラリの実装詳細に依存するため、より脆弱です。

方針1でいきましょう、ということでさらに説明してもらいます。

この新しいテストアプローチには以下の利点があります:
1. 実装の詳細に依存しない:特定のDOM構造や属性に依存するのではなく、動作(ビヘイビア)をテストしています。これにより、将来のライブラリアップデートやリファクタリングで壊れにくくなります。
2. フォールバック戦略:日付セルが見つからない場合でも、代替の検証方法(無効なセルが存在することの確認)を用意しています。
3. 最終的な結果の検証:最も重要なのは、過去の日付を選択しようとしても初期の日付から変更されないことを確認していることです。これが私たちの実際に欲している機能です。

OKを出して実装してもらったものの、今日以前の日付の取得方法がどうにも頭に入ってきづらいというか、シンプルに一つ前のセルを参照するではだめなの?って思ったので聞いてみる。

現在のコードでは確かに昨日の日付をセルのテキストで検索していますが、カレンダーの表示において、特に月をまたぐ場合などに問題が生じる可能性があります。例えば、5月1日にテストを実行した場合、「4月30日」のセルを探したいのですが、単に「30」という数字だけを探していることになります。
その場合、次のような問題が生じます:
1. 月をまたいで表示している場合、前月の日と当月の同じ日にち(例:4月30日と5月30日)を区別できない
2. セルのテキストが日付だけでなく他の情報も含んでいる場合、誤検出の可能性がある
より堅牢なアプローチとしては、カレンダー上の日付セルが持っている「日付データ」自体を確認することが考えられます。カレンダーコンポーネントの実装によっては、日付を示すdata属性(例:data-date="2025-04-30")が付与されていることがあります。

セルがdata-dayを持ってるからそっちを使う方がよさそうだな、ということで、data-dayでやってみて、と指示を出しました。ちゃんとテストも通りましたが、可読性と堅牢性がわからなくて(自信がなくて)、「可読性と堅牢性の面ではどっちのコードが優れてる?」と確認。

堅牢性
新しい実装(現在のもの)の方が明らかに堅牢です:
1. 複数の検出アプローチ:data属性とテキスト内容の両方を使って日付セルを検出します。カレンダーライブラリが実装を変更しても、少なくとも一方の方法で動作する可能性が高いです。
2. 日付の明確な特定:data-dayやdata-date属性を使用すると、日付が「2025-04-30」のような形式で指定されるため、月をまたぐ場合でも正確に昨日の日付を特定できます。
3. 優先順位:より信頼性の高いdata属性による検出を優先し、それが失敗した場合にのみテキスト検索を使用するという戦略は、エラーに対してより強靭です。

可読性
新しい実装は少し複雑になりましたが、コメントと構造がより明確です:
1. 意図の明確さ:新しい実装はコメントで各アプローチの目的と制限を明確に説明しています。
2. ステップの分離:日付の検出方法を複数のステップに分け、各ステップに名前を付けています(yesterdayCellsByData, yesterdayCellsByText)。
3. 変数名の明確さ:yesterdayStrのような変数名は、その内容(ISO形式の日付文字列)が何であるかを明確に示しています。

結論
新しい実装の方が明らかに優れています。少し長くなっていますが、その追加の複雑さは十分に正当化されます:
堅牢性が大幅に向上(特に月をまたぐケースや様々なカレンダーライブラリに対して)
コードの意図と戦略が明確になり、将来の保守が容易になる
障害耐性が高まり、テストの信頼性が向上

テストコードは「一度書いたら忘れる」ものではなく、コードベースが進化するにつれて保守が必要になります。そのため、少し複雑でも堅牢で保守しやすい実装の方が長期的には価値があります。
現在の実装を保持し、次のステップに進むことをお勧めします。

と。こんな感じで自分のインプットもしながら進めます。

テストコードの見直し

色々とテストを足していると、どうにも時間がかかってるのと、テストコードが長すぎない?ということに気付きました。言われるがままにやってるとダメだねという気づき。

ここで、ChatGPTのデスクトップアプリでo3を選択し、ファイルのレビューをしてもらいました。

  • たしかにちょっと多いかも
  • 出力結果が同じならeachにまとめるといいかも

等々、細かくレビューしてもらい、指示をまとめなおしてCursorと改善。
また改善が終わったらo3に見てもらい、最終的にGemini Code Assistを通してマージ。

アプリ改善の最後の方は大体このサイクルで行ってました。ちなみにCursorでo3選択してやればアプリあちこち移動しなくていいんですけど、デスクトップアプリで使える+見るのが1ファイルだけなのでケチりました。もっと見る範囲が膨大になったらCursorでモデル選択して使うと思います。

ついに起こった「全消し」

雑にチャットを投げるようになったくらいで、変更不要・削除不要なコードを消されてしまいました………。

「あれっ?このテストないな!?」ってPR画面で気づいたからよかったものの、マージしてたら大変だった………。
これがあってから、少なくともチャットの初回は

  • 頼みたいこと
  • やってほしくないこと
  • その他必要な情報

をしっかり書くようにしています。
最初はちゃんと書いてるのに、慣れてくると省略してしまう悪癖です。

同じような指摘が続くなと思ったとき

同じようなレビューがぐるぐる回り始めたら、一度コードを書かせることから立ち止まって、アプローチを変えることを検討する。頭の中にフェオ=ウル!迷ったら立ち止まる!

コードを見たり、調べたりしながら「こういう方向性はどう思う?」とCursorと壁打ちします。

まとめ

Cursorとペアプロ・壁打ちしながら作業を進めましたが、自分の力量不足を実感しました。

  • TypeScriptの知識の深掘り
    • まだ基本的な知識しかないなと実感した
  • どういうテストが必要かの知識
    • 書籍読んだりイベント参加したりして引き出しを増やす
  • そもそものテストの関数の知識(特にspy関数)
    • もっと書く。読む。
  • 指示の曖昧さ。まだ時々ふわっとした指示を出してしまってぐるぐる回ってしまう

ただ、書かせることで理解が深まったものもあります。

  • mock関数の具体的な使い方
  • モックのリセット・クリアの使い分け

今回めちゃくちゃがっつりAIとペアプロしましたが、AIに指示するために自分の考えを明確にする必要があるなと気づきました。
「2回呼び出されるならそれでテストしよう」とかいうバカの解決策を提案されたときみたいに、ただ指示に従うだけでなく、

  • 本当にそれが正しいアプローチなのか考える
  • 「なぜそうするのか」を問う
  • 納得いかなかったら納得するまで深掘りする。(時にはモデルを変える)

で、これからもこのアプリをもっとよくしていこうと思います。

Discussion