現実的に書くRspec

この記事は

Rspecらしい書き方とか、より良い書き方、みたいな記事はたくさんあります。
僕自身、それらをそこそこ理解しているつもりではあるし業務中ではRspecをずっと使っています。

しかし、Rspecらしく書くのはコストは高いしデメリットなところが多いと感じています。
ので、最近ではメリハリをつけてゆるふわに力を抜いて書いています。
この記事では、実際どう書いているのかを紹介したいと思います。

自己紹介

新卒で入った今の会社でずっとスマホのオンラインゲームのwebapi書いてます。
入社以来ずっとRSpec書いていて、minitestはチョット触ったことがある程度です。
開発のメンバーは、新卒の子がいたり業務委託の方がいたりして大体5人以下ぐらいの人数で書いています。
(クライアント側のエンジニアや他の職種の方も合わせるとそこそこの人数になります)

railsで開発はしていますが、
他の会社で働いたことないですし、ずっとゲーム開発をしていたので一般的なwebアプリの開発とは異なる部分が多いかもしれないです。

ゲームづくり固有の話

(他の会社のことを知らないので、今の会社での話)

• RESTではなくRPC
• 仕様が複雑
• 既存の機能に対して仕様追加・変更は日常茶飯事

例えば、装備アイテムを強化合成をするAPI

• ベースのアイテムのIDと素材となるアイテムのIDリストが引数
• 強化合成可能なアイテムか、素材にできるアイテムかとかをチェック
• 合計経験値を計算してベースのアイテムにプラス
• 経験値が一定以上になったらレベルをあげる
• 合成に必要なゴールドを計算して所持ゴールドからマイナス
• 素材となったアイテムは全部削除
• 変更のあった内容をレスポンスにまとめてCLに返す

というのが大雑把な流れですが、
そこに、

• 色んな経験値ボーナス
  • 同じ種類の装備アイテム、キャンペーン期間中、覚醒済みアイテムのときはボーナス、強化合成素材アイテムetc
• 戦闘力計算
  • 戦闘力ランキング更新
• トロフィーとかミッションとか呼ばれる機能
  • 強化合成○回達成・Lv○○到達・戦闘力○○到達etc
• KPIや問い合わせ用にログ出力

などなど。一つのAPIでやることはたくさんあります。

Rspecらしくを目指してガチガチに書くことのデメリットだと思っていること

上に書いたように、仕様が複雑で仕様追加・変更が日常茶飯事です。
また、新卒の子がいたり業務委託の方がいたりと能力もバラバラです。
その状況でspecをガチガチに書くと、

• describe, contextが超ネストしている
• トップレベルに大量のletが書かている
• 仕様追加・変更でどんどん秘伝のタレのようになっていく
  • 最初は綺麗にspecを書いたとしてもどんどん汚くなっていく
• specが落ちたときになんで落ちているか理解するのが難しい
• そのspecに必要な前提条件が何かわからない
• メインの実装よりもspecを書いている時間のほうが長い
  • なんなら、もとのspecを理解するために読んでる時間が一番長い
    • エスパー力が試される

といった感じでspecが負債になってしまっていることが多くありました。
みんなの能力が高くきれいなspecが書ける、常にリファクタリングする時間がある、というような理想的な状態だったら上記のようにはならないかもしれないですが、
現実は厳しいのです。
レビューやペアプロでどうにかできれば良いのかもしれないですが、
お互いに全てのPRを見るのことはできないし、チームに途中から入って既に焼け野原みたいな状況の可能性もあります。
ペアプロもずっとしてられるほど時間はないわけです。

これまでは、自己紹介と僕の置かれてる状況について書きました。
これ以降は、それらを踏まえて僕の最近のRspecの書き方や思っていることについて書いていきます。

何のために書くのか・大雑把な方針

会社やチームの方針的に、カバレッジ100%じゃなきゃダメ、とか、一切のバグを出してはいけない、とかそういうルール的なのはないです。
極論をいえば、一切仕様追加・変更がなく他の方法で挙動の検証が行われるのであれば、specはそもそも必要ないと思います。
なので、specを書くのは次の人のためだと思っています。
次の人が仕様追加・変更のときにリファクタリングをしやすいように、バグを起こさないように、
関係なさそうなところを変更したときにエンバグを出さないように、とか。

大雑把な方針としては、

• カバレッジは気にしない
  • リクエストスペックは手厚く。単体テスト系はあっさりと
• 全てのケースに関してテストは書かない
  • メインどころはもちろん書く
  • 今後の仕様追加・変更で壊れそうなところとかを意識して書く
    • 細かい例外処理のケースとかは一回書いてしまえば変更すること無いのでテストなくても別に問題ない
