🎉

テスト駆動せずにテストコードを書いたら上手くいかなかった話

2024/12/07に公開

背景

今更ですが初めてユニットテストのテストコードをちゃんと書いてみました(それ、t-wadaさんの前で言えんの?)
これまでの癖で先にアプリケーションコードを修正しちゃってたのですが、修正完了した後に「あ?そいえばユニットテストのテストコード書くか?」となり書くことに。

そしたらw
書いたコードがw
全然ひどくてw
なるほどテスト駆動の必要性ってそういうことか、と今更わかったのでその過程を記事にしておきます。

テストを書く前のアプリケーションコード

今回やりたかったことは、「Hogeデータを更新する際に特定の条件に一致していたらFugeデータを作成する」という処理の追加です。

以下はテストを書く前のコードです。
一応想定していた挙動にはなっている状態。

// コントローラー
class HogeController
{
    public function postEdit($request)
    {
        $hoge_service = HogeService::getInstance();
        return $hoge_service->update($request->input());
    }
}
// Hogeデータ用のサービスクラス
class HogeService
{
    public static function getInstance()
    {
        return new HogeService();
    }

    public function update(array $input)
    {
        // Hogeの更新処理いろいろ
        $hoge = Hoge::getById($input['hoge_id']);
        $hoge->update($input['hoge_data']);

        // ↓今回追加した箇所
        FugeService::createByInput($input['fugefuge'], $hoge);
    }
}
// Fugeデータ用のサービスクラス
class FugeService
{
    public static function createByInput(array $input, Hoge $hoge)
    {
        // Hogeを使って何かしら条件確認
        if ($hoge->isNg()) {
            return;
        }
        FugeRepository::create($input);
    }
}
// Fugeデータ用のリポジトリクラス
class FugeRepository
{
    public static function create(array $input)
    {
        FugeModel::create($input);
    }
}

テストを書く

アプリケーションコードを元にユニットテストを書こうとしたらつまづいたことを書いていきます。
書きたかったテストは、「FugeServiceクラスのcreateByInput()メソッドが想定通りの挙動になっているかどうかを確認できるテスト」です。
条件に一致した場合のみFugeRepositoryのcreate()が呼ばれることを確認できれば良いのかな🤔と考えながら着手。

困ったこと1. リポジトリクラスをモック化できない

FugeRepositoryのモックを作成してモックのcreate()が呼ばれることを確認したかったのですが、FugeRepository::create($input);という形で使用されているためモック化してもcreate()メソッドの呼び出しを確認することができませんでした。

なぜモック化できないのか

今回モックの生成にはMockeryというライブラリを使っています。
Mockeryではクラス名の衝突を避けるために、モックをMockery_0_FugeRepositoryのようなクラス名で生成するようです。
静的メソッドでは{クラス名}::{メソッド名}()という呼び出しになるので、モックではなく元々のクラスで呼び出されてしまうみたいです。(メソッドの呼び出しがクラス名に依存している)

静的メソッドは絶対モック化できないのか

Mockery::mock('alias:' . FugeRepository::class);という書き方でモックを作成すれば、静的メソッドにも対応できるようです。
上記の方法だと、モックのクラス名も「FugeRepository」で生成されます。
ただ、フレームワークを使ってテスト実装をしているとうまくいかないことが多いみたいです...。
(詳細はこちら)

困ったこと2. ユニットテストでModelのインスタンスが生成できない

LaravelのModelを継承しているクラスのインスタンスを生成しようとすると、resolveConnection()でエラーになってしまいました。
どうやらModelのインスタンスを生成するときにDBに接続するようになっているようです。
(正確にはgetGuarded()でDB接続しているみたいだった)

修正後のアプリケーションコード

ユニットをテストを書けるようにするためにアプリケーションコードを以下のように修正しました。
(HogeServiceは元々実装されていたコードなので他に影響が出ないような修正をした結果「最適じゃない感」が残ってます...)

// コントローラー
class HogeController
{
    public function postEdit($request)
    {
        $hoge_service = HogeService::getInstance();
        return $hoge_service->update($request->input());
    }
}
// Hogeデータ用のサービスクラス
class HogeService
{
    protected FugeService $fuge_service;

    public function __construct()
    {
         // FugeServiceのインスタンスを注入
         $this->fuge_service = app(FugeService::class);
    }

    public static function getInstance()
    {
        return new HogeService();
    }

    public function update(array $input)
    {
        // Hogeの更新処理いろいろ
        $hoge = Hoge::getById($input['hoge_id']);
        $hoge->update($input['hoge_data']);

        // FugeServiceのインスタンスでメソッド呼び出し
        $this->fuge_service->createByInput($input['fugefuge'], $hoge->status);
    }
}
// Fugeデータ用のサービスクラス
class FugeService
{
    protected FugeRepository $fuge_repository

    public function __construct(FugeRepository $fuge_repository)
    {
        // FugeRepositoryのインスタンスを注入
        $this->fuge_repository = $fuge_repository;
    }

    // staticを削除
    public function createByInput(array $input, int $hoge_status)
    {
        // Hogeを使っていたところをHogeのstatusを使用することに
        if (hoge_status == 0) {
            return;
        }

        // FugeRepositoryのインスタンスでメソッド呼び出し
        $this->fuge_repository->create($input);
    }
}
// Fugeデータ用のリポジトリクラス
class FugeRepository
{
    // staticを削除
    public function create(array $input)
    {
        FugeModel::create($input);
    }
}

修正した箇所

  1. リポジトリクラスのcreateメソッドを非静的メソッドに変更
  2. リポジトリクラスのcreateメソッドの引数をモデルクラスから数値に変更

実装したテストコード

上記修正によって実装できたテストコードが以下です。
実際はcreate()が呼ばれるパターンとそうでないパターンのテストケースを作成しましたが省略してます。

class FugeServiceTest extends TestCase
{
    public function test()
    {
        $hoge_status = 1;

        // モック生成、依存性注入
        $fuge_repository_mock = Mockery::mock(FugeRepository::class);
        $this->app()->instance(FugeRepository::class, $fuge_repository_mock);

        // リポジトリのcreate()が呼ばれること
        $fuge_repository_mock->('create')
            ->once()
            ->andReturnTrue();

        // サービスクラスのメソッド実行
        $fuge_service = $this->app(FugeService::class);
        $fuge_service->createByInput(['input1' => xxx, 'input2' => yyy], $hoge_status);
    }
}

最後に

上記のように、テストコード実装前にアプリケーションコードを書いたことでユニットテストのコードが書けない状態になっていました。
DI(依存性の注入)とかテスト駆動とか、大事なんだな〜ってことだけわかっていましたが、今回初めてテスト実装することで改めてそれらの重要性を感じることができました。(修正後のコードも最適とは言えないかもだけど)

やはり実践...!
実践がチカラになる...!

ちなみに元々なんでstaticメソッドを使っていたかというと、staticにしたい理由があったわけではなく一旦既存コードを真似て作ってみただけでした。
俺、馬鹿だからわかんねぇけどよ、$this使ってないからstaticメソッドにしていいってことか?
ということで書いてみましたがこれダメです。
みなさんもお気をつけて。

感想

たまにはコーディングもしないとね〜。

参考

Discussion