【Ruby on Rails/Rspec】集計系メソッドのテストの書き方ビフォーアフター

MIXI DEVELOPERS Advent Calendar 2024の9日目の記事です！

私は現在MIXIのFanstaというサービスの開発に携わっています。

FanstaではバックエンドにRuby on Railsを採用しており、テストはRspecで記述しています。

予約機能もあり、大量の予約の中から特定の条件のものを絞り込んで返したり、集計したりする処理をよく実装するのですが、specの書き方に悩んできました。
最近ようやく個人的にしっくりくる書き方にたどり着いたので、そのビフォーアフターを紹介します。

例：テスト対象のメソッド

例えば、このようなメソッドがあるとします。

sample_service.rb

def aggregate_fee(year:)
  Reservation.where(year:, state: 'visited')
             .joins(:user)
             .merge(User.where(state: 'active'))
             .sum(:fee)
end

reservationsテーブルのレコードから指定したyear、かつstateがvisited、かつ紐づくユーザーがactiveであるもの、のみ絞り込んで、そのfeeカラムの合計値を取得しています。
specの書き方の違いを伝えるのにシンプルな例として挙げていますが、実際はもっと条件が多くて複雑である場合を想像していただければと思います。

[before] モヤモヤしていたspecの書き方

基本的には処理が複雑であれば細かくメソッドやスコープを定義してそれぞれで単体テストを書くべきと思いますが、今回はそこまでするのはtoo muchな場合の想定です。

以前は上記のメソッドに対するテストは次のように書くことが多かったです。

※テスト用のレコードの生成にはfactory_botというgemを使っています。

sample_service_old_spec.rb

describe '.aggregate_fee' do
  subject { SampleClass.aggregate_fee(year: 2024) }

  before do
    # 含まれる
    create(:reservation, year: 2024, state: 'visited', fee: 100, user: create(:user, state: 'active'))

    # 以下は含まれない
    ## yearが異なる
    create(:reservation, year: 2023, state: 'visited', fee: 200, user: create(:user, state: 'active'))

    ## stateが異なる
    create(:reservation, year: 2024, state: 'canceled', fee: 200, user: create(:user, state: 'active'))

    ## userのstateが異なる
    create(:reservation, year: 2024, state: 'visited', fee: 200, user: create(:user, state: 'inactive'))
  end

  it '条件に合致するレコードのapplication_feeの合計が返ること' do
    expect(subject).to eq(100)
  end
end

いろんなパターンのレコードを一気に用意して、1つのテストケースでまとめて確認しています。
最初に実装する際には1つのテストケースでまとめて各条件を検証できて快適に感じるのですが、後から見ると結構読み解くのがつらいです。
もしコメントがなかった場合、それぞれどこの観点を変えてテスト用レコードを用意しているのか、パッと理解するのは難しそうです。
なのでコメントが必須になってくるのですが、コメントを足していくと1つのケース内の記述がどんどん増えていき、テスト用レコードの準備部分とexpectの記述のコード上の距離が離れていくので、期待値との一致確認もしづらくなってしまいます。

メソッドに修正が入った場合

最初の実装からしばらく経った後、別の条件を追加する必要が出てきたとします。

sample_service.rb

def aggregate_fee(year:)
  base_query = Reservation.where(year:)
                          .joins(:user)
                          .merge(User.where(state: 'active'))
  visited_sum = base_query.where(state: 'visited')
                          .sum(:fee)
  canceled_sum = base_query.where(state: 'canceled')
                          .sum(:cancel_fee)
  visited_sum + canceled_sum
end

集計の対象のstateがvisitedだけだったものが、visitedとcanceledの2つになり、
それぞれのステータスで集計するカラムがfeeとcancel_feeで異なるようになりました。
このとき、修正に応じてspecも修正する必要がありますが、canceledステータスの条件をどう入れようか迷ってしまいそうです。
spec全体を再度把握して、組み立て直す必要が出てきます。

[after] たどり着いたspecの書き方

最近は、もっと細かくテストケースを分ける形にしています。
ポイントは、一番最初に通しの確認と細かい条件の確認で分けていることです。
通しの確認は細かい条件の確認の結合テストのようなイメージです。
最初に通しで確認するケースを書いた後に、各条件ごとに1レコードで単体テストを書いていきます。