• かっこよく書こうとせずに次の人がわかりやすいspecを書く
  • わかりやすく書かれていればRspecらしく書かれているかどうかは些細な問題

つまり、
Rspecらしさとかバグが出ないこと、とかを少し犠牲にして、
specのわかりやすさ読みやすさを重視し、メンテナンスコストを下げる方向に倒しています。

以下は、Rpsecらしさを捨ててわかりやすさを重視しつつゆるふわに書いている具体的な内容です。

describe, contextのネストを深くしない

switchやif文をネストしているのと変わらないので、describe, contextのネストが深いのは良くないと思っています。
どうしても深くなりそうなときは、べたっとexampleに書いてしまった方が見通しは良くなると思っています。

例としてFizzBuzzのspecを書いてみます。
(普通にFizzBuzzのspecを書いてもそこまでネストは深くならないのでこう書く必要はないですが)

describe 'fizzbuzz' do
  it '3でも5でも割り切れるときFizzBuzz' do
    expect(fizzbuzz(15)).to eq 'FizzBuzz'
  end

  it '3で割り切れるときFizz' do
    expect(fizzbuzz(3)).to eq 'Fizz'
  end

  it '5で割り切れるときBazz' do
    expect(fizzbuzz(5)).to eq 'Bazz'
  end

  it 'それ以外は数字をそのまま' do
    expect(fizzbuzz(4)).to eq '4'
  end
end

こんな感じ。
Rspecらしさは完全に無視してますし、実際ここまで極端に書くことはないですが、
これ自体はすごくわかりやすいと思います。FizzBuzzの仕様がすっと理解できると思います。

例えば、3でも5でも15でも割り切れるときはHogeという仕様が入ったときでもサクッと直すことができます。
describe, contextネストしてる場合だと、なかなか難しいはずです。

そもそも

そもそもdescribe, contextをネストが深くなりがちなときはだいたい次のどちらかです。

• テスト対象の実装が複雑すぎる
• テストを書きすぎている

リファクタリングをするとか、そもそも本当に必要なテストなのかを検討する良い目安です。

shared_contextよりもメソッドとかに切り出す

shared_context 'user has enough gold'なら良いですが、
shared_context 'user played some quest', user: user, quest: questは使いづらいです。
def play_quest(user:, quest:)のほうが断然良いです。

describe 'hoge' do
  include_context 'user has enough gold'
  before { travel_to(1.day.ago) { play_quest(user: user, quest: quest) } }
  subject { play_quest(user: user, quest: quest) }
end

みたいな感じ。
メソッドであればbefore, subject, let、メソッドなどどこからでも呼べます。
shared_contextだとそれができないので、せっかく共通化したのに誰も使わない、ということが多いです。

shared_context自体がダメというわけではないです。
include_context 'user has enough gold'みたいのであればメッセージがつけれるので十分わかりやすいので良いと思います。
before { user.update(gold: 999_999) }よりも意図が伝わるはずです。
shared_contextは用法用量を守って使えば です。

beforeは分厚く書かない/letをたくさん用意しない

specを書くときに、
そのspecにとって必要な条件とspecを動かすために必要な条件、
の2種類があると思います。(うまく言葉にできていないのでいい感じに理解していただけると)

例えば、
強化合成でベースのアイテムと素材のアイテムが同じ職業のアイテムの場合ボーナス経験値がある、
というようなspecを想定してみます。(細かいケースは無視しています)

# user has_many items
# item belongs_to master_item です

let(:user) { FactoryGirl.create(:user) }
let(:job) { :knight }
let(:base_master_item) { FactoryGirl.create(:master_item, job: job, kind: :sord) }
let(:base_item) { FactoryGirl.create(:item, user: user, master_item: base_master_item) }
let(:material_master_item) { FactoryGirl.create(:master_item, job: job, kind: :sord) }
let(:material_item) { FactoryGirl.create(:item, user: user, master_item: material_master_item) }
subject { user.enhance_item(base_item: base_item, material_items: [material_item]) }

このspecにとって必要な条件は、
強化合成でベースのアイテムと素材のアイテムが同じ職業のアイテムの場合に関する部分だけです。
このspecを動かすために必要な条件として書かれているのは、

• 一時変数としての base_master_item, material_master_item
• kind: :sord
  • ベースや素材にできないアイテム(売却専用アイテムetc)があるので、剣をとりあえず指定している

の2点です。
リファクタリングすると

let(:user) { FactoryGirl.create(:user) }
let(:job) { MasterItem.jobs.values.sample }
let(:base_item) do
  master_item = FactoryGirl.create(:master_item, :for_base, job: job)
  FactoryGirl.create(:item, user: user, master_item: master_item)
