テスト初心者がRSpecで必要十分なテストを書く

今年の4月から新卒エンジニアとしてvivit株式会社で働いている氏家です。私は現在、主にアウトドアWebメディアhinataの開発を行うmediaチームに所属しており、Railsを中心にコードを書いております。

最近、ある機能の実装のためコントローラにメソッドを追加しそのテストをRSpecで書いていたのですが、テスト(この場合は単体テスト)についての知識が乏しく、どのように書けばいいか分からず苦戦しました。しかしこのタスクに着手しているとき、ちょうどソフトウェアテストをテーマにエンジニア研修が実施されており、自分の書いたテストのどこが不適切なのかを知ることができました。

今回は僕のようなテスト初心者がどのようなマインドでテストを書いていたのかと、必要十分なテストを書くために学んだことを話していけたらと思います。

メソッドの仕様

コントローラに追加するメソッドは「データベースのcategoriesテーブルから値を取得し、変数に格納する」という単純なものです。このメソッドはあらゆる場面で使用されるため、ApplicationControllerに記述しています。

# frozen_string_literal: true

class ApplicationController < ActionController::Base

~~~

before_action :set_categories

~~~

def set_categories
  @categories = Category.where.not(priority: 0).order(priority: :desc).limit(6)
end

取得する条件は「priorityカラムが0ではないレコードをpriorityの降順に6つまで取得すること」です。

必要十分なテストとは

テストはただ書けばいいというものではなく、テストケースが メソッドが仕様通りの動作をするかどうか(正常時、異常時の双方) を網羅している必要があります。さらに、1つの動作を確認するのに同じようなテストケースが複数あったり回りくどい書き方をしていても意味がなく、テストの実行時間が遅くなる原因になってしまいます。

メソッドが取りうる動作を、最低限のテストケースで網羅することが必要十分なテストといえます。

最初に書いたテスト

まず僕がテストについて理解せず書いたset_categoriesメソッドのテストがこちらです。

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe ApplicationController, type: :controller do

~~~

  describe '#set_categories' do
    let!(:category1) { FactoryBot.create(:category, name: 'カテゴリ1', priority: 6) }
    let!(:category2) { FactoryBot.create(:category, name: 'カテゴリ2', priority: 5) }
    let!(:category3) { FactoryBot.create(:category, name: 'カテゴリ3', priority: 4) }
    let!(:category4) { FactoryBot.create(:category, name: 'カテゴリ4', priority: 3) }
    let!(:category5) { FactoryBot.create(:category, name: 'カテゴリ5', priority: 2) }
    let!(:category6) { FactoryBot.create(:category, name: 'カテゴリ6', priority: 1) }
    subject { controller.send(:set_categories) }

    context 'カテゴリが6つ以下の場合' do
      it 'priorityの順にすべて取得する' do
        is_expected.to match([category6, category5, category4, category3, category2, category1])
      end
    end

    context 'カテゴリが7つ以上場の場合' do
      let!(:category1) { FactoryBot.create(:category, name: 'カテゴリ1', priority: 8) }
      let!(:category7) { FactoryBot.create(:category, name: 'カテゴリ7', priority: 7) }
      it 'priorityの順で6つだけ取得する' do
        is_expected.to match([category6, category5, category4, category3, category2, category7])
      end
    end
  end
end

僕が必要だと感じたテストケースは、降順に取得できるかどうかと7つ以上取得していないかどうかの2つでした。

コードを順番に見ていきます。

前提

まずFactoryBot.createでカテゴリを6つ作成します。テストケース毎に作成させたかったためlet!としています。

テストケース1

1つ目のテストケースではカテゴリがpriorityの降順で取得されているか確認したかったので、matchマッチャで順番通りに格納されているか確認します。

テストケース2

2つ目のテストケースではメソッドの.limit(6)が正常に動作しているか確認するため、追加でcategory7を作成しました。その際、category1のpriorityの値を変えて新しく作成することで、降順で取得されているかどうかを確実なものにしようと考えました。

エンジニア研修で学んだ内容と先輩エンジニアのフィードバックから、このテストはテストケースが不十分であり、かつ書き方にもいくつか問題があることがわかりました。特にlet!で余計なデータを作ることは、テストの実行速度が遅くなる原因になるうえに、テストケース前に全て実行されてしまうのであまり使うべきではないことを学びました。

必要十分なテスト

まず必要十分なテストケースを考えるために、改めてメソッドの仕様を確認してみます。

def set_categories
  @categories = Category.where.not(priority: 0).order(priority: :desc).limit(6)
end

