PHPUnitのアンチパターンとベストプラクティス

このエントリーをはてなブックマークに追加

みなさんこんにちは。@ryuzeeです。

SlideShareを徘徊していたらPHPUnitのアンチパターン・ベストプラクティスに関する素晴らしいスライドを見つけたので内容を抜粋で紹介します。

1. テストの中で何もテストしていない

class FooTest extends PHPUnit_Framework_TestCase
{
    public function testSomething() {
        $foo = new Foo;
        $foo->doSomething(new Bar);
    }
}

こういうテスト。どこにもアサーションがなくて何もチェックしていません。 $foo->doSomethingの戻り値を検証しないならなんの意味もありません。 純粋にTDDをしていれば、テストコード作成→テスト実行でRed→プロダクションコード作成→テスト実行でGreenなのでこういうテストは登場しません。 後から、カバレージだけを求めてテストを追加したとか、テストコードのレビューをしていないとか、プロセスに問題がある可能性も否定できません。 これを発見する方法は

phpunit --strict --verbose FooTest.php

のようにstrictオプションを付ければOKです。

2. １つのテストメソッドの中で色々テストし過ぎている

class StackTest extends PHPUnit_Framework_TestCase
{
    public function testPushAndPopWorks() {
        $stack = array();
        array_push($stack, 'foo');
        $this->assertEquals('foo', $stack[count($stack)-1]);
        $this->assertNotEmpty($stack);
        $this->assertEquals('foo', array_pop($stack));
        $this->assertEmpty($stack);
    }
}

この例だとテスト対象となるデータに変化が加わりながら複数の検証が行われています。 このくらいの行数だと分からないこともないですが、もっと長くなると訳がわからなくなるので、@dependsアノテーションを使ってテストの実行順序を制御しつつ分割を行うべきです。

3. 引数に真にしたい条件を与えてなんでもassertTrueでチェックしている

$this->assertTrue(empty($arr));
$this->assertTrue($val >= 3);

引数は検証対象の値やオブジェクトであるべきで、テストの意図を伝わりやすくすべきです。 そういう意味で何でもかんでも真であるかを検証するのは良いとは言えません。 ※ただし個人的にはDataProviderを利用してテストがすっきりする場合はその限りではないと思います。

$this->assertEmpty($arr);
$this->assertGreaterThanOrEqual($val, 3);

4. テストデータのパターンが複数あるのを理由に１つのテストの中で順番に検証する

たとえばうるう年かどうかを返すメソッドをテストする場合を考えてみましょう。 TDDなら、とりあえずTrueを返す→4で割ってあまりが0ならTrueを返す→さらに100で割ってあまり0ならFalseを返すという順番にコーディングされますが、単純にテストをベタベタ書くと以下のようになります。

$this->assertTrue($this->Util->isLeapYear(2012));
$this->assertFalse($this->Util->isLeapYear(2011));
$this->assertFalse($this->Util->isLeapYear(2000));

うるう年ならテストケース数はたかが知れていますが、実際のアプリケーションでデータのパターンが多数あるようなものでコピーペーストによるテストケースの増殖は厳しいです。 またテストメソッド内で配列を定義してテストすると、どこかで検証が失敗した場合にそれがどこなのか分かりにくいという問題もあります。 したがって、ここでは@dataProviderアノテーションを使うようにします。

static function forTestIsLeapYear() {
    return array(
        array(2012, true),
        array(2011, false),
        array(2000, false),
    );
}
/**
 * @dataProvider forTestIsLeapYear
 **/
public function testIsLeapYear($year, $expected) {
    $this->assertTrue($this->Util->isLeapYear($year), $expected);
}

5. テストが分類されていない。適切なフォルダ構成になっていない

解決策は適切にフォルダに分けて配置したり、テストスイートをつくったり、@groupアノテーションを使ってテストを分類します。

/**
 * @group functional
 */

こうしておけば、

phpunit --group=functional TestSuite.php

とすればファンクショナルテストのグループのみ実行してくれます。

まとめ

ここに紹介したのはほんの一部なので詳細はスライドを見たり、PHPUnitのマニュアルを熟読すると良いでしょう。 開発が始まった段階でチームでのテストに対する考え方、テストの書き方について議論し統一しておかないといけないですし、テストコードも個人所有ではなく共同所有なので、開発初期からレビューが繰り返され、悪い作りのテストが混入していくのを防がないといけません。 たくさん作ってしまったダメなテストは時にテストが無いのと同じくらいタチが悪いことになります。

それでは。

/作成者: