N予備校開発でのRSpecの書き方指針

サムネイル画像

はじめに

テストコードを書く習慣も、近年ではかなり一般的なものになってきました。 ドワンゴ教育事業のバックエンドチームでも自発的にテストコードを書く文化は根付いており、実際に計測はしていませんが、カバレッジの観点だけで言えば、だいぶ充実した自動テストが用意されているかと思います。

しかし、多くのテストが「変更を検知すること」だけを目的としたような書き方がされており、それによってテストの可読性や修正容易性などが大きく犠牲になっているように感じました。

そして、その状況を改善するために、テストコードを書き方について、言語によらない一般的な考え方とチームで利用しているRSpec機能関連の利用方針の2つの観点から、チーム内に向けての文章を書きました。 今回は、それに対してサンプルコードの追加などの増補改訂したものを掲載します。

なお、記事を読むにあたっては、以下のような点を承知しておいていただけますと幸いです。

  • すでにRSpecを使ったテストコードをある程度書いてきた開発者を想定読者としています
  • 個人的な考え方に基づいて書かれています。異論が出る部分も多々あるでしょう
  • 必ず守るべき「ルール」ではなく、参考にするための「方針」として書かれているため、ふんわりとした表現も多いです。(RuboCopで矯正しやすい部分についてはRuboCopのルールに導入しています)

テストコード一般の考え方

テストコードを書く目的としては、大きく次の3つを挙げることができます。

  • 実装したコードが正しく動作することを確認する
  • 後からコードを読むときに、仕様を把握するための助けとなる
  • 将来コードを修正したときに予期せぬ挙動変更に気付けるようにして、コードの修正に対する安心感を与える

このうちの後者2つに「後から」「将来」とあるように、テストコードを書く目的の多くは「将来」の開発を楽にするためにあります。

そのためテストコードは、製品コード以上に将来の負債とならないように、主に次のようなことに気を付けて書くことが求められます。

壊れにくいテストを書く

上記の通り、テストコードは将来の開発を助けるために書くものです。 しかし、バグ以外の本質的ではない理由で多くのテストが落ちるようになると、製品コードの修正よりもテストの修正に工数がかかるようになり、逆にテストコードが開発の足枷となってしまいます。

次のような特徴を持つテストコードは「壊れやすいテスト」になりがちだと言えるでしょう。

  • テストの結果が依存する要素が多い
  • クラスのインターフェースや入出力ではなく、内部実装の詳細をテストしている

また、モックやスタブはテスト対象クラスの依存先のクラスに対して用意するので、これらを使ったテストは基本的に対象クラスの実装の詳細に依存しており、内部実装の変化によるテスト修正が多くなりがちです。

その一方で、モックやスタブを使ったテストは実際にコードを実行するわけではないので、本来テストが落ちるべきである変更に対してもテストが通り、不具合を検知できないことがあります。

モックやスタブの利用は最小限にするべきであり、そのためにはテストコードだけではなく、テスト対象のクラス設計も重要になります。 もしモックやスタブを利用したくなった場合は、依存要素が多すぎる可能性があるので、一度クラス設計等から見直してみると良いでしょう。

実装した通りに動作することではなく、仕様通りに動作することをテストする

テストの粒度は細かければ細かいほど良いというものでもありません。 テストで検証する内容を細かくするほど、テストの実行するためのコストも、テストを読むためのコストも、どちらも増えていきます。

具体的なデメリットとして、例えば、

  • 本質的でないテストケースが多くなると、テストコードから仕様を把握するのが難しくなります。
  • テストケースの数が肥大化すると、CI等の実行時間が長くなり、開発効率の低下につながります。
  • 細かすぎるテストケースは、内部設計の些細な修正で失敗するようになり、「壊れやすいテスト」へとつながる一因となります。

テスト対象のコードを完成させた後にそれをテストを網羅するように書こうとすると、必要以上に観点が細かくなりがちです。 必ずしもテストコードから先に書く必要はないですが、テストを書く際には具体的なコードの内容は忘れ、「対象クラスが満たすべき仕様」の観点から必要なテストケースを考えるようにしましょう。

一般的に、次のようなものは仕様の本質的な部分の外にあり、テストコードで検証する必要性は低いと考えます。

  • 例外などのシステムメッセージの具体的内容 (確認する場合は定数等を利用する)
  • 引数の型チェックのような、呼び出し側が満たすべき事前条件の細かいチェック
  • 内部で他のクラスに委譲されている処理に対する、テスト条件の細かい分岐

テストコードはシンプルにわかりやすく書く

仕様把握のためにテストコードを読むときでも、テストの結果を元に製品コードを修正するときでも、何をどのようにテストされているか、読み手が容易に理解できることはとても重要です。

テストケースは継続的に追加されることはあっても、一度書かれたものが修正されることは製品コードよりも少なくなるため、テストコードにおいては変更容易性の重要度が比較的下がり、可読性や理解しやすさの重要度が高くなると言えます。

1つの目安としては、アサーション部分をエディタで表示したときに、スクロールせずともテストの内容がだいたい理解できるようになっていると良いでしょう。

そのためには以下のような指針が参考になります。

  • ifやループの利用は控える
  • メソッド定義などによるコード分割はほどほどに
  • 重複排除のために、メタプログラミングや高度な抽象化を使用しない
  • テストケースの階層は、少なすぎても多すぎても理解を妨げる
  • テストケース名などでテストの意図をちゃんと説明する。それでも足りなければ、コードコメントで補足する
  • RSpecなどのテストフレームワークの機能を使いすぎない

失敗の原因がわかりやすくなるように意識する

実行したテストの結果が失敗だった場合は、当然コードを修正しなければなりません。

コードを修正するためにはテストが失敗した原因箇所を特定する必要がありますが、テストの書き方によっては「何かが間違っていることはわかるけど、何が間違っているかわからない」状況になって困ってしまいます。

失敗した箇所のテストコードを読むことで修正箇所が把握できることは最低条件で、できればテストコードを見直さずともテスト結果の出力を見るだけで、問題のある箇所がある程度想像できるようになっていることが理想です。

そのためには、テストコードを書くときに以下のような点を意識すると良いでしょう。

  • 1つのテストケースでアサーションする範囲を適切に絞る
  • 結合テストのような高レイヤーのテストで、細かい条件にチェックを行わない
  • テストフレームワークが提供するアサーションメソッドを適切に使い分ける
  • 必要に応じて手動でアサーション失敗時のメッセージを設定する

RSpecの書き方

上記の考え方を実現するために、RSpecをどう利用していくかの話です。

掲載されたサンプルコードは説明用にゼロから用意されたもので、実際の開発で書かれたものとは無関係です。

テストケース名をitの引数で明記する

# bad
it do
  expect("leaf".pluralize).to eq("leaves")
end

# good
it "fで終わる単語を正しく複数形にできる" do
  expect("leaf".pluralize).to eq("leaves")
end

itには必ず、引数でテストの内容を端的に説明するテストケース名をつけましょう。 テストコードを読んで把握する際の大きな助けになります。

適切な名前が思いつかない場合は、次のような可能性があります。 テストケースの粒度を見直してみましょう。

  • 1つのテストケースで多くのことを確認しすぎている => テストケースを分割しましょう
  • contextの粒度が細かすぎる => そのcontextitにするべきではないでしょうか?

letよりもlet!を使う

# bad
describe "Company#user_exists" do
  let(:company) { create(:company) }
  let(:user) { create(:user, company: company) }

  it "指定したユーザーが存在する場合trueが返る" do
    # `user.id`が呼ばれたときにレコードが作成される
    expect(company.user_exists(user.id)).to be(true)
  end

  it "指定したユーザーが存在しない場合falseが返る" do
    # テストは通るが、`user`が呼ばれずレコードがそもそも作られないので、テストが正しく誤りを検知しない
    expect(company.user_exists(999)).to be(false)
  end
end


# good
describe "Company#user_exists" do
  let!(:company) { create(:company) }
  # 各テストケースの内容によらず必ずuserレコードが作成される
  let!(:user) { create(:user, company: company) }

  it "指定したユーザーが存在する場合trueが返る" do
    expect(company.user_exists(user.id)).to be(true)
  end

  it "指定したユーザーが存在しない場合falseが返る" do
    expect(company.user_exists(999)).to be(false)
  end
end

letは遅延評価で、初めて呼びされたときにブロックの内容が実行されます。 呼び出されなければ実行されません。

let!は定義された順番に、テストの実行時に必ず実行されます。

次のような理由から、基本的にletではなくlet!を使って変数を宣言していくことを推奨します。

  • ファイルの上から下に順に実行されるほうが、読み手が処理を追いやすい
  • letで実際に実行されないからと言って、テストケースに関係ない変数が宣言されていると、認知負荷が増えて可読性が低下する
  • 特にDBレコード作成などの副作用があるコードにおいて、テストのリファクタなどでletが呼ばれなくなってコードが実行されなくなると、原因に気付きにくいテストの失敗につながる

通常の変数と同じ方針に基づいてlet!を利用する

# bad
describe "UserList#add" do
  let!(:users) { [create(:user, active: true), create(:user, active: false)] }
  let!(:list) { UserList.new(users) }

  let!(:active_user) { create(:user, active: true) }
  let!(:inactive_user) { create(:user, active: false) }

  let!(:user) { nil }
  let!(:user_id) { user.id }
  let!(:should_success) { nil }

  context "userがactiveなとき" do
    let!(:user) { active_user }
    let!(:should_success) { true }

    it "戻り値がtrueになる" do
      expect(list.add(user)).to be(should_success)
    end

    it "idsに新しいuserのidが末尾に追加される" do
      list.add(user)
      expect(list.ids.last).to eq(user_id)
    end
  end

  context "userがinactiveなとき" do
    let!(:user) { inactive_user }
    let!(:should_success) { false }

    # 略
  end
end


# good
describe "UserList#add" do
  let!(:list) do
    users = [create(:user, active: true), create(:users, active: false)]
    UserList.new(users)
  end

  context "userがactiveなとき" do
    let!(:user) { create(:user, active: true) }

    it "戻り値がtrueになる" do
      expect(list.add(user)).to be(true)
    end

    it "idsに新しいuserのidが末尾に追加される" do
      list.add(user)
      expect(list.ids.last).to eq(user.id)
    end
  end

  context "userがinactiveなとき" do
    let!(:user) { create(:user, active: false) }

    # 略
  end
end

機能を考えれば当然の話でありますが、letlet!は変数定義の一種です。 だから、letlet!の使用する際には、次にのような通常のプログラミングにおける変数利用のプラクティスに従うことが望まれます。

  • 数を減らす
  • スコープを減らす
  • 再代入しない
  • できるだけ利用個所の近くで宣言する

letlet!は複数のブロックにまたがるスコープの広い変数なので、読む際の認知的負荷は通常のローカル変数より大きくなります。 次のような方法で、できるだけ少なくなるように意識しましょう。

  • letlet!で宣言したその変数が1つのブロックでしか呼ばれていないのであれば、その変数は呼び出し元ブロック内のローカル変数にすることができます。
  • 例えば、xxx:xxx_idの両方がletlet!で宣言されている場合は、後者の呼び出しをxxx.idとすることで宣言をまとめられないか確認してみましょう。

subjectを使わない

# bad
describe "#pluralize" do
  subject { str.pluralize }

  # ここに色んなテストケースが挟まって、上のコードと下のコードは同じ画面に入らないことがよくある

  context "fで終わる単語のとき" do
    let!(:str) { "leaf" }

    it { is_expected.to eq("leaves") }
  end
end

# good
describe "#pluralize" do
  it "fで終わる単語を正しく複数形にできる" do
    expect("leaf".pluralize).to eq("leaves")
  end
end

subjectについては以下の理由で利用を推奨しません。

  • ネストの浅い階層で宣言され、スコープが広くなりやすい変数である
  • letと同じ遅延評価の変数である
  • subjectという名前で参照すると、名前から値の中身が全く推測できない
  • is_expectedを使った省略記法の可読性が低い

subjectを使った記法は、特にテストケースが多くなってsubjectの中身の確認に画面のスクロールが必要になったときに、可読性が著しく下がります。

subjectの定義個所まで戻らずとも、各テストケースの定義個所だけを読んでテストの実行内容がわかるほうが、読む人に優しいテストになるでしょう。

不要なcontextでのネストを避ける

# bad
describe "#even?" do
  context "正常系" do
    context "偶数のとき" do
      let!(:number) { 2 }

      it "trueが返る" do
        expect(number.even?).to be(true)
      end
    end

    context "奇数のとき" do
      let!(:number) { 3 }

      it "正の数が返る" do
        expect(number.even?).to be(false)
      end
    end
  end
end

# good
describe "#even?" do
  it "正の数を渡したときtrueが返る" do
    number = 2
    expect(number.even?).to be(true)
  end

  it "負の数を渡したときfalseが返る" do
    number = 3
    expect(number.even?).to be(false)
  end
end

テストコードもプログラミングの一種なので、原則的に、ネストが浅い方が可読性が高くなります。

単一階層のテストケース内にはletlet!を宣言することができないので、テストケースをネストさせないことは、それらの変数の利用を減らすことにもつながります。

ネストを浅く保つためには、次のようなことに気を付けると良いでしょう。

  • context内にテストケースが1つしかない場合には、コンテクスト名とテストケース名をつなげて、1つのitのテストケースにまとめてしまいましょう。
  • 「正常系」「異常系」のような、共通変数などの"コンテクスト"の境界とならない単位でネストを作るのを避けましょう。

matcherを適切に使い分ける

# bad
it "配列に要素を追加できる" do
  list = ["hoge", "fuga"]
  item = "piyo"
  list.push(item)

  expect(list.include?(item)).to be(true)
end

# good
it "配列に要素を追加できる" do
  list = ["hoge", "fuga"]
  item = "piyo"
  list.push(item)

  expect(list).to include(item)
end

RSpecのテストコードは原理的にはbe_trueのmatcherだけで全てのテストケースを記述することができます。

しかし、例えば文字列比較のテストをexpect(a == b).to be_trueみたいに書いて、失敗したときにexpected true but got falseみたいな結果だけ表示されても困ります。

どのような文字列が実際に得られて、それがどう間違っているかが知りたいので、普通はexpect(a).to eq(b)みたいに書きます。

単純比較以外のテストでも失敗したときのメッセージは利用するmatcherによって異なります。 検証したい内容に応じてmatcherを適切に使い分けることで、間違いがあったときに修正を簡単にできるテストを作りやすくなります。

経験上、RSpecの組み込みのmatcherでは、以下のものを使うことが多いです。

factoryのデフォルト値に依存しないテストを書く

# factory定義(共通)
FactoryBot.define do
  factory :user do
    name { "default name" }
    age { 20 }
  end
end


#bad
describe "User#even_age?" do
  it "年齢が偶数のときtrueを返す" do
    # デフォルト値に依存したテスト
    user = create(:user)
    expect(user.even_age?).to be(true)
  end
end


#bad
describe "User#even_age?" do
  it "年齢が偶数のときtrueを返す" do
    # テスト無関係な値まで指定している
    user = create(:user, name: "another name", age: 18)
    expect(user.even_age?).to be(true)
  end
end


#good
describe "User#even_age?" do
  it "年齢が偶数のときtrueを返す" do
    user = create(:user, age: 18)
    expect(user.even_age?).to be(true)
  end
end

factory_botで自動で設定されるデフォルト値は後から変更されるものと考え、テストの実行結果が具体的な値に依存する場合は、各specファイルやtraitで値を明示的に設定しましょう。

ただし次に挙げるような、 初期状態であることがある程度明らかであるデフォルト値については、デフォルトで初期状態を返すものとしても問題は起きにくいでしょう。

  • 未定義値としてのnil0、空文字列、空配列等
  • initialのような、初期状態であることが名前から明白なenumの値
  • DBスキーマで定義されたデフォルト値

逆に、テストの実行結果に関係ない値については、factoryの生成時に具体的な指定を避け、読む人に余計なノイズを与えないようにしましょう。

参考にしたブログ記事等

付録:RuboCop設定

私たちのチームでは主に上記のような考え方に基づいて、RSpec周りについて下記のようなRuboCop設定を利用しています。 (デフォルト設定と異なる部分のみ記載)

利用できる機能を一部制限する一方で、メトリクス類の上限値は緩和することで、テストケースの構成には自由度を与えるようにしています。

# 英語を前提としたcontextの説明文の制限を無効化する
RSpec/ContextWording:
  Enabled: false

# GrapeAPI・Taskのテストは`describe SomeClass`から始めなくても良い
RSpec/DescribeClass:
  Enabled: true
  Exclude:
    - spec/api/**/*
    - spec/lib/tasks/**/*

# モックを用いた検証で、`receive`と`have_received`の両方を許容する
RSpec/MessageSpies:
  Enabled: false

# letよりもlet!を使うようにする代わりに、呼ばれないlet!を許容する
RSpec/Dialect:
  Enabled: true
  PreferredMethods:
    let: let!
RSpec/LetSetup:
  Enabled: false

# subject使わない
RSpec/ExampleWithoutDescription:
  Enabled: true
  EnforcedStyle: disallow
RSpec/ImplicitSubject:
  Enabled: true
  EnforcedStyle: disallow
RSpec/NamedSubject:
  Enabled: false

# RSpecまわりのMetrix制限の緩和
# 1つのテストケースあたりの行数
RSpec/ExampleLength:
  Enabled: true
  Max: 20 # default: 5
  CountAsOne: ["array", "hash", "heredoc"]
# 1つのテストケースあたりのアサーションの数
RSpec/MultipleExpectations:
  Enabled: true
  Max: 5 # default: 1
# letの定義回数の上限
RSpec/MultipleMemoizedHelpers:
  Enabled: true
  Max: 8 # default: 5
  AllowSubject: false # subjectも数に含める
# ネストの深さ
RSpec/NestedGroups:
  Enabled: true
  Max: 5 # default: 3

We are hiring!

株式会社ドワンゴの教育事業では、一緒に未来の当たり前の教育をつくるメンバーを募集しています。

カジュアル面談も行っています。 お気軽にご連絡ください!

カジュアル面談応募フォームはこちら

www.nnn.ed.nico

開発チームの取り組み、教育事業の今後については、他の記事や採用資料をご覧ください。

speakerdeck.com