sample_service_new_spec.rb

describe '.aggregate_fee' do
  subject { SampleClass.aggregate_fee(year: 2024) }

  describe '通しの確認' do
    before do
      3.times do
        create(:reservation, year: 2024, state: 'visited', fee: 100, user: create(:user, state: 'active'))
      end
    end

    it 'feeの合計値が返ること' do
      expect(subject).to eq(300)
    end
  end

  describe '細かい条件の確認' do
    describe 'yearによる絞り込み' do
      before do
        create(:reservation, year:, state: 'visited', fee: 100, user: create(:user, state: 'active'))
      end
      context 'yearが一致する場合' do
        let!(:year) { 2024 }
        it '集計の対象に含まれること' do
          expect(subject).to eq(100)
        end
      end
      context 'yearが一致しない場合' do
        let!(:year) { 2024 }
        it '集計の対象に含まれないこと' do
          expect(subject).to eq(0)
        end
      end
    end
    describe 'stateによる絞り込み' do
      before do
        create(:reservation, year: 2024, state:, fee: 100, user: create(:user, state: 'active'))
      end
      context 'stateが一致する場合' do
        let!(:state) { 'visited' }
        it '集計の対象に含まれること' do
          expect(subject).to eq(100)
        end
      end
      context 'stateが一致しない場合' do
        let!(:state) { 'canceled' }
        it '集計の対象に含まれないこと' do
          expect(subject).to eq(0)
        end
      end
    end
    describe 'userのstateによる絞り込み' do
      before do
        create(:reservation, year: 2024, state: 'visited', fee: 100, user: create(:user, state:))
      end
      context 'stateが一致する場合' do
        let!(:state) { 'active' }
        it '集計の対象に含まれること' do
          expect(subject).to eq(100)
        end
      end
      context 'stateが一致しない場合' do
        let!(:state) { 'inactive' }
        it '集計の対象に含まれないこと' do
          expect(subject).to eq(0)
        end
      end
    end
  end
end

こうすることで、各ケースの検証観点をdescribeやcontextの文言として書くことができ、テストの意図が伝わりやすくなります。
また、letを使って観点の変数の中身だけを変えることで、明確にこの条件だけ変えていることがわかります。

テストを書き始める際には、最初にdescribeやcontextの文言だけ書き出していくことでテスト観点の整理もできます。

ちなみに私たちのチームではGitHub Copilotを導入済みなので、上記のように大枠を書き出せばあとはCopilotが勝手に中身を書いてくれて爆速でテストが書けます。
AIにも意図が伝わりやすくなって一石二鳥ですね。

メソッドに修正が入った場合

後から新しい条件を足す場合でも、spec全体を組み立て直す必要はなく、「細かい条件の確認」セクションに新しいパターンを追加するだけで済みます。
先ほど例示した「canceledステータスの場合にはcancel_feeを加算する修正を加えた場合」でも、以下のようにstateによる絞り込みのセクションでvisitedの場合とcanceledの場合でcontextを分けて追加すれば十分そうです（通しの確認でもcanceledのレコードを追加してあげた方がベターですが）。

     describe 'stateによる絞り込み' do
       before do
-        create(:reservation, year: 2024, state:, fee: 100, user: create(:user, state: 'active'))
+        create(:reservation, year: 2024, state:, fee: 100, cancel_fee: 50, user: create(:user, state: 'active'))
       end
-      context 'stateが一致する場合' do
-        let!(:state) { 'visited' }
-        it '集計の対象に含まれること' do
+      context 'visitedの場合' do
+        it 'feeが集計の対象に含まれること' do
           expect(subject).to eq(100)
         end
       end
-      context 'stateが一致しない場合' do
-        let!(:state) { 'canceled' }
-        it '集計の対象に含まれないこと' do
+      context 'canceledの場合' do
+        it 'cancel_feeが集計の対象に含まれること' do
+          expect(subject).to eq(50)
+        end
+      end
+      context 'それ以外の場合' do
+        let!(:state) { 'pending' }
+        it 'feeもcancel_feeも集計の対象に含まれないこと' do
           expect(subject).to eq(0)
         end
       end

いかがでしょうか？？同じような悩みを持った方の参考になれば幸いです！