テストコード、書いてますかー！！＼書いてるー！！／

かくいう私も（？）テストコードを書きながら人生を送っているわけですが、実は最近「偽陰性」を引き起こす悪いテストコードを書いてしまいました。最初はどこがダメか気づかなくて「？？？」となっていたのですが、間違いに気づいたとき「これ偽陰性じゃん！！」とピンときたのが嬉しくなったのでブログに残してみます。

（本記事で出てくる「テストコード」は全て単体テストのことを指しています。）

今回の状況

外部から取得した「お手伝い券」を処理していき、信頼度を詰むプログラムがあるとします。ただしプロセスの中で同じ「お手伝い券」だった場合、信頼度は加算されません。

以下がプロダクションコード・テストコードです。 先に言いますが、このプログラムには間違っている部分があります。 どこが間違っているか探しながら読んでみてくださいΣ((( つ•̀ω•́)つ

<?php

declare(strict_types=1);

use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\TestCase;

// チケットオブジェクト
class Ticket
{
    public function __construct(
        public string $name,
        public int $reliablePoint
    ) {
    }
}

// 実際の処理
class UseCase
{
    /**
     * @param Ticket[] $ticketList
     * @return int
     */
    public function exec(array $ticketList): int
    {
        $reliablePoint = 0;

        $alreadyUsed = [];
        foreach ($ticketList as $ticket) {
            if (in_array($ticket, $alreadyUsed, true)) {
                continue;
            }

            $reliablePoint += $ticket->reliablePoint;
            $alreadyUsed[] = $ticket;
        }

        return $reliablePoint;
    }
}

// UseCaseのテストコード
class UseCaseTest extends TestCase
{
    #[Test]
    #[DataProvider('dataProvider')]
    public function 信頼度が正しく計算されること(array $data, int $expected): void
    {
        $useCase = new UseCase();
        $actual = $useCase->exec($data);

        $this->assertSame($expected, $actual, '信頼度の計算が正しく行われていない');
    }

    public static function dataProvider(): array
    {
        $ticket1 = new Ticket('sara-arai', 100);
        $ticket2 = new Ticket('kata-tataki', 200);
        $ticket3 = new Ticket('okaimono', 300);

        return [
            '信頼度の計算が正しいこと' => [
                'data' => [$ticket1, $ticket2, $ticket3],
                'expected' => 600,
            ],
            '同じことをしても信頼度は上がらないこと' => [
                'data' => [$ticket1, $ticket1],
                'expected' => 100,
            ],
        ];
    }
}

なにが間違っているか

答え合わせ

<?php

if (in_array($ticket, $alreadyUsed, true)) {
    continue;
}

ユースケースの上記の部分が間違っています。「プロセスの中で同じ「お手伝い券」だった場合、信頼度は加算されない」の仕様を実現しようとしている部分です。同じお手伝い券とみなしたくても、オブジェクト同士の厳密比較となっているため意図通りにスキップされずに加算されてしまいます。

では、なぜテストが通ってしまっていたか？それは、テストも間違っているからです。

<?php

public static function dataProvider(): array
{
    $ticket1 = new Ticket('sara-arai', 100);
    $ticket2 = new Ticket('kata-tataki', 200);
    $ticket3 = new Ticket('okaimono', 300);

    return [
        '信頼度の計算が正しいこと' => [
            'data' => [$ticket1, $ticket2, $ticket3],
            'expected' => 600,
        ],
        '同じことをしても信頼度は上がらないこと' => [
            'data' => [$ticket1, $ticket1],
            'expected' => 100,
        ],
    ];
}

テストデータの見通しをよくするため、$ticket1という変数名にテストデータを格納していたのですが、あろうことか、本当にまったく同じオブジェクトを入れてしまってしまっていたのです・・・！マジで厳密比較でも通るテストコードを書いてしまっていました。

これは「偽陰性」である

失敗すべきでないときに失敗してしまうことを「偽陽性」（⁠false positive）と言います。失敗すべきときに失敗してくれないことを「偽陰性」（⁠false negative）と言います。

第2回偽陽性と偽陰性～自動テストの信頼性をむしばむ現象を理解する～ | gihyo.jp

今回は「失敗すべきときに失敗してくれない」、「プロダクトコードが誤っている/テストコードの結果が成功」に該当するので偽陰性に該当するテストを書いてしまいました。

「テストコードの結果が成功」しているのはテストコードが誤っているためです。そのためもう少し深掘りした言い方をすると、「プロダクトコードが誤っている/テストコードも間違っているけどテストコードの結果が成功している」となります。

偽陰性の他の例としては「過剰なスタブ化」もあります。 PHPerKaigi2024で発表している資料があるので、まだ読んでない方は読んでみてください＞ω＜ ▼

解決方法を探る

プロダクトコード: 厳密比較をやめる

<?php

if (in_array($ticket, $alreadyUsed)) {
    continue;
}

問題の箇所を緩やかな比較にすれば、プロダクトコードに関しては改善されます。

ただ、以下の点に関しては改善されません。

テストコードの間違いは正されなくても通ってしまう
in_arrayにtrueがないと、なんか、なんか、ﾋｬｧ...とさせてしまうのではないか、という不安（IMO）

プロダクトコード: 必要なデータだけ入れて厳密比較にする

<?php

$alreadyUsed = [];
foreach ($ticketList as $ticket) {
    if (in_array($ticket->name, $alreadyUsed, true)) {
        continue;
    }

    $reliablePoint += $ticket->reliablePoint;
    $alreadyUsed[] = $ticket->name;
}

チケットオブジェクトの名前だけで比較して、厳密比較を保つ方法です。私は実際こちらを採用しました。

ただ、以下の点で注意が必要かな、と思います。

純粋なstringになってしまうのでバリューオブジェクトの良さが活かされない、という見方もある
- 寿命が短ければいいのでは？とおもってこちらを採用した

テストコード: データを使い回すのをやめる

これなぁ...（遠い目）マジでそうなんだけど、マジでそうだと思います・・・。

なんだけど！！（言い訳入りま〜〜〜〜す）

以下に示すコードのように、「差分」をうまく表現するためにテストデータを変数に代入する、という技が好きでテストデータを変数に入れて使い回すことがあります。特にでかいデータ構造だと、テストコードの可読性が高まって好きです。

<?php

public static function dataProvider(): array
{
    $apiData = [
        'id' => 'asumikam',
        'name' => 'あすみ',
        'suki' => 'omuraisu',
    ];

    return [
        'データが不足がない時正常に処理されること' => [
            'data' => $apiData,
            'expected' => true,
        ],
        '予定していないデータがあってもエラーとならないこと' => [
            'data' => array_merge($apiData, ['hobby' => 'tennis']),
            'expected' => true,
        ],
        'idが入っていない時エラーとなること' => [
            'data' => array_merge($apiData, ['id' => null]),
            'expected' => false,
        ],
    ];
}

ただ、今回は使い方をシンプルにミスってしまいました・・・。というか今回はそんなに使い回すことにメリットがないので採用しなくても良い場面で手ぐせでやってしまったって感じでした。

強いていうならテストデータを変数で使い回すのは「不正解」なのかな、とおもっていますが、この辺は正解・不正解は実際わたしもわかりません。個人的には時と場合により変数に入れて見やすくするのが自分にとっての「正解」です。

チームの温度感で合わせるのが良いんじゃないかと思います。

おわりに

オブジェクトで表現した方が何かと便利で素敵なのですが、比較の部分は気をつけていかなきゃだな、と身に沁みた体験となりました☕️

「失敗」してもうまいこと整理するとブログのネタとして扱えてべんり！！！

#あすみかんの上にあすみかん

#たのしいことしかかかないことをここに決意します

「偽陰性」なテストコードから、良いコードの書き方を探る