はじめに
先日、コードレビューをしていると、「むむ、これは！？」と思うようなRailsのテストコードに遭遇しました。
イメージ的にはこんなテストコード（RSpec）です。
# トレーニングジムの予約システムを開発していると仮定してください
describe 'キャンセル処理' do
let(:user) { create :user }
let(:reservation) {
create :reservation,
user: user,
start_at: '2017-08-10 10:00'.in_time_zone
}
context '24時間前をすぎるとキャンセル料が発生する' do
before do
travel_to '2017-08-09 10:00'.in_time_zone
reservation.cancel!
end
after { travel_back }
let(:billing) { user.billings.first }
it { expect(user.billings.count).to eq 1 }
it { expect(billing.amount).to eq 500 }
end
end
上のコードは僕から見ると、いろいろとよろしくないテストコードになっています。
みなさんはどこに問題があるか気づけるでしょうか？
この記事では上のテストコードの問題点を指摘し、望ましいテストコードに修正する方法を説明します。
なお、この記事を読むためにはRSpecの基礎知識が必要になります。
RSpecについて詳しくない方はこちらの入門記事を先に読んでおいてください。
テストコードのAAA（Arrange、Act、Assert）を理解する
テストコードの基本ステップはArrange、Act、Assert（準備、実行、検証、以下AAA）の3ステップに分かれます。
先ほどのテストコードであれば、それぞれ次のように分けられます。
- Arrange（準備）= 予約オブジェクト（
reservation）を作成し、システム日時を24時間前を過ぎた状態に変更する（travel_to ...） - Act（実行）= 予約のキャンセルを実行する（
reservation.cancel!） - Assert（検証）= 適切な金額のキャンセル料が発生していることを確認する（
expect...）
RSpecのコードであれば、これら3つのステップが以下の場所に分散していることが望ましいです。
- Arrange = let/let!、もしくはbeforeの中（場合によってはitの中でも可）
- Act = itの中
- Assert = itの中
この点を確認した上で先ほどのコードをもう一度確認してみましょう。
# トレーニングジムの予約システムを開発していると仮定してください
describe 'キャンセル処理' do
# Arrange
let(:user) { create :user }
let(:reservation) {
create :reservation,
user: user,
start_at: '2017-08-10 10:00'.in_time_zone
}
context '24時間前をすぎるとキャンセル料が発生する' do
before do
# Arrange
travel_to '2017-08-09 10:00'.in_time_zone
# Act
reservation.cancel!
end
after { travel_back }
# Arrangeではなく、Actの結果を取得している
let(:billing) { user.billings.first }
# Assert
it { expect(user.billings.count).to eq 1 }
it { expect(billing.amount).to eq 500 }
end
end
ご覧のように、Actがbeforeの中に入っています。
また、let(:billing)はArrangeのためではなく、Actの結果を確認するために存在しています。
AAAの切り分けが適切でないと何をテストしているのかわかりづらい
先ほどのテストコードを見た瞬間、僕は「ん？このテストは何の振る舞い（どのメソッド）をテストしてるんだ？？」と不思議に思いました。
これは、Actにあたる処理がbeforeの中に書かれ、itの中には書かれていなかったためです。
このようにAAAが不適切にテストコード内に散らばっていると、テストしたい振る舞いの「何をどうする（その結果、何がどうなる）」が不明確になり、コードの読み手を混乱させる原因になります。
AAAを適切に切り分けたテストコードに書き直す
というわけで、このテストコードのAAAを適切に切り分けましょう。
僕が書くなら、次のようなテストコードに書き直します。
describe 'キャンセル処理' do
# Arrange
let(:user) { create :user }
let(:reservation) {
create :reservation,
user: user,
start_at: '2017-08-10 10:00'.in_time_zone
}
context '24時間前をすぎた場合' do
# Arrange
before do
travel_to '2017-08-09 10:00'.in_time_zone
end
after { travel_back }
it 'キャンセルするとキャンセル料が発生する' do
# Act
reservation.cancel!
# Assert
expect(user.billings.count).to eq 1
billing = user.billings.first
expect(billing.amount).to eq 500
end
end
end
このように書くとitの中を見たときに、「ああ、キャンセルを実行したから支払いが発生したんだな」ということが明確にわかるはずです。
contextの中の文言にも注目！
また、contextで書いていた文言が変わっている点にも注目してください。
- 修正前 =
context '24時間前をすぎるとキャンセル料が発生する' - 修正後 =
context '24時間前をすぎた場合'+it 'キャンセルするとキャンセル料が発生する'
修正前はcontextの中に条件と振る舞いが両方入っていました。
一方、修正後はcontextの中には条件が、itの中には期待する振る舞いが書かれています。
RSpecはBDD（Behavior Driven Development、振る舞い駆動開発）向けのテスティングフレームワークです。
つまり振る舞いを検証するが目的なので、itの引数には振る舞いに関する説明を記述し、ブロックの内部にはそれを検証するコードを書くのが望ましいです。
it '(振る舞いに関する説明)' do
# 対象のメソッドが上の説明のとおりに振る舞うことを検証する
end
このようにcontextとitの文言をきちんと書くことで、AAAが正しく切り分けられているかどうかの確認ができます。
Tips：先に中身のないitを書くとAAAをきれいに切り出しやすい
AAAがちゃんと切り出せているかどうか自信がない人は、テストコードを書き始める前に大枠だけ固めてしまいましょう。
具体的には次のように「中身のないit」から書き始めると、AAAをうまく切り出しやすくなります。
describe 'キャンセル処理' do
context '24時間前をすぎた場合' do
it 'キャンセルするとキャンセル料が発生する'
end
context '24時間前よりも前の場合' do
it 'キャンセルしてもキャンセル料が発生しない'
end
end
RSpecは中身のないitを「未着手（pending）のテスト」して扱うので、この状態でテストを実行することもできます。
【Special thanks】
このテクニックを提案してくれた @mah_lab に感謝！
ソニックガーデンのメンバーもよくやってるようです。
（というか、僕もよくやります！）
2017.9.17追記：changeを使うのもアリ
先ほどの例ではActとAssertを明確に分けるために、cancel!のあとで普通にexpectを使いましたが、「AするとBになること」を検証する場合は次のようにexpect + changeを使うのもアリです。
この場合はActとAssertを1行で実施することになります。
it 'キャンセルするとキャンセル料が発生する' do
# Act & Assert
expect { reservation.cancel! }.to \
change { user.billings.count }.from(0).to(1)
# Assert
billing = user.billings.first
expect(billing.amount).to eq 500
end
beforeやletを乱用して「一石二鳥でDRYなコード」を目指すのはNG
ところで、最初に紹介した問題のコードがどうして生まれてきたのか、僕にはなんとなく予想がつきます。
おそらくこのコードを書いた人は「記述効率のよいテストコード」を目指していたんだと思います。
beforeやletなど、RSpecに用意された各種機能を駆使すると、重複を排除して効率よくテストコードを書くことができます。
そしてプログラマは「DRYなコードこそ善」と信じているので、「テストコードもDRYに書きたい」という心理が働きがちです。
しかし、プロダクションコードとテストコードはそもそもの目的が異なることを理解しましょう。
「DRYであることが常に善」であるプロダクションコードとは異なり、テストコードの場合は「対象の処理がちゃんと動いていることが一目でわかる、ドキュメントのようなコードこそが善」です。
「beforeやletを駆使した無駄にカッコいいコード」よりも、「愚直にベタ書きした読みやすいテストコード」を重視しましょう。
余談：subjectは無理に使わない方がよい
ちょっと話が脱線しますが、もしかするとみなさんの中には「subjectは？ねえ、subjectは使わないの？？」と思っている人がいるかもしれません。
TwitterとかでRSpecに関するツイートを見ていたりすると、ときどき「subjectをどう書くか」で悩んでいる人を見かけます。
また、「RSpecのテストコードはsubjectをうまく切り出して、it { is_expected.to ... }の形式で書くのが一番スマートだ」と考えている人も結構多いように思います。
ですが、僕のポリシーは「subjectは原則使わない。subjectを使った方が明らかに効率がよく、可読性も落ちないときだけ例外的に使う」です。
ためしに先ほどのコードをsubjectを使うように直してみましょう。
describe 'キャンセル処理' do
let(:user) { create :user }
let(:reservation) {
create :reservation,
user: user,
start_at: '2017-08-10 10:00'.in_time_zone
}
context '24時間前をすぎるとキャンセル料が発生する' do
before do
travel_to '2017-08-09 10:00'.in_time_zone
end
after { travel_back }
# subjectを定義する
subject { reservation.cancel!; user.billings }
# subjectを使って検証する
it { expect(subject.count).to eq 1 }
it { expect(subject.first.amount).to eq 500 }
end
end
うーん、やるとすればこんな感じでしょうか。。
もしかすると「いやいや、subject(:billings)って書いた方がいいよ！」とか「こんなふうにsubjectを切り出せば、itの中をもっとスマートに書けるよ！」みたいに思っている人もいるかもしれません。
ですが、そもそもsubjectをあらゆる場面で適用しようとすると、「どうやってsubjectをうまく切り出すか」ということに頭を使いすぎてしまいます。
時間をかけてでもうまく切り出せたならまだ良いですが、subjectとして扱いづらいオブジェクトをむりやりsubjectに切り出すと、いびつで読みにくいテストコードになります（実際、上のコードも可読性が悪いと思います）。
なので、僕個人の意見としては、「subjectはいったん窓から投げ捨ててitの中にベタ書きした方が、読みやすいテストコードを短時間で書けるよ」と考えています。
まとめ
というわけで、この記事では「Arrange、Act、Assert（AAA）を意識できていないRSpecのコード例」と、その対処法を説明してみました。
これまでAAAを意識してこなかった人は、自分のテストコードを見直してAAAが適切な場所に切り分けられているかチェックしてみましょう。
beforeやletを駆使しまくって「やった、DRYになった！」と喜んでいると、テストの可読性を落とす原因になりますよ！
あわせて読みたい
AAAの話は先日発売された書籍「Effective Testing with RSpec 3（洋書）」にも載っています。
他にも「テストコードはかくあるべし」という話が豊富に載っているので、この本を読めばRSpecの書き方とテストコードの基礎知識をしっかり学べます。
DRYでカッコいいテストコードは無理に書かなくていいよ、という話は過去にもいくつか書いています。
こちらもあわせてどうぞ。
可読性の高いRSpecのテストコードを書くためには、こちらのサイトも参考になると思います。
