phpunitでスタブを作成した時のmixed型が気になった話


はじめに

ユニットテストを書いていて、スタブを作成する時にふと、期待される値の型がふわっと決められていたのに疑問を感じましたヽ( ´_つ`)ノ ?

例えば、以下のようなクラスがあったとします。

BillingManager.php
class BillingManager
{
    private $id;

    public function collate(Billing $billing)
    {
        $billing_manager_id = $billing->getBillingManagerId($this->id);
        if ($this->id == $billing_manager_id) {
            return true;
        }
        return false;
    }

    public function getId()
    {
        return $this->id;
    }

    public function setId($id)
    {
        $this->id = $id;
    }
}

この時のcollateメソッドに関してテストを書こうとした時に、BillingクラスのgetBillingManagerIdメソッドのモックを書く必要があるため

BillingManagerTest.php
private function getBillingStub($id, $value)
{
  $stub = $this->createMock(Billing::class);
  $stub->expects($this->any())
    ->method('getBillingManagerId')
    ->with($id)
    ->will($this->returnValue($value)); 
    return $stub;
}

のようなメソッドを作りました。
BillingクラスのgetBillingManagerIdメソッドに第一引数の$idを与えると、$valueが返ってことを期待する共通のスタブを取得できるようなメソッドです。
第二引数に入れられた値がそのままreturnValueのメソッドに入力される形になってます。

そしてテストを書いてみる

BillingManagerTest.php
public function test_collatePropertyIsStringButArgumentIsInteger()
{
    $this->billing->setId(1000);
    $this->assertFalse(
        $this->target->collate(
            $this->getBillingStub(1192296, "10e2")
        )
    );
}

public function test_collatePropertyAndArgumentIsInteger()
{
    $this->billing->setId(1000);
    $this->assertTrue(
        $this->target->collate(
            $this->getBillingStub(1192296, 1000)
        )
    );
}

この時に二つのテストが通ることを期待したとしても結果は

PHPUnit 6.5.6 by Sebastian Bergmann and contributors.

F.                                                                  2 / 2 (100%)

Time: 32 ms, Memory: 4.00MB

There was 1 failure:

1) tests\BillingManagerTest::test_collatePropertyIsStringButArgumentIsInteger
Failed asserting that true is false.

/Users/goto.kensuke/workspace/php-training/tests/apps/BillingManagerTest.php:26

FAILURES!
Tests: 2, Assertions: 2, Failures: 1.

でテストが通りません。

なんでこんなことが起きたの?

collateメソッドの

if ($this->id == $billing_manager_id) 
{
    return true;
}

この比較が厳密比較になっていなかったため、インスタンスにセットされた1000とスタブの返却値の"10e2"を比較したため、たまたまこの処理をパスしてしまい想定外の動きとなってしまい、テストをパスできませんでした。
phpの比較演算子で==を使った場合、比較する前に内部で暗黙的に型変換を行なってしまい、"10e2"が1000に変換されて、うまく処理を通ってしまっています。

今回の問題は2点あって、

  • if文の分岐で厳密比較をしていなかった
  • そもそもテストで入力する値の型を指定していなかった

phpunitのreturnValueメソッドを追っていくと

/**
* @param mixed $value
*
* @return ReturnStub
*/
public static function returnValue($value)
{
  return new ReturnStub($value);
}

で、入力する型はmixed型になっています。
今回の例でいうと、getBillingStubメソッドの$valueにどんな型の値を入れたとしても型で弾かれることはなくテストが行われます。
そのため、入力する値の型にもテストを書く人が気を配る必要があります。
そこまで考えるのもしんどいのでどこかでこの$valueの型をチェックしてもらいたい。

じゃあどうするか

assertInternalTypeで型チェックしよう。

変更箇所

private function getBillingStub($id, $value)
{
  $this->assertInternalType(IsType::TYPE_INT, $value); // 追加
  $stub = $this->createMock(Billing::class);
  $stub->expects($this->any())
    ->method('getBillingManagerId')
    ->with($id)
    ->will($this->returnValue($value)); 
    return $stub;
}

実行

PHPUnit 6.5.6 by Sebastian Bergmann and contributors.

F.                                                                  2 / 2 (100%)

Time: 28 ms, Memory: 4.00MB

There was 1 failure:

1) tests\BillingManagerTest::test_collatePropertyIsStringButArgumentIsInteger
Failed asserting that '10e2' is of type "int".

/Users/goto.kensuke/workspace/php-training/tests/apps/BillingManagerTest.php:44
/Users/goto.kensuke/workspace/php-training/tests/apps/BillingManagerTest.php:25

FAILURES!
Tests: 2, Assertions: 3, Failures: 1.

テスト内で入力された値の型が間違ってることをテストの実行時に確認できるようになりました。

まとめ

実装時に型を意識していないとphpは思わぬところで落とし穴にハマってしまいます。
実装をしている時はもちろんのこと、テストの際にも型を意識してコードを組んで行きましょう!

参考文献