1回目に軸を分けてケースを減らす話を書きましたが、今回はどこまで書くかそのものです。
ケースは放っておくと増えます。増えた分だけ実行時間もレビュー負荷も増える。かといって減らしすぎると守れなくなる。どこで線を引くか、という話になります。
テストしやすいコードを書くのが原則
先に順序を確認しておきます。
テストが書きにくい時、まず疑うのは設計です。mockを重ねたり、内部状態を覗いたりして無理やり書くと、実装の詳細に密着したテストができあがります。リファクタのたびに落ちる。
テストの書き方を工夫する前に、テストしやすいコードになっているかを見る。以降の話も、だいたいそこに帰着します。
実装を見てケースを絞る
入力値を10通り試す必要があるかどうかは、実装を見れば分かります。分岐が3本しかないなら、各分岐を通す代表値3つで足ります。
これが一番効くケース削減です。1回目に書いた「独立している軸は組み合わせない」も、根っこは同じです。
早期returnが独立性を保証する
return head :unauthorized unless logged_in?
return head :forbidden unless authorized?
# 以下、正常系
認証や認可で止まるなら、その先の処理は一切走りません。だからパラメータが何であれ結果は同じです。
1回目に、「認証とパラメータは独立しているので組み合わせない」と書きました。あれは推測ではなく、このコードを見て確認できることです。早期returnが独立性をコードとして保証している。
逆に、途中にreturnがなく条件が後ろの処理に影響するなら、組み合わせが必要になります。
つまり実装の書き方がテストケース数を決めています。
ネストしたifを早期returnに直すのは可読性の話だと思われがちですが、ケース数にも効きます。「テストしやすいコードを書く」の具体例の1つです。
向きも大事で、正常系を流して異常系をreturnで弾きます。
# こうではなく
if valid?
# 正常系が全部ネストの中
else
return head :forbidden
end
# こう書く
return head :forbidden unless valid?
# 以下、正常系
正常系がネストの中に入ると、条件を抱えたまま読むことになります。条件が増えるほど差が開き、3つ弾くならネストは3段になりますが、早期returnなら3行並ぶだけです。
減らした根拠は、実装が変われば崩れる
ここには弱点があります。
分岐が3本だからケースを3つにした。その後、誰かが分岐を4本に増やしたら? ケースは3つのままです。 増やし忘れても、既存のspecは全部通ります。
「実装を見て減らす」は、実装と一緒にspecを直すことが前提になっています。片方だけ変えると穴が開く。
ここまで「リファクタでは落ちず、仕様変更では落ちる」を目標にしてきましたが、ここだけは仕様が変わっても落ちないケースが作れてしまいます。
完全な対処はないと思っています。減らさずに全パターン書けば穴は塞がりますが、それではケース爆発に戻る。
現実的にできるのは2つくらいです。
分岐を足したら、テストパターンのコメントも直す。 1回目に書いた宣言のコメントがあれば、「この分岐に対応するケースが無い」とレビューで気づける可能性が上がります。
カバレッジで拾う。 新しい分岐を通るケースが無ければ、その行は赤くなります。カバレッジは「何を検証したか」は見ていませんが、「通っていない行がある」ことだけは分かります。減らす判断をしている以上、カバレッジは見ておいた方がいい。
gemはブラックボックスで押さえる
ケースを減らせるのは、自分が実装を把握していて、変更も検知できる場合だけです。
gemは違います。分岐を知らないし、知ったつもりで減らすとバージョンアップで前提が崩れる。しかも崩れたことに気づけません。
なので外形で押さえます。「この入力でこの出力」を、内部構造を仮定せずに並べる。ケース数は増えますが、増えた分が安全を買っていることになります。
DeviseやKaminari、Ransackのように挙動がバージョンで変わるものが典型です。gem自体のテストはgemがやっているので、自分が守りたいのは自分の使い方が壊れていないこと。そこを押さえます。
線引きはシンプルで、自分が変更を検知できるかどうかです。自分のコードなら実装と同時にspecも直せる。gemは自分の意図と無関係に変わる。
責務で分ける
同じ検証を2箇所に書くと、実装が変わった時に2箇所落ちます。原因は1つなのに。itを分けた時と同じ問題が、ファイル間で起きている形です。
たとえばnameの必須チェックを、model specとrequest specの両方で書く。バリデーションのメッセージを変えると、両方直すことになります。
modelとrequestの分担
- model spec — その値が妥当か
- request spec — その値がどう扱われ、どう返るか
分かれていれば、バリデーションを1つ足した時に触るのはmodel specだけです。request specは接続の確認しかしていないので影響を受けません。
改修時に触る範囲が局所化される。目的から素直に降りてきます。
「全部requestでやれば?」への答え
理屈の上では成立します。request specだけで網羅すれば、バリデーションのバグは検出できる。間違いとは言えません。
ただ破綻します。
- そのバリデーションを使うendpointが3つあれば、同じケースを3回書く
- バリデーションを1つ足すと、3箇所に足す
- 1つ直すと、3箇所落ちる
共通のものを、使う側の数だけ複製している状態です。しかも複製なので、片方だけ更新されて食い違う。
肥大化というのは行数の話ではなく、変更がN箇所に伝播する状態のことだと思っています。
速度も当然効いてきます。同じケースでも、HTTPを通ると数倍から10倍程度の差が出ます。ただこれは副次的な効果で、本体は責務の分け方です。
request specでバリデーションを網羅しない
具体的にはこうなります。
- model spec — 各バリデーションを網羅する
- request spec — validが1つ、invalidが1つ
model側で網羅する理由は4回目に書いた通りで、そのケースがないとバリデーションが消えても気づけないからです。
invalidが1つでいいのは、エラーの載り方がどのバリデーションでも同じだからです。逆に、バリデーションによってレスポンス形式が変わるなら、その分だけ必要になります。
scopeへの移譲と、その穴
絞り込みや検索のロジックがscopeにあるなら、その検証はmodel specに移せます。
# model spec
describe '.by_public' do # 公開・非公開
describe '.by_join' do # 参加・未参加
describe '.by_active' do # 有効・削除予定
request specはHTTPを通るので、同じケース数でも数倍から10倍の差が出ます。移せるなら移した方がいい。
ただし穴があります。
model specは「scope単体が正しい」ことしか保証しません。 「パラメータがそのscopeに渡っている」ことは別です。
ここが抜けると、scopeを差し替えたり条件分岐を追加したりした時に、どちらのspecも落ちないまま壊れます。
移譲の原則
埋め方はシンプルで、request側で「繋がっていること」を確認します。全パターンではなく、絞り込みが効いていることだけ。0件と1件以上が区別できれば十分なことが多い。
一般化すると、
移譲先が正しいことは移譲先で、繋がっていることは移譲元で。
バリデーションでvalid/invalidを1つずつ残すのと、完全に同じ形です。バラバラのTipsに見えて、1つの規則の適用例になっています。
それでも残る不安
正直に書くと、ここは判断が割れます。
検索条件が動的に組み立てられる場合、パラメータの有無でscopeの組み合わせが変わります。「1ケースだけ通す」では組み合わせが漏れる。
1回目のサンプルで、絞り込みをrequest spec側で10ケース書いていたのは、この不安があるからです。model specに移せば速くなりますが、組み立て部分はrequestでしか見えない。
最適解を持っているとは言えません。移せるものは移す、組み立てが複雑なところは残す、くらいの判断でやっています。
まとめ
- テストが書きにくいのは設計の問題。書き方を工夫する前に実装を見る
- 実装の分岐を見てケースを絞る。早期returnが独立性を保証する
- 実装の書き方がケース数を決める。早期returnは可読性だけの話ではない
- 減らした根拠は実装が変われば崩れる。分岐を足した時にケースも足す必要がある
- その穴はテストパターンのコメントとカバレッジで拾う
- gemは実装を把握できないのでブラックボックスで押さえる。ケース数が増える分が安全を買っている
- 線引きは「自分が変更を検知できるか」
- modelは値の妥当性、requestは扱いと返し方。責務で分けると改修時に触る範囲が局所化する
- request specでバリデーションを網羅しない。validが1つ、invalidが1つ
- 移譲先が正しいことは移譲先で、繋がっていることは移譲元で
次回は、テストで守れない部分をどう塞ぐかを書きます。書き忘れても静かに通ってしまうものへの対処です。
