はじめに
テストコードを書く習慣も、近年ではかなり一般的なものになってきました。 ドワンゴ教育事業のバックエンドチームでも自発的にテストコードを書く文化は根付いており、実際に計測はしていませんが、カバレッジの観点だけで言えば、だいぶ充実した自動テストが用意されているかと思います。
しかし、多くのテストが「変更を検知すること」だけを目的としたような書き方がされており、それによってテストの可読性や修正容易性などが大きく犠牲になっているように感じました。
そして、その状況を改善するために、テストコードを書き方について、言語によらない一般的な考え方とチームで利用している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
の粒度が細かすぎる => そのcontext
はit
にするべきではないでしょうか?
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
機能を考えれば当然の話でありますが、let
やlet!
は変数定義の一種です。
だから、let
やlet!
の使用する際には、次にのような通常のプログラミングにおける変数利用のプラクティスに従うことが望まれます。
- 数を減らす
- スコープを減らす
- 再代入しない
- できるだけ利用個所の近くで宣言する
let
やlet!
は複数のブロックにまたがるスコープの広い変数なので、読む際の認知的負荷は通常のローカル変数より大きくなります。
次のような方法で、できるだけ少なくなるように意識しましょう。
let
やlet!
で宣言したその変数が1つのブロックでしか呼ばれていないのであれば、その変数は呼び出し元ブロック内のローカル変数にすることができます。- 例えば、
xxx
と:xxx_id
の両方がlet
やlet!
で宣言されている場合は、後者の呼び出しを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
テストコードもプログラミングの一種なので、原則的に、ネストが浅い方が可読性が高くなります。
単一階層のテストケース内にはlet
やlet!
を宣言することができないので、テストケースをネストさせないことは、それらの変数の利用を減らすことにもつながります。
ネストを浅く保つためには、次のようなことに気を付けると良いでしょう。
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では、以下のものを使うことが多いです。
- Equality matchers
- Predicate matchers
- Type matchers
contain_exactly
matcherend_with
matcherinclude
matcherraise_error
macherstart_with
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
で値を明示的に設定しましょう。
ただし次に挙げるような、 初期状態であることがある程度明らかであるデフォルト値については、デフォルトで初期状態を返すものとしても問題は起きにくいでしょう。
- 未定義値としての
nil
や0
、空文字列、空配列等 initial
のような、初期状態であることが名前から明白なenumの値- DBスキーマで定義されたデフォルト値
逆に、テストの実行結果に関係ない値については、factoryの生成時に具体的な指定を避け、読む人に余計なノイズを与えないようにしましょう。
参考にしたブログ記事等
- #2 テスト自動化の落とし穴:テストの不吉な臭い - 時を超えたプログラミングの道
- 【アンチパターン】Arrange、Act、Assert(AAA)を意識できていないRSpecのコード例とその対処法 - Qiita
- 【翻訳】RSpecのリードメンテナだけど何か質問ある? Q. シンプルなスタイルと複雑なスタイル、選ぶならどっち?
- rspecを読みやすくメンテしやすく書くために - Zenn
- 僕がRSpecでsubjectを使わない理由 - give IT a try
- Rails tips: RSpecの
let
ブロックやbefore
ブロックは基本避けるべき(翻訳)|TechRacho by BPS株式会社 - willnet/rspec-style-guide: 可読性の高いテストコードを書くためのお作法集
- リーダブルテストコード / #vstat
付録: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!
株式会社ドワンゴの教育事業では、一緒に未来の当たり前の教育をつくるメンバーを募集しています。
カジュアル面談も行っています。 お気軽にご連絡ください!
開発チームの取り組み、教育事業の今後については、他の記事や採用資料をご覧ください。