DBにアクセスしないユニットテストのコーディングをしたい

Question

Laravel単体テストでDBにアクセスしないでユニットテストをコーディングしたいと思ってます。

ドリンクのECサイト的なWEBアプリを作ってます。
飲み物購入時に行われるDBアクセス（ストックを確認する、未成年のチェック、残金を減らしてストック減らすなど）を行うコントローラのユニットテストを書くことになりました。

書くことになったのですが、、、
後述のDrinkService.phpはDBへの永続化を責務とするDrinkRepository.php（DBモデルクラス）に直接依存しているため、、、

DrinkServiceクラス, DrinkRepositoryクラスをリファクタリングし
DBに依存しないユニットテストを書く必要がある（テストごとにDBの状態を適切にセットする）とのことなのですが、、、

このような設計センス（？）を今まで意識してコーディングする機会がなかったので、いきなり言われて戸惑ってます。

どのようなコーディングが「DBに直接依存している」ことを指すのか、
また直接依存してたらなぜよろしくないのか？リファクタリングしたらどのようないい事があるのかなど教えていただけたらと思います。（新たなクラス, 抽象クラス, インターフェースを実装はあり）

よろしくお願いします。

##追記
DBに直接依存しているとはdbにアクセスが行われることとご回答をいただきました。

そこで今度は、DBにアクセスさせないで上記クラスをリファクタリングしたり、別クラスを追加したりして実現できるものなのか？とまた疑問に感じました？（予め擬似的なDBで使われる値をセットした別クラスを用意しておくとか？）

##ソース（計３ファイル）

<?php

declare(strict_types=1);

namespace App\Services\Drink;

use App\Exceptions\Drink\Buy\GuiltyException;
use App\Exceptions\Drink\Buy\NoStockException;
use App\Exceptions\Drink\Buy\NotEnoughMoneyException;
use App\Exceptions\Drink\NotFoundException;
use App\Models\Drink;
use App\Models\User;
use App\Repositories\DrinkRepository;
use App\Repositories\UserRepository;

/**
 * Class DrinkService
 * @package App\Services\Drink
 */
final class DrinkService
{
    private DrinkRepository $drinkRepository;
    private UserRepository $userRepository;

    public function __construct()
    {
        $this->drinkRepository = new DrinkRepository();
        $this->userRepository = new UserRepository();
    }

    /**
     * ドリンク一覧を取得する
     * @return iterable
     */
    public function list(): iterable
    {
        return $this->drinkRepository->all();
    }

    /**
     * ドリンクを購入する
     * @param User $user 購入するユーザー
     * @param int $drinkId ドリンクID
     * @return DrinkBoughtResult 購入後のドリンクとユーザーの状態
     * @throws GuiltyException
     * @throws NoStockException
     * @throws NotEnoughMoneyException
     * @throws NotFoundException|\App\Exceptions\User\NotFoundException
     */
    public function buy(User $user, int $drinkId): DrinkBoughtResult
    {
        $drink = $this->drinkRepository->find($drinkId);

        if (is_null($drink)) {
            throw new NotFoundException();
        }

        if ($drink->stock <= 0) {
            throw new NoStockException();
        }

        if ($user->wallet < $drink->price) {
            throw new NotEnoughMoneyException();
        }

        if ($drink->isAlcohol && $user->age < 20) {
            throw new GuiltyException();
        }

        $newDrink = new Drink(
            $drink->id,
            $drink->name,
            $drink->price,
            $drink->stock - 1,
            $drink->isAlcohol,
        );

        $newUser = new User(
            $user->id,
            $user->name,
            $user->age,
            $user->wallet - $drink->price,
        );

        $this->drinkRepository->save($newDrink);
        $this->userRepository->save($newUser);

        return new DrinkBoughtResult($newUser, $newDrink);
    }
}

<?php

declare(strict_types=1);

namespace App\Repositories;

use App\Exceptions\Drink\NotFoundException;
use App\Models\Drink;
use Illuminate\Database\ConnectionInterface;

/**
 * Class DrinkRepository
 * @package App\Repositories
 */
final class DrinkRepository
{
    /**
     * テーブル名
     */
    private const TABLE = 'drinks';

    /**
     * @var ConnectionInterface
     */
    private ConnectionInterface $connection;

    public function __construct()
    {
        $this->connection = app()->get(ConnectionInterface::class);
    }

    /**
     * ドリンクを全件取得する
     * @return iterable
     */
    public function all(): iterable
    {
        return $this->connection->table(self::TABLE)
            ->get()
            ->map(fn(\stdClass $d) => $this->createDrinkModel($d));
    }

    /**
     * ドリンクをIDにより取得する
     * @param int $id
     * @return Drink|null
     */
    public function find(int $id): ?Drink
    {
        $d = $this->connection->table(self::TABLE)
            ->find($id);

        return is_null($d) ? null : $this->createDrinkModel($d);
    }

