自動テストの2歩目 - あしたからがんばる

これは2/3 タピオカLT4杯目、2/26 第145回PHP勉強会@東京で発表した内容です。

最初に

突然ですが、自動テストって難しくないですか？

というのも、だいたいのユニットテストのサイトでは

<?php

assertEquals(2, 1 + 1)

みたいなサンプルしかなく、「え、それだけでどうやってプロダクションのテストコード書くの？」って感じになるんですよね。

まぁ、そんななので僕も最初に書いたテストコードとかは今見返すと微妙だったりするんですよね。
また、コードレビューしていると同じような微妙なコードを見るわけです。

そうすると、「これはたぶん、みんな同じように最初は微妙なコードを書いてるのでは？」という気がします。

なので、ここで自分のハマった陥りがちなパターンを紹介します。
ちょっと好みが分かれそうな箇所もあります。

パターン1 同じロジック

例

さて、以下のような実装があるとします。

<?php

function sum(array $array)
{
    $sum = 0;
    foreach ($array as $value) {
        $sum += $value;
    }
    return $sum;
}

渡された配列の各要素の合計値を出すだけのシンプルな関数です。

それに対するテストが以下です。

<?php

public function testSum()
{
    $input = [1, 2, 3];

    $expected = 0;
    foreach ($input as $value) {
        $expected += $value;
    }

    $this->assertEquals($expected, sum($input));
}

問題点

さて、お気付きでしょうか・・・
以下のロジックが実装にもテストにもあるのがわかりますね。

<?php

    foreach ($array as $value) {
        $sum += $value;
    }

そう、実装とテストが同じロジックで書かれているのです。

これはもはやテストとして成立していないですね。
なぜなら、同じ計算をしたら同じ結果になるのは当然だからです。
小学生のときに「検算」というものを習いましたが、あれも別の計算方法で検算しているはずです。

このパターンに対しては、「決め打ちテスト」が有効です。

決め打ちテスト

以下のようにテスト結果を決め打ちにすると良いです。

<?php

public function testSum()
{
    $this->assertEquals(6, sum([1, 2, 3]));
    $this->assertEquals(2, sum([2]));
    $this->assertEquals(0, sum([0, 0, 0]));
    $this->assertEquals(0, sum([]));
}

ちなみに、データプロバイダを使用すると、データとテストを分離して書けます。

<?php

/**
 * @dataProvider dataProvider
 */
public function testSum(array $input, bool $expected)
{
    $this->assertEquals($expected, sum($input));
}

public function dataProvider(): array
{
    return [
        ['input' => [1, 2, 3], 'expected' => 6],
        ['input' => [2], 'expected' => 2],
        ['input' => [0, 0, 0], 'expected' => 0],
        ['input' => [], 'expected' => 0],
    ];
}

パターン2 データプロバイダの使いまわし

例

以下のようなテストコードがあります。

<?php

/**
 * @dataProvider dataProvider
 */
public function testGetQuestion(array $input, array $expected)
{
    $service = new QuestionService();
    
    $this->assertEquals($expected, $service->getQuestion($input));
}

public function dataProvider(): array
{
    return [
        [
            'input' => [
                'title' => 'test title',
            ],
            'expected' => [
                'title' => 'test title',
                'created_at' => '2000-01-01 00:00:00',
                'updated_at' => '2000-01-01 00:00:00',
                'deleted_at' => null,
            ],
        ],
    ];
}

質問を取得するだけのシンプルなメソッドのテストです。

さて、このサービスに、日付以外の情報のみを取得する getQuestionWithoutDate を追加しました。
テストコードは以下のようになりました。

<?php

// 上と同じコード
/**
 * @dataProvider dataProvider
 */
public function testGetQuestion(array $input, array $expected)
{
    $service = new QuestionService();
    
    $this->assertEquals($expected, $service->getQuestion($input));
}

public function dataProvider(): array
{
    return [
        [
            'input' => [
                'title' => 'test title',
            ],
            'expected' => [
                'title' => 'test title',
                'created_at' => '2000-01-01 00:00:00',
                'updated_at' => '2000-01-01 00:00:00',
                'deleted_at' => null,
            ],
        ],
    ];
}

// ここから新規追加のコード

/**
 * @dataProvider dataProvider
 */
public function testGetQuestionWithoutDate(array $input, array $expected)
{
    $expected['created_at'] = null;
    $expected['updated_at'] = null;
    $expected['deleted_at'] = null;
    
    $service = new QuestionService();
    
    $this->assertEquals($expected, $service->getQuestionWithoutDate($input));
}

問題点

以下の部分です。

<?php

    $expected['created_at'] = null;
    $expected['updated_at'] = null;
    $expected['deleted_at'] = null;

データプロバイダから受け取った値を消しています。このように、受け取った値を更新したり、一部を使用しなかったりするパターンがこれです。

これは、意図としてはよくわかります。
使用する値が似ているから、データプロバイダを流用したいんですね。