このメソッドはcategoriesテーブルからpriorityが0ではないレコードをpriorityの降順に6つまで取得し、@categoriesに格納します。つまり以下のような仕様になります。

priorityが0ではないレコードを取得すること
priorityの降順で取得すること
最大6つまで取得すること
@categoriesに格納されること

それぞれのテストケースを作成することを考えると、例えば「priorityの降順に取得すること」をテストするのにテスト用のカテゴリが6つもいらなかったり、「priorityが0ではないレコードを取得すること」をテストするテストケースが不足していることが分かります。

以上の点を踏まえ、必要十分なテストを目指し修正したset_categoriesメソッドのテストは以下のようになりました。

# frozen_string_literal: true

require 'rails_helper'

RSpec.describe ApplicationController, type: :controller do

~~~

  describe '#set_categories' do
    subject { controller.send(:set_categories) }

    context do
      before do
        FactoryBot.create(:category, priority: 1)
        FactoryBot.create(:category, priority: 2)
        FactoryBot.create(:category, priority: 0)
      end

      it 'priorityが0でないカテゴリをpriorityの降順ですべて取得する' do
        res = subject
        expect(res.size).to eq 2
        expect(res.first.priority).to eq 2
        expect(res.last.priority).to eq 1
      end

      before { subject }

      it '@categoriesに格納される' do
        expect(assigns(:categories)).not_to be_nil
      end
    end

    context 'カテゴリが7つ以上ある場合' do
      before do
        FactoryBot.create(:category, priority: 1)
        FactoryBot.create(:category, priority: 2)
        FactoryBot.create(:category, priority: 3)
        FactoryBot.create(:category, priority: 4)
        FactoryBot.create(:category, priority: 5)
        FactoryBot.create(:category, priority: 6)
        FactoryBot.create(:category, priority: 7)
      end

      it 'priorityの降順で6つ取得する' do
        res = subject
        expect(res.size).to eq 6
        expect(res.first.priority).to eq 7
        expect(res.last.priority).to eq 2
      end
    end

    context '該当するカテゴリが無い場合' do
      before do
        FactoryBot.create(:category, priority: 0)
        subject
      end

      it '@categoriesに空の配列が格納される' do
        expect(assigns(:categories)).to eq []
      end
    end
  end
end

順番に見ていきます。

テストケース1

まず1つ目のテストケースでは、以下の仕様をテストしています。

priorityが0ではないレコードを取得すること
priorityの降順で取得すること
@categoriesに格納されること

1つ目の仕様のテストのためにpriorityが0のカテゴリが1つと、priorityの値が違うカテゴリを2つ作成しています。先ほどのテストと違い、let!でインスタンス変数を定義しておらず、テストするのに最低限のレコードしか作成していません。

テストの内容も、わざわざマッチャを使用して順番まで完全に一致しているか見る必要がなく、priority: 1、priority: 2として作られたカテゴリを取得した際に、最初のカテゴリのpriorityが2、最後のカテゴリのpriorityが1であれば降順で取得できていることが分かります。配列の大きさを確認することで、priorityが0のカテゴリは含まれていないことも確認できます。

テストケース2

2つ目のテストケースでは以下の仕様をテストしています。

最大6つまで取得すること
priorityの降順で取得すること

6つまでしか取得しないことをテストするのにカテゴリを7つ作成する必要がありますが、先ほど同様にインスタンス変数を定義する必要はありません。配列の大きさと最初と最後のpriorityの値を見て、降順で6つ取得されているかをテストします。

テストケース3

3つ目のテストケースは、「何も取得できなかった場合(異常時)」をテストしています。

priorityの値が0のカテゴリを1つだけ作成し、set_categoriesが実行されたとき、@categoriesには空の配列が格納されることを確認します。

まとめ

これからはテストを書く際、以下のような部分に気を付けていきたいと思いました。

異常時のことも考える
不要なコードを書かない(テストが遅くなる)
そもそも機能の実装時に仕様を正しく理解しておく

特に不要なコードを書かないようにするためには、それが不要かどうか知らなくてはいけないし、考えれなくてはいけません。テストを書く回数を重ね、何が最適であるかを模索するのも必要十分なテストを書くうえで必要です。

おわりに

メソッドの仕様や異常時の振る舞いを最低限の処理、テストケースで確認することができたと思っていますが、まだテストケースが不十分だったり余計な処理が含まれているかもしれません。今までテストを作成するという文化を知らずに生きてきたのでまだ苦手意識がありますが、テストを作成する際は必要十分なテストになるよう心がけていきたいです。