関心事が異なるテストはメソッドを分けるか parametrize を使う

概要

不思議な UT シリーズ第2弾 (早くもシリーズ化決定)。
今回は、複数のケースを1つの UT にまとめてしまったことで、実施したいテスト項目がわかりにくくなった例を振り返ってみよう。世にも不思議な変数名のおまけつき (?)

ゆっくりしていってね!!!

まえおき

ここに登場するコードは以下で実装されたものを前提としている。

例やコードは、サンプル向けにアレンジしたもののため、違和感はご愛嬌。

やりたかったこと

検索条件に合致するオブジェクトを取得するメソッドの UT (ユニットテスト) を実装したかった。
以下のように、引数で受け取ったidに合致する本オブジェクトを取得するようなメソッドである。


class BookClass:
  """本クラス"""

  def get_by_id(self, id: int) -> Book | None:
      """idに合致する本を1件返す

      :param id: 検索したい本のid
      :return: idに合致する本。合致する本がない場合はNoneを返す
      """

      return Book.objects.filter(id=id).first()

このメソッドの UT として、以下のようなテストコードを実装した。


def test_get_by_id(self):
    """idに合致する本を1件返すこと"""

    # act
    book = BookClass.get_by_id(id=BookFactory().id)
    no_books = BookClass.get_by_id(id=999)

    # assert
    assert book
    assert not no_books

このコードの問題点

上記のコードには、以下のようなイマイチな点がある。


  • 「idに合致する本を1件返すこと」のテストの中で、条件に合致しなかった場合のテストも実施している
    • → テストメソッド名や docstring の内容からは、条件に合致しなかった場合のテストも実施するようには見えない
  • not no_books という二重否定のような assert になっている
    • no_books なので Book が見つからなかったことを表しているようだけど、not がついているからその逆で…えっと??
  • 1件の Book オブジェクトを返すメソッドのはずだけど、books と複数形になっている
    • → 実際は複数の Book オブジェクトを返すメソッドなのかな…?

などのような混乱を招くテストになっている。

なぜこうなった?

  • 複雑なテストではないので、起こり得るパターンをまとめても問題ないと思った
  • 自分もレビューする側だったら「not no_books ???」となったと思うが、実装しているとなぜか気がつけない
    • おそらく、「本が存在しないことを表したいので no_books でよかろう」のように視野が狭くなっていたのだと思う
    • no_books が変数名として適切なのか、not をつけたときの可読性はどうなるか、自分以外の人が読んだときに混乱しなそうか、などが見えていない

どうするとよさそうか?

「条件に合致する場合」「条件に合致しない場合」のように、関心事 (何をテストしたいのか) ごとにテストメソッドを分けると Good。


def test_get_by_id(self):
    """idに合致する本を1件返すこと"""

    # act
    book = BookClass.get_by_id(id=BookFactory().id)

    # assert
    assert book

def test_get_by_id_returns_none(self):
    """idに合致する本が存在しない場合はNoneが返ること"""

    # act
    book = BookClass.get_by_id(id=999)

    # assert
    assert book is None

pytest の parametrize を使って、複数の関心事についてテストしているということがわかるようにするのもよい。
pytest.param()id で属性名をつけておくと、各ケースに名前付けができ、どのテストを実行しているのか/どのテストで失敗したのかもわかりやすくなる 🌞


@pytest.mark.parametrize(
  "book_id,expected",
    [
      # idに合致する本が存在する場合
      pytest.param(
        1,
        1,
        id="exist",
      ),
      # idに合致する本が存在しない場合
      pytest.param(
        999,
        None,
        id="does-not-exist",
      ),
    ]
)
def test_get_by_id(self, book_id, expected):
    """get_by_id() が期待通りの値を返すこと"""

    # arrange
    BookFactory(id=1)

    # act
    result = BookClass.get_by_id(id=book_id)
    actual = result.id if result else result

    # assert
    assert actual == expected

$ pytest test_book.py
...
PASSED test_book.py::TestBook::test_get_by_id[1-1]
PASSED test_book.py::TestBook::test_get_by_id[999-None]

$ pytest test_book.py
...
PASSED test_book.py::TestBook::test_get_by_id[exist]
PASSED test_book.py::TestBook::test_get_by_id[does-not-exist]

おわり

シンプルな実装なのにテストの可読性が悪いという不思議な例を振り返ってみた。
テストの実装に慣れていないと起こりやすいかもしれない。

参考