end
let(:material_item) do
  material_item = FactoryGirl.create(:master_item, :for_material, job: job)
  FactoryGirl.create(:item, user: user, master_item: material_item)
end
subject { user.enhance_item(base_item: base_item, material_items: [material_item]) }

こんなかんじ。
ポイントは2つ。

• base_master_item, material_master_itemのletが消えたこと
• kind: :sordをtraitで置き換えたこと

どちらもこのspecを動かすために必要な条件なだけだったので、少し奥にしまい込むように直しました。
letはこのspecにとって必要なものだけになっています。
(base_item, material_itemの中身がゴチャっとしてしまっているので、もう少しなんとかしても良いかもしれません)
(jobはついでにランダムに直しました。)

beforeを書くときも同じで、メソッドに追い出すとかtraitやfactory用意するとかして、
そのspecに必要な条件とspecを動かすために必要な条件を見極められる様に書きます。

だからといってDRYにはしすぎない

よく言われるやつです。
テストコードでDRY意識しすぎて書いてしまうと、書いているときは満足感高いですが、
理解しづらくメンテコストが高いことが多いです。

マジックナンバーを書かない

specにはマジックナンバーが書かれることが多い気がしています。

let(:base_item) { create :item }
let(:material_item) { create :item }
subject { user.enhance_item(base: base_item, material_items: [material_item]) }
specify do
  expect { subject }.to change { item.exp }.to(256) 
end

こんな感じ。256のマジックナンバーさすごいですね。
直すとしたらこう。

let(:base_item) { create :item, exp: 100 }
let(:material_item) { create :item, exp: 100 }
subject { user.enhance_item(base: base_item, material_items: [material_item]) }
specify do
  expect { subject }.to change { item.exp }.to(256) # 100 + 100 * 職業ボーナス * 部位ボーナス
end

256のマジックナンバーを消す方法は他にもたくさんあると思いますが、
とりあえずコメントで補足しちゃうのがコスパが一番良いと思います。
(ついでに、itemのfactoryの初期値に依存してしまうのは良くないのでexp: 100を追加してあります。)

ちなみに

expect { subject }.to change { item.exp }.to(base_item.exp + material_item.exp * SYOKUGYO_BONUS * BUI_BONUS)

みたいな直しはしないです。(たまにすることもありますが。。)
参考 => テストコードの期待値はDRYを捨ててベタ書きする ～テストコードの重要な役割とは？～

もう一つのマジックナンバーの別の例を。

describe '#consume_gold' do
  let(:user) { FactoryGirl.create :user }
  subject { user.consume_gold(10) }
  specify { expect { subject }.to change { user.gold }.by(-10) }
end

このspecはスコープがすごく短いので別に問題ないのですが、
あえていちゃもんを付けるとすると、
10である意味は？999999でも通る？あたり。
あえて直すとすると

describe '#consume_gold' do
  let(:user) { FactoryGirl.create :user, gold: 999 }
  let(:consumption_gold) { rand(1..user.gold) }
  subject { user.consume_gold(consumption_gold) }
  specify { expect { subject }.to change { user.gold }.by(-consumption_gold) }
end

こんな感じ。こう直したことで、

• factoryの初期値に依存していたのが消えた
• userの所持金以下なら消費できる、ということをテストしていることが分かる

というメリットがあります。
(この例に関しては少しやりすぎだと思いますし、直す前のほうが見やすいです)

ガシガシ日本語で書いてしまう

コード中に日本語書いてあるとダサさがあったりしますが、
よくわからない英語でdescribe等のメッセージを書くぐらいだったら日本語で確実に伝えたほうが100倍マシです。
でも、最近はgoogle翻訳が超絶便利になったのでだいぶお世話になっています。

そもそも

そもそも複雑なメッセージを書きたいということは、
どこかに複雑さがある証拠なのでリファクタリングを検討するいい目安です。

最後に

長々と書きましたが、
このような感じで
次の人が見てわかりやすいように、気をつけるところはちゃんと気をつけつつ、ゆるふわに書いても良いところはゆるふわに力を抜いて書いています。
ただ、基本的にはRspecらしく綺麗に書こうとして、それだとわかりづらい・負債になりそう、というときに
ゆるふわ基準でspecを書いています。

この記事を通して、自分が書くコードを含めて、
理解するのにエスパー力を必要とするようなspecが世の中から少しでも減ることを願っています。

(もしかしたら、半年・一年後とかには全く違う書き方をしているかもしれないですが。。)

蛇足

去年書いた記事も読んでいただけると幸いです
リーダブルRspec