    /**
     * ドリンクを保存する
     * @param Drink $drink
     * @throws NotFoundException
     */
    public function save(Drink $drink): void
    {
        $query = $this->connection->table(self::TABLE)
            ->where('id', $drink->id);
        if (!$query->exists()) {
            throw new NotFoundException('ドリンクの新規作成はできません。');
        }

        $query->update(
            [
                'name' => $drink->name,
                'price' => $drink->price,
                'stock' => $drink->stock,
                'is_alcohol' => $drink->isAlcohol,
            ]
        );
    }

    private function createDrinkModel(\stdClass $d): Drink
    {
        return new Drink(
            $d->id,
            $d->name,
            $d->price,
            $d->stock,
            (boolean)$d->is_alcohol,
        );
    }
}

<?php

declare(strict_types=1);

namespace Tests\Unit\Services\Drink;

use App\Exceptions\Drink\Buy\NoStockException;
use App\Models\User;
use App\Services\Drink\DrinkService;
use Tests\TestCase;

class DrinkServiceBuyTest extends TestCase
{
    // TODO: DrinkService::buy()の分岐をすべてテストすること

    /**
     * FIXME: 在庫数がDBに依存しており, DBの状態によってこのテストは失敗するので困っています。
     * @testdox 在庫が0以下の場合, NoStockExceptionをスローすること
     * @throws
     */
    public function test_buy_should_throws_no_stock_exception_if_stock_0_or_less(): void
    {
        $this->expectException(NoStockException::class);

        $service = new DrinkService();
        $user = new User(1, 'テストユーザー', 20, 100);
        $service->buy($user, 1);
    }
}

##DB定義

create table drinks
(
  id bigint unsigned auto_increment comment 'ID'
    primary key,
  name varchar(255) not null comment '商品名',
  price int unsigned not null comment '価格',
  stock int unsigned not null comment '在庫数',
  is_alcohol tinyint(1) not null comment 'アルコール飲料であるか'
)
  collate=utf8mb4_unicode_ci;

create table users
(
  id bigint unsigned auto_increment comment 'ID'
    primary key,
  name varchar(255) not null comment '名前',
  age tinyint unsigned not null comment '年齢',
  wallet int unsigned not null comment '所持金'
)

Answer 1

Laravel でのクリーンアーキテクチャとテストをどうすべきかについての持論です

• 5年間 Laravel を使って辿り着いた，全然頑張らない「なんちゃってクリーンアーキテクチャ」という落としどころ

既存の実装のダメ出しをすると，極端な話にはなりますが

/**
 * ドリンクをIDにより取得する
 * @param int $id
 * @return Drink|null
 */
public function find(int $id): ?Drink
{
    $d = $this->connection->table(self::TABLE)
        ->find($id);

    return is_null($d) ? null : $this->createDrinkModel($d);
}

このメソッドで find() 結果を返してる時点でリポジトリ失格です。なぜなら返り値を使って ->fill(...)->save(); とか勝手に呼べちゃうからです。 それをやってはいけないと縛るのは，プロジェクトをメンテナンスする人の責任であり，チームメンバーへの思想の共有がスムーズにできない or 怠るのであればやってはなりません。

Laravel が ActiveRecord ベースのフレームワークなので Repository パターンは基本的にミスマッチだと思っていますが，それでもどうしてもやりたいなら

• Eloquent Model は返さず，データベースに一切依存しない，値を詰め込むだけのオブジェクトを1個1個丁寧に定義する
• Eloquent Model からデータベースコネクションを削除する（ Model クラスの定義そのままでは実現できないので改造必須）

のいずれかぐらいはやっておいたほうがいいかなと思います。

Answer 2

DBにアクセスしないではなくDBに依存しないでした。
テストデータを挿入して実行する実用があるみたいです。

Answer 3

ユニットテストはリポジトリのチェックアウト直後に何も設定しなくてもテストを実行して通る状態が良いです。
そのためDBに依存してはいけません。
「DBに依存している」とは、テスト実行時にDBアクセスが行われる状態のことです。
また、DBに依存していると、自分以外の誰かが自分の参照しているDBと違うDBのデータでテストが通るようにしてコミットすると、自分の環境ではコードは正しく、何も間違ったことをしていなくてもテストが通らなくなります。

Comments

[shoco]

ありがとうございます。

DBにアクセスしないとのことですが、DrinkRepositoryのアクセスをしないで、どうやってSQLが正しく機能しているかのテストをするのでしょうか？

上記クラスをリファクタリングしたり、別クラスを追加したりしてどうにかなるものなのでしょうか？

[rysh]

PHPは勝手がわからないんですが、SQLをテストしないとバグるんですか？

どうしてもテストしたい時はSQLを出力して文字列としてアサーションするか、H2のようなオンメモリDBを使うと良いと思いますよ。