ですが、これはもはやデバッガがないと読めません。
というのも、コードを読むときは変数にどのような値が入っているかは、頭の中で記憶しているものです。
この頭の中の記憶にある値を更新したりすれば、混乱するわけです。

また、同じデータプロバイダを使用しているため、今後に実装やテストの変更があった際に影響範囲が広がってしまいます。

このパターンに対しては、「1テスト1データプロバイダ」が有効です。

1テスト1データプロバイダ

その名の通り、1つのデータプロバイダを使用できるテストは1つに絞るという話ですね。
以下のような感じですね。

<?php

/**
 * @dataProvider getQuestionDataProvider
 */
public function testGetQuestion(array $input, array $expected)
{
    $service = new QuestionService();
    
    $this->assertEquals($expected, $service->getQuestion($input));
}

// getQuestion用のデータプロバイダ
public function getQuestionDataProvider(): array
{
    return [
        [
            'input' => [
                'title' => 'test title',
            ],
            'expected' => [
                'title' => 'test title',
                'created_at' => '2000-01-01 00:00:00',
                'updated_at' => '2000-01-01 00:00:00',
                'deleted_at' => null,
            ],
        ],
    ];
}

/**
 * @dataProvider getQuestionWithoutDateDataProvider
 */
public function testGetQuestionWithoutDate(array $input, array $expected)
{
    $service = new QuestionService();
    
    $this->assertEquals($expected, $service->getQuestionWithoutDate($input));
}

// getQuestionWithoutDate用のデータプロバイダ
public function getQuestionWithoutDateDataProvider(): array
{
    return [
        [
            'input' => [
                'title' => 'test title',
            ],
            'expected' => [
                'title' => 'test title',
                'created_at' => '2000-01-01 00:00:00',
                'updated_at' => '2000-01-01 00:00:00',
                'deleted_at' => null,
            ],
        ],
    ];
}

受け取った値を変更しなていないので、シンプルに読めます。テストコードがそれぞれ3行になり、見通しもよくなりました。

ただ、機能テストなどを書いていると、データプロバイダの中身が多くなり、読みづらくなることもあると思います。
その際はテストコードで値を変更するのではなく、データプロバイダ内で値を変更する形式が良いと思っています。
僕は以下のような感じでデフォルトの情報を返すメソッドを作成して共通化したりしてます。

<?php

public function getQuestionDataProvider(): array
{
    return [
        'input' => [
            'title' => 'question title',
        ],
        'expected' => $this->defaultQuestionAttributes(),
    ];
}

public function getQuestionWithoutDateDataProvider(): array
{
    return [
        [
            'input' => [
                'title' => 'test title',
            ],
            'expected' => $this->defaultQuestionAttributes([
                'created_at' => null,
                'updated_at' => null,
                'deleted_at' => null,
            ]),
        ],
    ];
}

private function defaultQuestionAttributes(array $overrides = []): array
{
    $default = [
        'title' => 'question title',
        'created_at' => '2000-01-01 00:00:00',
        'updated_at' => '2000-01-01 00:00:00',
        'deleted_at' => null,
    ];
    return array_merge($default, $overrides);
}

ただし、共通化をすると脳内デバッガが死ぬのでやりすぎには注意です。

パターン3 ランダム値の検証

例

以下はLaravelのfactoryを使用したテストです。

<?php

public function testDelete()
{
    $question1 = factory(Question::class)->create();
    $question2 = factory(Question::class)->create();

    $service = new QuestionService();

    $service->delete($question1);

    $this->assertDatabaseMissing(['title' => $question1->title]);
    $this->assertDatabaseHas(['title' => $question2->title]);
}

質問を2つ用意して、1つを消して、それが消えているかを確認するテストですね。

問題点

これは一見問題なさそうに見えますが、 たまにテストが落ちます。
具体的には、Questionの各要素はランダムに値が決まっているため、$question1のtitleと$question2のtitleがたまたま同じ値になったときに落ちます。

テストにおいて関心がある値にランダム値を使うとこのパターンになります。なので、「ランダム値を比較・確認しない」が有効です。

ランダム値を比較・確認しない

単純です。固定値を使いましょう。

<?php

public function testDelete()
{
    $question1 = factory(Question::class)->create(['title' => '消える']);
    $question2 = factory(Question::class)->create(['title' => '残る']);

    $service = new QuestionService();

    $service->delete($question1);

    $this->assertDatabaseMissing(['title' => '消える']);
    $this->assertDatabaseHas(['title' => '残る']);
}

これでテスト結果がランダム値に依存しなくなったので、たまに落ちることもなくなります。

まとめ

自分の踏み抜いた罠パターンを紹介しました。

同じロジック
データプロバイダの使いまわし
ランダム値の検証

それぞれに対する対策として紹介しました。

決め打ちテストをしよう
1テスト1データプロバイダ
ランダム値で比較・確認をしない