概要
原著者の許諾を得て翻訳・公開いたします。
- 英語記事: RubyOnRails testing antipatterns — part 2/2 – selleo – Medium
- 原文公開日: 2018/01/03
- 著者: Błażej Kosmowski
Rails: テスティングアンチパターン --後編（翻訳）
part 1では、スタブ化に関連するアンチパターンや、テストしやすくするためだけに度を越して複雑になってしまった実装の詳細の問題について扱いました。後半は、テストそのものについてもう少し詳しく扱うことにします。
アンチパターン07: フレームワークやライブラリのメソッドを使いすぎる
フレームワークやライブラリのメソッドをふんだんに使ってspecを書くと、ライブラリメソッドをスタブ化してしまったときと似たような問題を引き起こします。レコードを1件検索するときにORMの機能をたまに使うぐらいなら大丈夫かもしれませんが、独自のspecを書くときに使うメソッドが複雑になればなるほど、specが変更を嫌がるようになります。プロジェクトが依存しているライブラリを更新すると、実際の実装は問題がなくてもテストが動かなくなってしまう可能性があります。また、複雑なメソッドを使っていると、テストの意図がわかりにくくなってしまい、他の開発者が意図の理解に苦しむ可能性もあります。
上述したとおり、フレームワークやライブラリのシンプルなメソッドを使う分には害はありませんが、普段は言語の機能や標準ライブラリの機能だけを、最もシンプルな方法で使うべきです。テストをやりやすくするするライブラリを使う必要が生じたときは、ライブラリをヘルパーメソッドやマッチャーにラップして、テストコードを読みやすくわかりやすいものにするとよいでしょう。
（Gist）
# 残念なspec
# ==========
# サードパーティのライブラリやフレームワークのメソッドがテストで直接使われている
describe ExportActiveOrders do
describe '.call' do
it 'アクティブな注文をすべてエクスポートしてそのファイルへのパスを返す' do
create_list(:order, 2, :active)
create(:order, :inactive)
path = ExportActiveOrders.call
expected_columns = Order.where(active: true).pluck(:customer_name, :total_value)
book = Spreadsheet.open(path)
sheet = book.worksheet(0)
expect(sheet.rows).to match_array(expected_columns)
end
end
end
# よいspec
# ============
# サードパーティライブラリの利用はヘルパーとして抽象化されている
# 期待する値の取得にフレームワークメソッドを使わず、明示的に値を渡している
module Support
module XlsHelpers
def open_spreadsheet(path)
book = Spreadsheet.open(path)
sheet = book.worksheet(0)
sheet.rows
end
end
end
RSpec.configure do |config|
config.include Support::XlsHelpers
end
describe ExportActiveOrders do
describe '.call' do
it 'アクティブな注文をすべてエクスポートしてそのファイルへのパスを返す' do
create(:order, :active, customer_name: 'Bruce Wayne', total_value: 1000)
create(:order, :inactive, customer_name: 'Dr. Strange', total_value: 1200)
create(:order, :active, customer_name: 'Edward Nygma', total_value: 500)
path = ExportActiveOrders.call
rows = open_spreadsheet(path)
expect(rows).to match_array([['Bruce Wayne', 1000],['Edward Nygma', 500]])
end
end
end
アンチパターン08: feature testを使いすぎる
feature test（機能テスト）はシステムテストや結合テスト（integration test）とも呼ばれ、ビヘイビア駆動開発（BDD）アプローチで開発をすすめるうえで非常に強力なツールです。しかしfeature testはすぐ「やりすぎ」になってしまいがちです。特に副作用や例外をすべて網羅するテストにfeature testを使ったときにそうなります。feature testは必要なテストコードの量が増える（セットアップとアサーションの両方が必要）だけでなく、単体テスト（unit test）よりもずっと低速です。しかもテストの粒度が低い（荒っぽい）のでバグの原因を突き止めるのが難しくなる可能性もあります。
feature testを使いすぎないようにするには、feature testを書く一番大きな理由を理解する必要があります。BDDの場合、開発をさらに進める出発点を定めることが最も大きな理由です。もうひとつの理由は、（BDDに従うかどうかとは無関係に）feature testを高レベルの安全ネットとして使い、アプリのhappy pathが正常に動作していることを担保するためです。feature testをこの2つの目的のためだけに用い、その他のケースについては低レベルの（単体）テストを書くようにすれば、両者のバランスをうまく保てるようになるはずです。
（Gist）
# feature specがバリデーションや認証の詳細をテストするのに使われている
# ここを高速にすべきであれば、specのレベルを下げること（model specやcontroller specなど）
RSpec.describe '資産管理', type: :feature do
scenario 'Adminが資産を作成する'
scenario 'Adminが名前を思い出せない資産を作成しようとする'
scenario 'Adminが短すぎる名前の資産を作成しようとする'
scenario 'Adminが名前の重複した資産を作成しようとする'
scenario 'Guestが資産を作成しようとする'
scenario '常連ユーザーが資産を作成しようとする'
end
アンチパターン09: テストの最適化を頑張りすぎる
初めのうちは、テストコードの重複を完全に消し去ってコードが可能な限り短くなるまでテストスイートのリファクタリングを繰り返したくなるものです。しかし長い目で見ると、その努力が裏目に出る可能性があります。ループや複雑なヘルパーやshared exampleやcontextは、テストの実装をスリムにする方法のほんの一端に過ぎません。テストを最適化する方が更新時の変更量が少なくなるのでメンテしやすくなるという意見もありますが、私はメンテナンス性というものはコード量の少なさだけで決まるものではないと考えています。私はメンテナンス性について、変更にどれだけの努力を要するかを重視します。変更のための努力とは、どこを変更すればよいかを理解してそれを見つけ出し、思いがけない副作用が発生せず安全に変更できることを確かめた上で実際に変更するまでの作業を指します。全知全能を注ぎ込んでテストを緻密に最適化すればするほど、実際の変更作業の時間が短くなる代わりに、その準備に必要な時間の方が増えてしまう可能性があります。読みやすさを過剰に追求したテストもまた、高度に最適化されたテストの一種です。このトピックについて詳しくは私の別記事「An opinionated guide to readable rspec」でもご覧いただけますが、ここまで極端に走らなくても、テストのコメントを少し余分に書いておくとか、少々のコード重複に目をつぶるだけでかなりの成果が得られます。
アンチパターン10: テストを書いている途中でつい実装を進めてしまう
これは非常に強烈な誘惑です。シンプルなリスト表示をTDDで実装することになったとしましょう。「テストをgreenにしたいからDBからレコードをフェッチするか。おっとやべぇ！データを名前順でソートしてなかった。忘れないうちにorder(name: :asc)をここに足しておくべ」とこんな具合に誘惑に身を任せていると、必要以上に実装されたコードがテストされないまま放置されてしまうことがあります。さらに困ったことに、コードカバレッジツールはこの種の未テストコードをほとんど検出できません。後でやり方を改めない限り、漏れているテストの追加が必要かどうか気づくことすらできなくなります。
この状況に対処するには、失敗するテストを必ず最初に書いてから実装を進める必要があります。あるコード片を削除してもテストスイートがred（失敗）にならないときは、常に「漏れているテストや足りないコードを書くべし」という危険信号として受け止めるべきです。何らかの理由でコードのテストが不要なこともあるかもしれませんが、この点を常に肝に銘じて、気まぐれに方法を変えないようにすべきです。
（Gist）
class BookingsController < ApplicationController
def index
# 雑に追加した`order`がテストされずに放置される可能性がある
bookings = Booking.order(customer_name: :asc)
render :index, locals: { bookings: bookings }
end
end
（Gist）
RSpec.describe BookingsController do
describe '#index' do
it 'すべての予約を表示' do
bookings = create(:booking, 5)
allow(controller).to receive(:render).and_call_original
get :index
expect(controller).to have_received(:render).
with(:index, locals: { bookings: match_array(bookings) })
end
end
end
アンチパターン11: 無関係なデータを混ぜる
テストのセットアップで無関係なデータを含めると、たいていテストが読みにくくなります。実際のセットアップ量も必要以上に増えてしまいますし、実際にはありえないデータとアサーションを結びつけて考えるという努力を開発者に強いるため、混乱の元にもなります。
普通はテストのセットアップを準備するときに無関係なデータを含めないように心がけていれば十分なのですが、オブジェクトやオブジェクトの集合が有効であるために無関係なデータが必要になってしまうことがあります。そうした目的にはファクトリーを使うべきでしょう。これによって（属性を明示的に与えなくても）オブジェクトがデフォルトで有効になりますし、無関係なデータをファクトリーに隠すこともできます。ファクトリーで作成するときには常に最終的に有効なオブジェクトを得られるようにしておけば、ファクトリーのlintをオンにできるようになります。
まとめ
ご紹介した問題の多くは、BDDアプローチに沿ってテストしていればめったに発生しませんので、BDDに沿うことを強くおすすめします。最後に、どんな規則にも例外はありますが、知っておくべきアンチパターンをいくつか頭の隅に置いておくだけでも、長い目で見たときにテストスイートの品質が大きく向上することでしょう。