Rspec style guide
可読性の高いテストコードを書くためのお作法集
みんなの意見を取り入れることでより良いガイドにしていきたいと思っているので、次に当てはまる場合はどんどんIssuesやPull Requestを送ってください。 The project is written primarily in HTML, first published in 2015. Key topics include: factory-girl, rspec, test.
RSpec スタイルガイド
これはなに
可読性の高いテストコードを書くためのお作法集です。
みんなの意見を取り入れることでより良いガイドにしていきたいと思っているので、次に当てはまる場合はどんどんIssuesやPull Requestを送ってください。
- 疑問に思う点がある
- ここに書かれていないお作法がある
- 内容はいいけど表現がおかしい
- もっといいサンプルコードがある
前提
- RSpec
- FactoryBot
describeとcontext
describeとcontextは同じメソッドだが、次のように使い分けることで何をテストしているのかをわかりやすくできる。
- describe の引数にはテストの対象を書く
- context の引数にはテストが実行される際に前提になる条件や状態を書く
例
rubyRSpec.describe Stack, type: :model do let!(:stack) { Stack.new } describe '#push' do context '文字列をpushしたとき' do it '返り値がpushした値であること' do expect(stack.push('value')).to eq 'value' end end context 'nilをpushした場合' do it 'ArgumentErrorになること' do expect { stack.push(nil) }.to raise_error(ArgumentError) end end end describe '#pop' do context 'スタックが空の場合' do it '返り値はnilであること' do expect(stack.pop).to be_nil end end context 'スタックに値があるとき' do before do stack.push 'value1' stack.push 'value2' end it '最後の値を取得すること' do expect(stack.pop).to eq 'value2' end end end end
FactoryBotのデフォルト値
FactoryBotを利用した場合、各モデルのデフォルトのカラム値を設定することになる。このとき、各カラムの値がすべてランダム値となるように設定を書くとよい。その上で、必要な値のみをテスト中で明示的に指定することにより、「このテストで重要な値はなにか」がわかりやすくなる。
よくない例
アカウントが有効化されているかどうかをactiveカラムで管理しているとする。このような、有効/無効を表すカラムが固定されているケースはよく見かける。
rubyFactoryBot.define do factory :user do name { 'willnet' } active { true } end end
rubyRSpec.describe User, type: :model do describe '#send_message' do let!(:sender) { create :user, name: 'maeshima' } let!(:receiver) { create :user, name: 'kamiya' } it 'メッセージが正しく送られること' do expect { sender.send_message(receiver: receiver, body: 'hello!') } .to change { Message.count }.by(1) end end end
このテストはUser#activeがtrueであることが暗黙的な条件になってしまっている。sender.active #=> falseのときやreceiver.active #=> falseのときにどう振る舞うかを伝えることができていない。
さらに、このテストではnameを明示的に指定しているがこれは必要な指定なのだろうか?テストを読む人に余計なリソースを消費させてしまう、無駄なデータ指定はなるべく避けるのが好ましい。
よい例
rubyFactoryBot.define do factory :user do sequence(:name) { |i| "test#{i}" } active { [true, false].sample } end end
rubyRSpec.describe User, type: :model do describe '#send_message' do let!(:sender) { create :user } let!(:receiver) { create :user } it 'メッセージが正しく送られること' do expect { sender.send_message(receiver: receiver, body: 'hello!') } .to change { Message.count }.by(1) end end end
このテストだと「User#activeの戻り値がUser#send_messageの動作に影響しない」ということが(暗黙的にであるが)伝わる。もしUser#activeが影響するような修正が加えられた場合、CIで時々テストが失敗することによって、テストが壊れたことに気付けるはずだ。
大事なのは「テストに依存している値をすべてテスト中に明示している」ということなので、それが守られているのであればFactoryBotのデフォルト値を固定にしてもかまわない。
ランダム値が原因でテストが失敗したときの再現方法
「各カラムの値をランダム値にしてしまうと、CIでテストが失敗した時に再現できないのでは?」という意見があるが、RSpecはデフォルトの設定で次のようにseed値を外から注入できるようになっている。
spec/spec_helper.rb
rubyKernel.srand config.seed
なので、次のようにCIで失敗したときのseed値をrspecコマンドの--seedオプションに指定することで同じ状況を再現できる。
rspec --seed 1234
FactoryBotでbelongs_to以外の関連をデフォルトで作成しない
FactoryBotでモデルを作成する際に、関連しているモデルも同時に作成することができる。
対象の関連がbelongs_toであれば特に問題はないが、has_manyの関連を扱う場合には注意が必要になる。
例として、UserとPostが一対多だったとしてFactoryBotでの定義を書いてみる。
rubyFactoryBot.define do factory :user do sequence(:name) { |i| "username#{i}" } after(:create) do |user, evaluator| create_list(:post, 2, user: user) end end end
after(:create)を使い、Userが作成された際に関連するPostも作成されるようにした。この定義を用いてUserが投稿したPostのうち、人気順で返すUser#posts_ordered_by_popularityのテストを書いてみる。
rubyRSpec.describe User, type: :model do describe '#posts_ordered_by_popularity' do let!(:user) { create(:user) } let!(:post_popular) do post = user.posts[0] post.update(popularity: 5) post end let!(:post_not_popular) do post = user.posts[1] post.update(popularity: 1) post end it 'return posts ordered by popularity' do expect(user.posts_ordered_by_popularity).to eq [post_popular, post_not_popular] end end end
理解しづらいテストコードになった。このテストはUserのレコードを作成した時に、関連するPostを2つ作成することに依存している。また、updateを利用してデータを変更しているため、最終的なレコードの状態を把握しづらくなっている。
これを避けるには、まず、デフォルトで一対多の関連レコードを作成することをやめるとよい。
rubyFactoryBot.define do factory :user do sequence(:name) { |i| "username#{i}" } trait(:with_posts) do after(:create) do |user, evaluator| create_list(:post, 2, user: user) end end end end
traitを利用し、デフォルトではPostを作成しないようにした。どんな値でも良いので関連先のPostがほしいときにはtraitを指定しUserを作成すれば良い。
関連先はテスト中で明示的に作成するようにする。
rubyRSpec.describe User, type: :model do describe '#posts_ordered_by_popularity' do let!(:user) { create(:user) } let!(:post_popular) { create :post, user: user, popularity: 5 } let!(:post_not_popular) { create :post, user: user, popularity: 1 } it 'return posts ordered by popularity' do expect(user.posts_ordered_by_popularity).to eq [post_popular, post_not_popular] end end end
これで、最初の例よりだいぶ見やすくなった。
日時を取り扱うテストを書く
注意: ただしく境界値分析を行いテストデータを生成すれば以下の手法は必要なくなる。以下の記述は補助的に使うのが良い。
日時を取り扱うテストを書く場合、絶対時間を使う必要がないケースであればなるべく現在日時からの相対時間を利用するのが良い。その方が実装の不具合に気づける可能性が増すからだ。
例として、先月に公開された投稿を取得するscopeと、そのテストを絶対時間を用いて記述する。
rubyclass Post < ApplicationRecord scope :last_month_published, -> { where(publish_at: (Time.zone.now - 31.days).all_month) } end
rubyrequire 'rails_helper' RSpec.describe Post, type: :model do describe '.last_month_published' do let!(:april_1st) { create :post, publish_at: Time.zone.local(2017, 4, 1) } let!(:april_30th) { create :post, publish_at: Time.zone.local(2017, 4, 30) } before do create :post, publish_at: Time.zone.local(2017, 5, 1) create :post, publish_at: Time.zone.local(2017, 3, 31) end it 'return published posts in last month' do Timecop.travel(2017, 5, 6) do expect(Post.last_month_published).to contain_exactly(april_1st, april_30th) end end end end
このテストは常に成功するが、実装にはバグが含まれている。
テストを相対日時に変更してみる。
rubyrequire 'rails_helper' RSpec.describe Post, type: :model do describe '.last_month_published' do let!(:now) { Time.zone.now } let!(:last_beginning_of_month) { create :post, publish_at: 1.month.ago(now).beginning_of_month } let!(:last_end_of_month) { create :post, publish_at: 1.month.ago(now).end_of_month } before do create :post, publish_at: now create :post, publish_at: 2.months.ago(now) end it 'return published posts in last month' do expect(Post.last_month_published).to contain_exactly(last_beginning_of_month, last_end_of_month) end end end
このテストは、例えば3月1日に実行すると失敗する。常にバグを検知できるわけではないが、CIを利用することで、ずっと不具合が残り続ける可能性を減らすことができるだろう。
日付を外部から注入する
timecopやtravel_toなどを使い現在時刻を変更しテストを実行するよりも、時刻を外部入力できるようにしたほうが堅牢なコードになる。詳細は次のブログエントリにまとまっている。
「現在時刻」を外部入力とする設計と、その実装のこと - クックパッド開発者ブログ
beforeとlet(let!)の使い分け
テストの前提となるオブジェクト(やレコード)を生成する場合、let(let!)やbeforeを使う。このとき、生成後に参照するものをlet(let!)で生成し、それ以外をbeforeで生成すると可読性が増す。
次のようなscopeを持つUserモデルがあるとする。
rubyclass User < ApplicationRecord scope :active, -> { where(deleted: false).where.not(confirmed_at: nil) } end
このテストをlet!のみを用いて書くと次のようになる。
rubyrequire 'rails_helper' RSpec.describe User, type: :model do describe '.active' do let!(:active) { create :user, deleted: false, confirmed_at: Time.zone.now } let!(:deleted_but_confirmed) { create :user, deleted: true, confirmed_at: Time.zone.now } let!(:deleted_and_not_confirmed) { create :user, deleted: true, confirmed_at: nil } let!(:not_deleted_but_not_confirmed) { create :user, deleted: false, confirmed_at: nil } it 'return active users' do expect(User.active).to eq [active] end end end
let!とbeforeを併用して書くと次のようになる。
rubyrequire 'rails_helper' RSpec.describe User, type: :model do describe '.active' do let!(:active) { create :user, deleted: false, confirmed_at: Time.zone.now } before do create :user, deleted: true, confirmed_at: Time.zone.now create :user, deleted: true, confirmed_at: nil create :user, deleted: false, confirmed_at: nil end it 'return active users' do expect(User.active).to eq [active] end end end
後者のほうが「メソッドの戻り値となるオブジェクト」と「それ以外」を区別しやすく、見やすいコードとなる。
※let!(:deleted_but_confirmed)のように名前をつけることで、どんなレコードなのか理解しやすくなると感じる人もいるかもしれない。しかしレコードに名前付けが必要であれば、単純にコメントとして補足してやればよいだろう
letとlet!の使い分け
基本的にlet!を推奨する。letの特性である遅延評価が有効に働く次のようなケースではletを使っても良い。
rubylet!(:user) { create :user, enabled: enabled } context 'when user is enabled' do let(:enabled) { true } it { ... } end context 'when user is disabled' do let(:enabled) { false } it { ... } end
letで定義した値は呼ばれない限り実行されないのでlet!よりコストが低い、ということを利点として挙げている人もいるが、これを鵜呑みにすると「letで定義した値がテストケースによって使われたり使われなかったりする」状態になってしまう。これはテストの前提条件を理解するコストを上げ可読性を落とす。
控えめなDRY
DRYにする行為は常に善だと思われているかもしれないが、そうではない。例えばコードの重複を一つにまとめるということは処理の抽象化をするということで、状況や抽象化の仕方によってはDRYによって減らされたコストを上回るコストが発生することもある。
shared_examplesはよく考えて使う
shared_examplesを利用するとコードの重複を削除できるが、書き方によってはかえって可読性を落とすことがある。
例として、引数として渡された曜日に対応した分だけポイントを増やすメソッドPoint#increase_by_day_of_the_weekのテストをshared_examplesを利用して書いてみる。shared_examplesの定義は別ファイルに書かれているとして、先にshared_examplesを利用する側だけのコードを見てみる。
rubyRSpec.describe Point, type: :model do describe '#increase_by_day_of_the_week' do let(:point) { create :point, point: 0 } it_behaves_like 'point increasing by day of the week', 100 do let(:wday) { 0 } end it_behaves_like 'point increasing by day of the week', 50 do let(:wday) { 1 } end it_behaves_like 'point increasing by day of the week', 30 do let(:wday) { 2 } end # ... end end
どんな前提条件で結果として何を期待しているのか、これだけを見て理解できるだろうか。
定義は次のようになる。
rubyRSpec.shared_examples 'point increasing by day of the week' do |expected_point| it "increase by #{expected_point}" do expect(point.point).to eq 0 point.increase_by_day_of_the_week(wday) expect(point.point).to eq expected_point end end
このテストが読みづらいのは、shared_examplesを成立させるために必要な前提条件が多いことが一つ挙げられる。
- テストされる主体である
point - メソッドに渡される引数である
wday - 期待する結果である
expected_point
また、それぞれの定義が分散していることが挙げられる
- 外側のlet(point)
it_behaves_likeブロック内のlet(wday)it_behaves_likeの第二引数(expected_point)
可読性を上げるには、まず、適切に名前をつけることが考えられるだろう。
rubyRSpec.shared_examples 'point increasing by day of the week' do |expected_point:| it "increase by #{expected_point}" do expect(point.point).to eq 0 point.increase_by_day_of_the_week(wday) expect(point.point).to eq expected_point end end RSpec.describe Point, type: :model do describe '#increase_by_day_of_the_week' do let(:point) { create :point, point: 0 } context 'on sunday' do let(:wday) { 0 } it_behaves_like 'point increasing by day of the week', expected_point: 100 end context 'on monday' do let(:wday) { 1 } it_behaves_like 'point increasing by day of the week', expected_point: 50 end context 'on tuesday' do let(:wday) { 2 } it_behaves_like 'point increasing by day of the week', expected_point: 30 end # ... end end
新しくcontextを作り、wdayについての説明を加えた。そして、期待する結果であるexpected_pointをキーワード引数として利用することで、実引数側に名前がつき、数値が何を表すのかすぐに理解できるようになった。
しかし、そもそもこれはshared_examplesを利用すべきケースなのだろうか?shared_examplesを利用せずに書くと次のようになる。
rubyRSpec.describe Point, type: :model do describe '#increase_by_day_of_the_week' do let(:point) { create :point, point: 0 } context 'on sunday' do let(:wday) { 0 } it "increase by 100" do expect(point.point).to eq 0 point.increase_by_day_of_the_week(wday) expect(point.point).to eq 100 end end context 'on monday' do let(:wday) { 1 } it "increase by 50" do expect(point.point).to eq 0 point.increase_by_day_of_the_week(wday) expect(point.point).to eq 50 end end context 'on tuesday' do let(:wday) { 2 } it "increase by 30" do expect(point.point).to eq 0 point.increase_by_day_of_the_week(wday) expect(point.point).to eq 30 end end # ... end end
it_behaves_likeでの前提条件や引数が多くなればなるほど複雑度が増す。DRYにするメリットが複雑度を上回るかどうか、慎重に考えるべきだ。
スコープを考慮する
describe 外にテストデータを置かない
例えば、次のような spec があるとする
rubydescribe 'sample specs' do context 'a' do # ... end context 'b' do let!(:need_in_b_and_c) { ... } # ... end context 'c' do let!(:need_in_b_and_c) { ... } # ... end end
この場合、b と c で同じ前提条件を利用しているので、一つ上のレベルに移動してDRYにしようと考える人もいるかもしれない。
rubydescribe 'sample specs' do let!(:need_in_b_and_c) { ... } context 'a' do # ... end context 'b' do # ... end context 'c' do # ... end end
しかし、これは良くない。'a' のコンテキストに、必要のない前提条件が含まれてしまうからだ。この例だけだとピンとこないかもしれない。このような let! が 10、context が 30 あって、どの let! がどの context に対応する前提条件なのかわからない状況を想像すると、下手に前提条件をまとめる怖さがわかるだろうか。
もちろん、すべての context において、共通で使う前提条件であれば、まとめてしまうのは問題ない。
各ブロックにおける前提条件の配置ルール
- 各ブロックの前提条件は、その配下のすべての expectation で利用するもののみ書く
- 特定の expectation のみで利用するものにおいては、その expectation に書く
落穂拾い
- 各ブロックにおける前提条件の配置ルール、の例外は次のようなケース
- だが、基本的には各ブロック内で宣言するほうが望ましい
rubylet!(:user) { create :user, enabled: enabled } context 'when user is enabled' do let(:enabled) { true } it { ... } end context 'when user is disabled' do let(:enabled) { false } it { ... } end
必要ないレコードを作らない
パフォーマンスの観点から、レコードを作らなくてすむ場合は作らないようにしたい。
rubydescribe 'posts#index' do context 'when visit /posts' do let!(:posts) { create_list :post, 100 } before { visit posts_path } it 'display all post titles' do posts.each do |post| expect(page).to have_content post.title end end end end
「100件の投稿タイトルが表示できること」をテストしたい場合は別だが、ただ投稿タイトルを表示できているかチェックできればいい場合、明らかに無駄なレコードを作っている。
この場合の最小限のレコード数は1件である。
rubydescribe 'posts#index' do context 'when visit /posts' do let!(:post) { create :post } before { visit posts_path } it 'display post title' do expect(page).to have_content post.title end end end
モデルのユニットテストでも、作らなくてよいレコードを作っているケースはよくある。
rubyRSpec.describe User, type: :model do describe '#fullname' do let!(:user) { create :user, first_name: 'Shinichi', last_name: 'Maeshima' } it 'return full name' do expect(user.fullname).to eq 'Shinichi Maeshima' end end end
User#fullnameはレコードが保存されているか否かに影響しないメソッドである。この場合はcreateではなくbuild(もしくはbuild_stubbed)を使う。
rubyRSpec.describe User, type: :model do describe '#fullname' do let!(:user) { build :user, first_name: 'Shinichi', last_name: 'Maeshima' } it 'return full name' do expect(user.fullname).to eq 'Shinichi Maeshima' end end end
このような単純なケースではUser.newを利用しても良い。
updateでデータを変更しない
FactoryBotで作成したレコード中のカラムをupdateメソッドで変更すると、最終的なレコードの状態がわかりにくくなるし、テストに依存している属性もわかりにくくなるので避ける。
rubyRSpec.describe Post, type: :model do let!(:post) { create :post } describe '#published?' do subject { post.published? } context 'when the post has already published' do it { is_expected.to eq true } end context 'when the post has not published' do before { post.update(publish_at: nil) } it { is_expected.to eq false } end context 'when the post is closed' do before { post.update(status: :close) } it { is_expected.to eq false } end context 'when the title includes "[WIP]"' do before { post.update(title: '[WIP]hello world') } it { is_expected.to eq false } end end end
Post#published?メソッドに依存している属性をすぐに理解することができるだろうか?updateはたいていFactoryBotのデフォルト値を「一番データとして多い形」に設定し、それを少し変更して使うために使われる。
updateは使用せず、FactoryBotのデフォルト値に記載したようにデフォルト値をランダムに保つと良い。
letを上書きしない
letで定義したパラメータを内側のcontextで上書きすると、updateでデータを変更しないで説明した例と同様に、最終的なレコードの状態がわかりにくくなるので避ける。
rubyRSpec.describe Post, type: :model do let!(:post) { create :post, title: title, status: status, publish_at: publish_at } let(:title) { 'hello world' } let(:status) { :open } let(:publish_at) { Time.zone.now } describe '#published?' do subject { post.published? } context 'when the post has already published' do it { is_expected.to eq true } end context 'when the post has not published' do let(:publish_at) { nil } it { is_expected.to eq false } end context 'when the post is closed' do let(:status) { :close } it { is_expected.to eq false } end context 'when the title includes "[WIP]"' do let(:title) { '[WIP]hello world'} it { is_expected.to eq false } end end end
subjectを使うときの注意事項
subjectはis_expectedやshouldを使い一行でexpectationを書く場合は便利だが、逆に可読性を損なう使われ方をされる場合がある。
rubydescribe 'ApiClient#save_record_from_api' do let!(:client) { ApiClient.new } subject { client.save_record_from_api(params) } # # ...多くのexpectationを省略している... # context 'when pass { limit: 10 }' do let(:params) { { limit: 10} } it 'return ApiResponse object' do is_expected.to be_an_instance_of ApiResponse end it 'save 10 items' do expect { subject }.to change { Item.count }.by(10) end end end
このようなとき、expect { subject }のsubjectは一体何を実行しているのかすぐには判断できず、ファイルのはるか上方にあるsubjectの定義を確認しなければならない。
そもそも"subject"は名詞であり、副作用を期待する箇所で定義すると混乱を招く。
is_expectedを利用し暗黙のsubjectを利用する箇所と直接subjectを明示する箇所が混在しており、どうしてもsubjectを使いたい場合は、subjectに名前をつけて使うと良い。
rubydescribe 'ApiClient#save_record_from_api' do let!(:client) { ApiClient.new } subject(:execute_api_with_params) { client.save_record_from_api(params) } context 'when pass { limit: 10 }' do let(:params) { { limit: 10} } it 'return ApiResponse object' do is_expected.to be_an_instance_of ApiResponse end it 'save 10 items' do expect { execute_api_with_params }.to change { Item.count }.by(10) end end end
expect { subject }の時よりはわかりやすくなったはずだ。
is_expectedを利用していない場合は、subjectの利用をやめてclient.save_record_from_api(params)を各expectationにべた書きするのが良い。
allow_any_instance_ofを避ける
公式のドキュメントにも書かれているが、allow_any_instance_of(expect_any_instance_of)が必要な時点でテスト対象の設計がおかしい可能性がある。
例として、次のようなStatement#issueのテストを書いてみる。
rubyclass Statement def issue(body) client = TwitterClient.new client.issue(body) end end
rubyRSpec.describe Statement do describe '#issue' do let!(:statement) { Statement.new } it 'call TwitterClient#issue' do expect_any_instance_of(TwitterClient).to receive(:issue).with('hello') statement.issue('hello') end end end
expect_any_instance_ofを使ってしまったのは、StatementクラスとTwitterClientクラスが密結合しているのが原因である。結合を弱めてみる。
rubyclass Statement def initialize(client: TwitterClient.new) @client = client end def issue(body) client.issue(body) end private def client @client end end
rubyRSpec.describe Statement do describe '#issue' do let!(:client) { double('client') } let!(:statement) { Statement.new(client: client) } it 'call TwitterClient#issue' do expect(client).to receive(:issue).with('hello') statement.issue('hello') end end end
issueメソッドを持つオブジェクトであれば、どのクラスでもclientとして扱えるように修正した。外部からclientを指定できる作りにしたことで、将来的にFacebookClientなど別のクライアントにも対応できるようになった。結合が弱まり、単純なモックオブジェクトでテストが記述できるようになった。
Contributors
Showing top 12 contributors by commit count.
