はじめに
先日Qiitaでこちらの記事を拝見しました。
Tama.rb に行ってモブプロをやって feature spec をリファクタリングした話 - Qiita
僕が翻訳に携わっている電子書籍「Everyday Rails - RSpecによるRailsテスト入門」（以下Everyday Rails）を題材にしてTama.rbで勉強会が開かれたようで、そこで書籍内のコードをリファクタリングする例がこの記事に載っています。
Everyday Railsを題材にして勉強会を開いてもらったのは大変ありがたいですし、リファクタリングされたコードもこれはこれで興味深いのですが、個人的には「僕は普段こういう書き方はしないなー」と思いました。
というわけで、上の記事でリファクタリングされたコードを僕だったらどう書くか、という話をこの記事で紹介したいと思います。
動画があります
僕が具体的にどんなことを考えてコードを書き換えていったのかは以下の動画で詳しく説明しています。
こちらもご覧いただくと、より理解が進むと思います。
Re: Tama.rb に行ってモブプロをやって feature spec をリファクタリングした話 - YouTube
テストコードに関する僕の基本方針
僕はテストコード（特にフィーチャスペックやシステムスペック）を書くとき、こういったことを重視しています。
- 上から下に自然に読み下せること
- 特別な理由がない限り文字列や数値のベタ書きを用いること（変数を多用しないこと）
- ループ処理やif文など、プログラムチックなテストコードにしないこと
もっとざっくりいうと、「あたかも人間が読む文書のようなテストコード」が好みです。
この場合、DRYなテストコードにはなりませんが、アプリケーション側のコードとは違い、テストコードは多少DRYを捨てても構わないと考えています。
また、テストとして必ず担保したい仕様をカバーできる（＝テストコードがテストコードの目的を果たしている）のであれば、ラクをするために雑に書くことも許容することが多いです。
元記事のテストコードを見て感じたこと
上に書いた「僕の基本方針」に照らし合わせると、元記事のテストコードは以下のような点を修正したくなりました。
- subjectは複雑さが増す（文書らしさを損ねる）ので使わない
- 単純な文字列をletで定義するぐらいならベタ書きする
また、example（itやscenario）は本来であれば細かく分ける（1 exampleにつき、1エクスペクテーションとする）方が望ましいですが、フィーチャスペックにおいては実行速度の低下にもつながりやすいので、1つのexampleにまとめてしまっても良いと思います。
あと、context "with empty project description"として project_description が未入力だったケースをテストしていますが、これは「登録に成功するケース」の別パターンなので、わざわざテストケースを追加する必要はないと思いました。
もし「○○が未入力なら登録に成功する/失敗する」を細かく検証したいのであれば、モデルスペックにテストを追加するのが良いと思います。
僕が修正したテストコードはこちら
上記のような点をふまえて、僕が書き直したテストコードはこちらになります。
require 'rails_helper'
RSpec.describe "Projects", type: :system do
describe "project create" do
let(:user) { FactoryBot.create(:user) }
before do
sign_in_as(user)
end
context "with project name and description" do
scenario 'user creates a new project' do
click_link "New Project"
fill_in "Name", with: "tama.rb"
fill_in "Description", with: "omotesando"
expect {
click_button "Create Project"
}.to change(user.projects, :count).by(1)
expect(page).to have_content "Project was successfully created"
expect(page).to have_content "tama.rb"
end
end
context "with empty project name" do
scenario "user can't create a new project" do
click_link "New Project"
fill_in "Name", with: ""
fill_in "Description", with: "omotesando"
expect {
click_button "Create Project"
}.to_not change(user.projects, :count)
expect(page).to have_content "Name can't be blank"
end
end
end
end
まあ、ほとんど「リファクタリング前のコードに戻した」と説明した方が正しいかも知れませんね😅
ちなみにbeforeブロック内で使っている sign_in_as メソッドは、spec/support/login_support.rbに元から定義されていたものです。
module LoginSupport
def sign_in_as(user)
visit root_path
click_link "Sign in"
fill_in "Email", with: user.email
fill_in "Password", with: user.password
click_button "Log in"
end
end
RSpec.configure do |config|
config.include LoginSupport
end
細かいテストはモデルスペックに書く
あと、以下のようなモデルスペックも追加して「descriptionが未入力だった場合」も検証するようにしています。
require 'rails_helper'
RSpec.describe Project, type: :model do
# 省略
describe 'validation' do
let(:project) { FactoryBot.build(:project) }
context 'with empty name' do
it 'is invalid' do
project.name = ''
expect(project).to be_invalid
end
end
context 'with empty description' do
it 'is valid' do
project.description = ''
expect(project).to be_valid
end
end
end
end
コントローラやviewのロジックをテストするのでなければ、このようにモデルスペックでテストした方が、書きやすさの面でも実行速度の面でも有利です。
発展：さらに雑に書く
フィーチャスペックはDRYにしづらい＆実行速度が遅い、という傾向があるので、「検証エラーが発生する」というような基本的なテストケースは、以下のようにcontextを分けずに1つのexampleにまとめてしまうことが多いです（雑）。
require 'rails_helper'
RSpec.describe "Projects", type: :system do
describe "project create" do
let(:user) { FactoryBot.create(:user, first_name: 'Alice', last_name: 'Ito') }
scenario 'user creates a new project' do
sign_in_as(user)
click_link "New Project"
# 検証エラーが発生する場合
fill_in "Name", with: ""
fill_in "Description", with: "omotesando"
expect {
click_button "Create Project"
}.to_not change(user.projects, :count)
expect(page).to have_content "Name can't be blank"
# 正常な値を入力した場合
fill_in "Name", with: "tama.rb"
expect {
click_button "Create Project"
}.to change(user.projects, :count).by(1)
expect(page).to have_content "Project was successfully created"
expect(page).to have_content "tama.rb"
expect(page).to have_content "omotesando"
expect(page).to have_content "Owner: Alice Ito"
end
end
end
僕が必ずcontextを分けるのは、たとえば「一般ユーザーでアクセスした場合（は○○ボタンが表示されない）」と「管理者ユーザーでアクセスした場合（は○○ボタンが表示される）」のように、システムの振る舞いが根本的に変わるケースですね。
まとめ
というわけで、この記事ではEveryday Railsのテストコードを題材にして、フィーチャスペック（システムスペック）の書き方を議論してみました。
こうした書き方の違いは「良し・悪し（or 正しい・間違っている）」というよりも、「好き・嫌い」の世界に近いです。
いろいろな流派（？）があることを知って、自分の好みの書き方は何か、自分の開発チームではどういった書き方を推奨するか、といったテーマについて考えてみましょう。
あわせて読みたい
僕のテストコードの好みについては、過去にもいろいろと書いています。
RSpecの書き方については以下の記事の「Q. シンプルなスタイルと複雑なスタイル、選ぶならどっち？」も参考になると思います。
もちろん、「Everyday Rails - RSpecによるRailsテスト入門」のコーディングスタイルにあわせて書くのもおすすめです。