RSpec で書くひとにやさしいテストコード

こんにちは。 STORES ネットショップの開発をしている、hsm_hx です。

この記事は STORES Product Blog Advent Calendar 2024 8日目の記事です。

わたしと RSpec

STORES ネットショップチームでは、ネットショップの注文データを保存するために作られたモデルを POS レジなどのデータを保存できるように拡張された新しい注文モデルにリプレースする大きなプロジェクトを進めています。

このプロジェクトについては phayacell さんのこちらの記事でも扱われているので、興味のある方はお読みください！

product.st.inc

そして、このプロジェクトを推進するにあたって、大きな役割を担っているのが RSpec です。

みなさんは普段自動テストをどのような目的で書いていますか？

一般的には、

新規開発した機能が想定通りのパラメータで動作していることを確認するため
あとから追加・改修した機能が既存の機能に影響を及ぼしていないかを確認するため

などの目的が多いかと思います。

今回わたしが携わっているモデルのリプレースプロジェクトでは、リプレースの前後で挙動が異なっていないことを確認するために自動テストを活用しています。

今回の記事では、そんな大規模プロジェクトを進めるにあたって考えた「ひとにやさしいテストコードとは？」という話題についていくつかの観点で触れていきたいと思います。

ひとにやさしいテストコードとは？

テストの構造を事前に整理する

チーム開発でアプリケーションコードを書く際にはまず設計を考え、チーム内で一定の合意を得てから詳細の実装に入ることが多いかと思います。

テストコードも同様に、特に context の多いテストコードであればはじめに設計のレビューをしてもらっておくとその後の手戻りが少なくて良いかもしれません。

レビューする側としても、 PR ひとつあたりのレビュー観点が減ってうれしいでしょう。

例として、テストコードの追加量が大きくなりそうな場合は以下のような状態で一度レビューに出すようにしています。

describe '何らかの処理' do
  context '外部 API リクエストが正常に行われる場合' do
    it '200 を返し、各種データが更新される' do
      pending '別 PR でテストを実装する'
      raise
    end
  end

  context '外部 API がメンテナンス中の場合' do
    it '503 を返し、各種データが更新されない' do
      pending '別 PR でテストを実装する'
      raise
    end
  end

  context '外部 API リクエストに失敗する場合' do
    it '500 を返し、各種データが更新されない' do
      pending '別 PR でテストを実装する'
      raise
    end
  end

  ...
end

このようにテストケースの設計を行うことで、結果的にテストケース全体の見通しも良くなることがあります。

どうしてもアプリケーションコードの実装と同時に完成されたテストコードをマージしたい！などという場合ではやや使いづらいかもしれませんが、

PR の話題を最小化し、手戻りを少なくするために活用できる方法のひとつかと思います。

外部 API を利用する際の Mock の粒度

外部 API を利用する際、アプリケーションコードでは一般的に Web API に対するリクエストをし、その結果をフォーマットしたものを返す というメソッドを用意すると思います。

そのような外部 API 利用を伴う機能のテストを書く際、我々はうっかり自前で用意したメソッドを mock してしまうことがあります。

# application code
class WebApiClient
  ...

  def detail
    response = Net::HTTP.get_response(uri)
    JSON.parse(response.body)
  end
end

# test code

before do
  allow(WebApiClient).receive(detail).and_return(
    ... # 何らかのデータ
  )
end

このようにテストを実装した場合、 WebApiClient#detail の内部を通過しないテストになっているため、堅牢であるとは言えません。

突然「ひとにやさしいテストコード」から「堅牢なテストコード」の話が出てきた、と考えられる方もいるかもしれませんが、ひとにやさしいテストコードとは、壊れにくい・信頼できるテストコードであるとも言うことができるでしょう。

「このテストは本当に堅牢であるか」「このコードは信頼可能なものか」を考えながら扱わなければいけないテストコードは、やさしくはないからです。

もちろん、 detail メソッドをしっかりと単体テストできている場合であればこの例のように detail メソッドを通過しないようなテストを書いても問題ないケースがありますが、
特に今回行ったモデルのリプレースプロジェクトでは、より安全にプロジェクトを進めるためには「ひとつの場所 (=request spec) を見れば確実にすべてのケースが網羅されていることがわかる」という状況が望ましいと言えます。

そのため、上記のような通過するメソッドを丸ごと mock する実装ではなく、ひとつの機能が間違いなく通しで動いていることを確認するため、WebMock を用いました。

before do
  WebMock.stub_request(:get, url)
    .with(body: hash_including({ params: })
    .to_return_json(
      status: 200,
      body: {
        ... # 何らかのデータ
      },
    )
end

このように WebMock を使って stub 化することで得られるメリットはテストが堅牢になるだけではありません。

メソッドを mock した場合、返却するデータの形式や項目が正しいことを確認するためには mock されたメソッドの処理を追う必要があります。
メソッドのひとつひとつに適切にドキュメントが書かれていれば話は別ですが、必ずしもそのようなプロジェクトばかりではありません。

API リクエストを mock した場合、返却するデータの形式や項目が正しいことは API ドキュメントを参照すれば確認できるでしょう。

このように、メソッドではなく API リクエストを mock することはテストの堅牢化だけでなくレビュワーの負担減にもつながると考えられます。

テストデータは適切に配置する

テストを書くとき、共通のテストデータをトップレベルに作成して使い回すことはないでしょうか？

例えばある model spec を書く際に、以下のように書くことがあるかもしれません。

describe TestModel do
  let!(:test_model) { Fabricate(:test_model) }

  describe '#method_A' do
    ...
  end

  describe '#method_B' do
    ...
  end
end

この例では、テストデータを作成するために Mongoid 向けのテストデータ作成機能を持った gem fabrication を用いています。

一見すると問題なさそうにも見えますが、このような方法にはいくつかの問題点があります。

例えば、このあと別の人が以下のように method_C のテストを追加します。

describe TestModel do
  let!(:test_model) { Fabricate(:test_model) }

  describe '#method_A' do
    ...
  end

  describe '#method_B' do
    ...
  end

  describe '#method_C' do
    let!(:instance) { Fabricate(:test_model) }

    it 'instance が削除されること' do
      expect { instance.method_C }.to change { TestModel.count }.to(0)
    end
  end
end

このテストコードは例え method_C が意図通り instance を削除するように実装されていても fail します。
何故なら、トップレベルで意図せず別の TestModel データが作成されているためです。

※実際にはこのテストは .to change { TestModel.count }.by(-1) と実装すれば意図通り動作しますが、プロダクトとテストコードが大きくなればより複雑で解消が難しい類似の問題に当たることがあるでしょう。

また、 method_A, method_B, method_C の動作の前提となるデータがすべて同様な状態であるとは限りません。むしろ、プロダクトが大きくなりたくさんのメソッドが生えた状態のモデルでは各メソッドの前提となるデータの状態が同様であることのほうが珍しいでしょう。

このような場合にトップレベルでテストデータを定義することは過度な共通化にあたり、アプリケーションコードと同様に避けるべき事例であると考えます。

describe TestModel do
  describe '#method_A' do
    let!(:instance) { Fabricate(:test_model_with_parameter_for_method_A) }
    ...
  end

  describe '#method_B' do
    let!(:instance) { Fabricate(:test_model_with_parameter_for_method_B) }
    ...
  end

  describe '#method_C' do
    let!(:instance) { Fabricate(:test_model_with_parameter_for_method_C) }
    ...
  end
end