なんか気軽にメソッド追加したら、\\
わけわかんないテストでこけて、\\
今日はテスト通すのに1日かかりました、上長。
テストを通すのに時間をかかりすぎているときが一番、
僕はエンジニア辞めたくなります。
「なんでこんな書き方したの?」の大海原Rspec。
今回の記事は、
describe,context,exampleの使い時を掴めない人や、
人が理解しやすくなるようにrspecを書きたい人向けの記事でございます
describe,context,exampleはなぜ書くか?
describe,context,exampleはなくても、
テストデータを生成しexceptに期待する動作を書くことでテストは行えます。
しかし、describe,context,exampleが無かったり間違った使い方をすると以下のようなことが起きます。
・テストを追加するときに無駄なデータ生成や宣言を行いやすくなる。
・テスト内容が理解しづらくなる。
塵も積もれば山となる的な問題なので、その場では無視しやすいですが、
正しい使い方を心がけましょう。
今回は、正しくテストはできているがわかりづらい書き方をしているテストをリファクタするような流れで説明していきます。
段階的にリファクタしていくので、部分的に見るとベストプラクティスではないので、ご注意ください!ただ、どれもより良く改善をするように書くのを心がけました。
今回の例
今回は、並び替え(order_by_displayorder_and_updatedat
)を例に書いていきます。
この並び替えメソッドの処理は、
⓪更新日順の並び替えよりも、表示順の並び替えを優先する。
①表示順(display_order
)を昇順に。
②更新日順(updated_at
)を降順に。
という並び替えを行います。
例えば、以下のような"元データ"を差し込んだ場合、
"並び替え後のデータ"は以下のようになります。
###[元データ]
名前 | 表示順(display_order) | 更新日(updated_at) |
---|---|---|
カテゴリ1 | 2 | 2017/05/09 13:04 |
カテゴリ2 | 1 | 2017/05/09 13:05 |
カテゴリ3 | 1 | 2017/05/09 13:06 |
カテゴリ4 | 2017/05/09 13:07 | |
カテゴリ5 | 2017/05/09 13:08 |
###[並び替え後のデータ]
名前 | 表示順(display_order) | 更新日(updated_at) |
---|---|---|
カテゴリ3 | 1 | 2017/05/09 13:06 |
カテゴリ2 | 1 | 2017/05/09 13:05 |
カテゴリ1 | 2 | 2017/05/09 13:04 |
カテゴリ5 | 2017/05/09 13:08 | |
カテゴリ4 | 2017/05/09 13:07 |
アンチパターン
テスト自体は正確に行えているが、
分かりにくくなりやすいテストが以下です。
require 'rails_helper'
RSpec.describe Category, type: :model do
# 生成順でupdated_atが変わるテイです。
# updated_atはこれから省略します
create(:category, id: 1, name: "category 1", display_order: 2, updated_at: Time.current + 1.days)
create(:category, id: 2, name: "category 2", display_order: 1, updated_at: Time.current + 2.days)
create(:category, id: 3, name: "category 3", display_order: 1, updated_at: Time.current + 3.days)
create(:category, id: 4, name: "category 4", updated_at: Time.current + 4.days)
create(:category, id: 5, name: "category 5", updated_at: Time.current + 5.days)
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [3, 2, 1, 5, 4]
end
一見、問題ないようにも思えます。
事実、categoryテストがこれからも少ないなら問題ないかもしれません。
しかし、このテストには以下のような問題があります。
- このrspecから、テストデータだけではテスト内容を理解しづらい。category.rbに定義されている"order_by_displayorder_and_updatedat"の処理を読みに行く必要が出てくる。
- 「display_order」「updated_at」以外の要素も考慮しなくてはいけないメソッドになった場合、その分だけテストデータが増え、並び替え処理で何をしているかわかりにくくなる。
- Categoryモデルの他のメソッドもテストする時に、テストデータやexceptが何を対象としているのか分かりにくくなる。
- 本来、
Category.order_by_displayorder_and_updatedat
のテストをしたいだけなのに、テストデータの生成のコードが多く、処理が埋もれてしまっている。 - テスト項目が増えたときに同じ内容のcreate文が重複するので、改変が必要になったときに変更箇所が多くなる。
ざっくりこれらをまとめて言うと、
テスト項目が増えてくるとテスト内容がより理解しづらくなります。
明日からプロジェクトに参加する人にも分かるように書くつもりで、
リファクタしていきましょう。
リファクタ1:テスト内容を伝える「example」
まずは何のテストをしているのか説明してあげましょう。
RSpec.describe Category, type: :model do
example "order_by_displayorder_and_updatedatはdisplay_order昇順,updated_at降順で並び替える" do
create(:category, id: 1, name: "category 1", display_order: 2)
create(:category, id: 2, name: "category 2", display_order: 1)
create(:category, id: 3, name: "category 3", display_order: 1)
create(:category, id: 4, name: "category 4")
create(:category, id: 5, name: "category 5")
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [3, 2, 1, 5, 4]
end
end
example
を書いたおかげで、spec内だけで、
order_by_displayorder_and_updatedat
おおよそ理解できるようになりました。
今回は、order_by_displayorder_and_updatedat
という処理内容が分かりやすいメソッド名にしたので、説明過多にも思えますが、
例えば、order_default
のようなメソッド名だと、名前だけで処理を把握するのは困難になるので、説明してあげましょう。
[注意!]
この時にexample "order_by_displayorder_and_updatedatが正しく並び替えする"
というような説明を過去自分は書いていましたが、これはやめましょう。
正しく処理することは当たり前なので、どのような処理をしているのかを説明してあげましょう。
リファクタ2:場合分けを伝える「context」
現在の例では、5つのテストデータを並び替えています。
order_by_displayorder_and_updatedat
には、
「display_orderの並び替えが昇順で行われているか」という問題と
「updated_atの並び替えが降順で行われているか」という問題と
「updated_atよりもdisplay_orderを優先できているか」という問題が関わっています。
このような3つの問題を一つのexampleに収めていると、
example内で並び替えの条件が見えづらい問題があります。
なので、これらの問題をcontextで場合分けしてテストを行います。
RSpec.describe Category, type: :model do
context "order_by_displayorder_and_updatedatで元データにdisplay_orderの違いがある場合" do
example "display_orderを昇順に並び替える" do
create(:category, id: 1, name: "category 1", display_order: 2)
create(:category, id: 2, name: "category 2", display_order: 1)
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [2, 1]
end
end
context "order_by_displayorder_and_updatedatで元データにupdated_atの違いがある場合" do
example "updated_atを降順に並び替える" do
create(:category, id: 1, name: "category 1")
create(:category, id: 2, name: "category 2")
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
end
context "order_by_displayorder_and_updatedatで元データにdisplay_order, updated_atの違いがある場合" do # ちょっと日本語がおかしいのはご容赦ください🙇
example "display_orderを優先して並び替える" do
create(:category, id: 1, name: "category 1", display_order: 1)
create(:category, id: 2, name: "category 2")
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
end
end
このように場合分けすることで、それぞれのexampleに関わっているテストデータも減り、
それぞれのexample内で何をチェックしたいのかが明示的になります。
リファクタ3:テストの対象を説明する「describe」
リファクタ2で3つのcontextを作成しましたが、
これらはすべてorder_by_displayorder_and_updatedat
メソッドに関するテストです。
この状態で、他のメソッドのテストを追加したときに、そのcontext,exampleが何を対象としたテストなのかが分かりづらくなってしまいます。
なので、この3つのcontextをdescribeでまとめてあげましょう。
RSpec.describe Category, type: :model do
describe "#order_by_displayorder_and_updatedat" do
context "元データにdisplay_orderの違いがある場合" do
example "display_orderを昇順に並び替える" do
create(:category, id: 1, name: "category 1", display_order: 2)
create(:category, id: 2, name: "category 2", display_order: 1)
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [2, 1]
end
end
context "元データにupdated_atの違いがある場合" do
example "updated_atを降順に並び替える" do
create(:category, id: 1, name: "category 1")
create(:category, id: 2, name: "category 2")
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
end
context "元データにdisplay_order, updated_atの違いがある場合" do # ちょっと日本語がおかしいのはご容赦ください:bow:
example "display_orderを優先して並び替える" do
create(:category, id: 1, name: "category 1", display_order: 1)
create(:category, id: 2, name: "category 2")
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
end
end
describe "#other_method" do
# hogehoge
end
end
3つcontextをdescribeで囲んであげただけですが、
これで他のメソッド(#other_method)のようなものを追加しても、
テストの対象が明示的になります。
リファクタ4: テストデータを共通化し、example内でのテストデータ生成を控える。
リファクタ1〜3でdescribe,context,exampleを作成した後は、
テストデータの共通化をしていきましょう。
テスト項目が少ない時は共通化のメリットはあまりありませんが、
example内ではテストしたい処理だけが書かれているようにすることで、このexampleで何をテストしたいのかが明確になります。
RSpec.describe Category, type: :model do
describe "order_by_displayorder_and_updatedat" do
context "元データにdisplay_orderの違いがある場合" do
before(:each) do
create(:category, id: 1, name: "category 1", display_order: 2)
create(:category, id: 2, name: "category 2", display_order: 1)
end
example "display_orderを昇順に並び替える" do
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [2, 1]
end
example "other example ..." do
#hogehoge
end
end
context "元データにupdated_atの違いがある場合" do
before(:each) do
create(:category, id: 1, name: "category 1")
create(:category, id: 2, name: "category 2")
end
example "updated_atを降順に並び替える" do
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
example "other example ..." do
#hogehoge
end
end
context "元データにdisplay_order, updated_atの違いがある場合" do # ちょっと日本語がおかしいのはご容赦ください:bow:
before(:each) do
create(:category, id: 1, name: "category 1", display_order: 1)
create(:category, id: 2, name: "category 2")
end
example "display_orderを優先して並び替える" do
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
example "other example ..." do
#hogehoge
end
end
end
end
また、テストデータを共通化しているときは、それぞれのexampleでテストデータの内容を更新するのは控えましょう。あるdesribe,context内で共通化しているテストデータを変更すれば、その以下にある、すべてのexampleは影響を受ける関係がベストです。
リファクタリングのビフォーアフター
リファクタリングの前後は以下のようになりました。
[before]
RSpec.describe Category, type: :model do
example "order_by_displayorder_and_updatedatはdisplay_order昇順,updated_at降順で並び替える" do
create(:category, id: 1, name: "category 1", display_order: 2)
create(:category, id: 2, name: "category 2", display_order: 1)
create(:category, id: 3, name: "category 3", display_order: 1)
create(:category, id: 4, name: "category 4")
create(:category, id: 5, name: "category 5")
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [3, 2, 1, 5, 4]
end
end
[after]
RSpec.describe Category, type: :model do
describe "order_by_displayorder_and_updatedat" do
context "元データにdisplay_orderの違いがある場合" do
before(:each) do
create(:category, id: 1, name: "category 1", display_order: 2)
create(:category, id: 2, name: "category 2", display_order: 1)
end
example "display_orderを昇順に並び替える" do
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [2, 1]
end
end
context "元データにupdated_atの違いがある場合" do
before(:each) do
create(:category, id: 1, name: "category 1")
create(:category, id: 2, name: "category 2")
end
example "updated_atを降順に並び替える" do
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
end
context "元データにdisplay_order, updated_atの違いがある場合" do # ちょっと日本語がおかしいのはご容赦ください:bow:
before(:each) do
create(:category, id: 1, name: "category 1", display_order: 1)
create(:category, id: 2, name: "category 2")
end
example "display_orderを優先して並び替える" do
expect(Category.order_by_displayorder_and_updatedat.map(&:id)).to eq [1, 2]
end
end
end
end
###「めちゃくちゃ行数増えたけど、これ優しいの・・・?」
ぱっと見そう思いますね。
しかし、テストコードは基本的に、「こけてから出会うもの」です。
もし、order_by_displayorder_and_updatedat
の処理において、「updated_atを降順に並び替えできなくする」コードを書いてしまったとします。
そのとき、
beforeの場合は、どんな間違った処理も「5つのテストデータから並び替えられたデータが違っている」ことしかわからないですが、
afterの場合は、example "updated_atを降順に並び替える"
でこけるので、「updated_atを降順に並び替える処理が違っている」ことがわかります。
こう考えると後者の方が優しいのは明らかですね🏹
まとめ
rspecをより良くする方法は無限大です。
今回のメソッドの例だと、過保護なテストだったかもしれません。
メソッドの複雑さ・理解しづらさ・重要度・利用頻度などから、
どれだけ充実させるかは決めましょう。
しかし、最低限の配慮としてdescribe,context,exampleの基本的な使い方は正しくしましょう。(エラーが起きてくれないので気をつけましょう。)
あなたのテストで、誰か、もしくは数週間後の自分が足止めを食らってしまいます
いつかの誰かに定時という名のプレゼントを
rspecでした。