きっかけ

途中から関わることになったプロジェクトで、既存のテストスイートを一通り動かしてみたところ、かなりの数が失敗しているのを見て驚いたことがありました。失敗の原因を調べていくと、コードの不具合ではなく、実装とテストがかみ合わなくなっていることが分かりました。この経験をきっかけに、テストが失敗する原因の切り分け方について整理しておこうと思います。

TL;DR

テストが失敗している原因には、大きく分けて2種類あります。コードに実際の欠陥がある「バグ」と、実装は変更されているのにテストコードがそれに追随できていない「テストドリフト」です。この記事では、テストドリフトとはどういう状態か、なぜ起きるのか、そして遭遇したときにどう対処するかを整理します。

テストが大量に失敗していた

そのプロジェクトでは、あるサービスクラスのテストを実行すると、数十件単位で失敗していました。

PHPUnit 10.5.x by Sebastian Bergmann and contributors.
..................................................  50 / 91 ( 54%)
F..F..F..F..F..F..F..F..F..F..F..F..F..F..F..F  91 / 91 (100%)

Tests: 91, Assertions: 130, Failures: 41

最初はバグを疑いました。しかしエラーメッセージを読むと、失敗の原因は一様に「モックの呼び出し回数が期待値と違う」というものでした。

PHPUnit\Framework\MockObject\ExpectationFailedException:
Expectation failed for method name is "prepare" when invoked 3 time(s).
Method was expected to be called 3 times, actually called 2 times.

テストドリフトとは何か

テストドリフトとは、実装が変更・進化したのに、テストコードがそれに追随できていない状態のことです。

テストが失敗しているものの、コードに実際のバグはなく、テストの方が古い実装の動作を前提にしたままになっている、という状態を指します。

バグとテストドリフトを混同すると、「テストが失敗しているのだからどこかがおかしいはずだ」という方向で調査を進めてしまい、時間を無駄にしてしまいます。

今回遭遇したケースでは、次のような経緯が推測できました。

  • あるDB操作が「3回」呼ばれることを期待するテストが書かれていた
  • その後、実装が改善され、同じ処理をより少ない呼び出し回数で行えるように最適化された
  • テストはそのことを知らず、失敗し続けていた

なぜこの問題が起きるのか:実装手順を固定するテスト

問題のあるテストは、おおむね次のような形をしていました。

// 問題のあるテストの例
public function test_sync_data(): void
{
    $mockPdo = $this->createMock(PDO::class);

    // PDO::prepareが「3回」呼ばれることを期待している
    $mockPdo->expects($this->exactly(3))
        ->method('prepare')
        ->willReturn($mockStatement);

    $syncer = new DataSyncService($mockPdo);
    $syncer->sync($testData);
}

このテストが検証しているのは、処理の「仕様」ではなく「実装手順」です。

PDO::prepare が何回呼ばれるかは実装の詳細であり、外部から見た振る舞いの仕様ではありません。不要な呼び出しを削減するといった正当な最適化によって、呼び出し回数は変わりうるものです。それなのにテストがその回数に縛られていると、正しい改善を行うたびにテストが壊れてしまいます。

たとえ話で考える:合計を求める処理のテスト

PDO::prepare の呼び出し回数の話だけだとイメージが湧きにくいかもしれないので、もう少し身近な例で考えてみます(わざわざ Adder というインターフェースを持ち出す時点でかなりわざとらしい例ですが、そこはご容赦ください)。

1から10までの総和を求める処理を考えます。素朴に実装すると、ループで1つずつ足し込んでいく形になります。

interface Adder
{
    public function add(int $a, int $b): int;
}

function sum(int $n, Adder $adder): int
{
    $total = 0;
    for ($i = 1; $i <= $n; $i++) {
        $total = $adder->add($total, $i);
    }
    return $total;
}

この実装に対して、次のようなテストを書いたとします。最終的な合計が正しいことに加えて、加算が何回行われたかまで検証しています。

public function test_sum_returns_55_and_calls_add_ten_times(): void
{
    $mockAdder = $this->createMock(Adder::class);
    $mockAdder->expects($this->exactly(10))
        ->method('add')
        ->willReturnCallback(fn($a, $b) => $a + $b);

    $result = sum(10, $mockAdder);

    $this->assertEquals(55, $result);
}

素朴な実装であれば、このテストは通ります。初期値$totalに1から10まで1つずつ足していくので、加算はちょうど10回発生するからです。

ところが、同じ「1から10までの合計を求める」という仕様を、ガウスの方法(初項と末項をペアにして計算する、n(n+1)/2という式で一気に求める方法)で実装し直したとします。

function sum(int $n, Adder $adder): int // 引数のAdderは受け取るが使わない
{
    return intdiv($n * ($n + 1), 2);
}

(引数に Adder を残しているのは、さきほどと同じテストコード(sum(10, $mockAdder))をそのまま使い回せるようにするためです。実際、この実装は $adder を一度も使っていません。返る値は変わらず55なので仕様は満たしていますが、「addが10回呼ばれること」を期待していたテストは失敗します。)

これは極端な例に見えるかもしれませんが、起きていることの構造は先ほどの PDO::prepare の例とまったく同じです。テストが「合計が正しいか」という仕様ではなく、「addという手順を何回踏んだか」という実装の都合を検証してしまっているため、より効率の良いアルゴリズムに置き換えるという正当な改善が、テストの失敗という形で罰せられてしまいます。

良いテストは、実装がループであろうとガウスの方法であろうと変わらず成立するはずです。つまり assertEquals(55, $result) の部分だけが本質であり、add の呼び出し回数を検証する部分は、そもそも書くべきではなかったということになります。

ただし、これは「呼び出し回数を検証するテストは常に不適切だ」という話ではありません。もし「addがちょうど指定回数だけ実行されること」自体が満たすべき仕様なのであれば、それは検証すべき正当な振る舞いです。たとえば、決済APIを二重に呼び出してはいけない、外部への通知は一度きりでなければならない、といった冪等性や副作用の回数そのものに意味がある処理であれば、呼び出し回数の検証はむしろ欠かせません。

今回の合計処理の例で問題だったのは、「addが何回呼ばれるか」が仕様として要求されていたわけではなく、たまたまその時点の実装がそうなっていただけだった、という点です。回数を検証すること自体が悪いのではなく、その回数が本当に仕様なのか、それとも実装の都合にすぎないのかを見極めずにテストへ固定してしまうことが問題だと言えます。

テストが本来検証すべきもの

テストが検証すべきなのは、実装の仕様、つまり振る舞いであり、実装の手順ではありません。

同じデータ同期処理を例にすると、次のように書き直せます。

// 振る舞いを検証するテストの例
public function test_sync_data_stores_expected_records(): void
{
    $repository = new InMemoryRecordRepository(); // テスト用リポジトリ
    $client = $this->createMock(SourceApiClient::class);
    $client->method('fetch')->willReturn([
        new SourceRecord(id: '1', name: 'テスト株式会社'),
    ]);

    $syncer = new DataSyncService($repository, $client);
    $syncer->sync();

    // 何回prepareを呼んだかではなく、結果が正しいかを検証する
    $stored = $repository->findById('1');
    $this->assertEquals('テスト株式会社', $stored->name);
}

// 異常系の振る舞いを検証するテストの例
public function test_sync_throws_when_source_returns_error(): void
{
    $this->expectException(SyncException::class);

    $repository = new InMemoryRecordRepository();
    $client = $this->createMock(SourceApiClient::class);
    $client->method('fetch')->willThrowException(new ApiException('接続エラー'));

    $syncer = new DataSyncService($repository, $client);
    $syncer->sync();
}

呼び出し回数を数える代わりに、処理の結果として何が保存されるか、異常時にどう振る舞うかを検証しています。これなら、内部の実装手順が変わってもテストは影響を受けません。

段階的に対処する

このようなテストドリフトに遭遇したとき、すぐに「テストの質もまとめて改善しよう」と考えたくなりますが、実際にはそれをやると次のような問題が起きがちです。

  • 作業範囲がどこまでも広がってしまう
  • 「直した」のか「直していない」のかの境界が曖昧になる
  • レビューしにくい巨大な変更になってしまう

そこで、対応を2段階に分けて進めました。

まず1段階目として、テストドリフトを解消し、テストが通る状態に戻すことだけを目的にします。期待値を現在の実装に合わせるだけで、テストの質そのものは問いません。

次に2段階目として、別タスクとしてテストの質を改善します。呼び出し回数に依存したテストを、振る舞いを検証する形に書き直していきます。

「まず通す、改善は別」という方針をチームに明示したことで、対応の責任範囲が明確になり、合意も取りやすくなりました。

チームへの伝え方

状況をチームに共有する際は、次のような順序を意識しました。

  • まず「バグではなくテストドリフトである」ことを最初に伝える
  • 現状(何件失敗していて、原因は何か)を簡潔に共有する
  • 対応方針(まず通す、改善は別タスク)を提案する

「バグではない」ということを先に伝えることで、緊急対応ではなく計画的な修正として扱ってもらいやすくなったと感じています。

まとめ

バグ テストドリフト
意味 コードに実際の欠陥がある テストが実装の変化に追随できていない
対処 バグを修正する テストを現在の実装に合わせる
緊急度 高い場合が多い 計画的に対処できることが多い
根本原因 コードのミス テストが実装の手順を固定してしまっている

テストが失敗したとき、最初に「これはバグか、テストドリフトか」を切り分けることで、その後の対処の方向性が変わります。途中から関わったプロジェクトであっても、既存のテストをすぐに信頼しきらず、失敗の意味を確認するところから始めることの大切さを実感した出来事でした。

参考