概要
原著者の許諾を得て翻訳・公開いたします。
- 英語記事: Refactoring tests for better application design
- 原文公開日: 2014/03/27
- 著者: Noah Davis
- サイト: https://codeclimate.com/blog/
Rails: テストのリファクタリングでアプリ設計を改良する（翻訳）
Code Climate編集者メモ: 今回はゲストとしてMarko Anastasovの記事もご紹介します。Markoは開発者であると同時に、CI/デプロイサービスで知られるSemaphoreの共同設立者であり、Code ClimateのCIパートナーでもあります。
単体テストを書くという行為は、検証よりも設計という行為に近い -- Bob Martin
テスト駆動開発（TDD）はテストのためのものであるという思い違いを未だによく見かけます。TDDを遵守することで開発が迷走する可能性を最小限に抑えることができ、最初にテストを書くことを義務付けることでテストの書き忘れも最小限に留められます。いつもの私は、超人であり続けなければとてもなしえないようなソリューションではなく、普通の人間のために設計されたソリューションを選びますが、ここでは少し違います。TDDは自動化テストを一種の乗り物のように用いて、私たちがコードを書く前にコードのことをいやでも考えざるを得ないように設計されています。なおこの方法は、特定の機能に接続されるすべてのコードが期待どおり動作していることを確認するのにデバッガを起動するよりもずっとよい方法です。TDDの目的はソフトウェア設計の改良であり、テストコードはその副産物のひとつです。
テストを必ず最初に書くことで、テストされるオブジェクトのインターフェイスについてじっくり考えるようになります。必要だがまだ存在しないオブジェクトについても同様です。作業は制御可能な小さな範囲で少しずつ進められます。テストが初めてパスしてもそこで作業は終わりではありません。再び実装に立ち戻ってコードをリファクタリングし、コードを美しく保ちます。コードが正しく動作していることを担保するテストスイートが存在するおかげで、自信を持ってコードを変更できます。
TDDの経験者なら誰でも、コードの設計力を問われ、そして磨かれることに気づくようになります。開発しながら常に「むー、このコードはprivateのままではまずそうだな」とか「このクラスの責務が増えすぎてしまった」という風に考えるようになるのです。
テスト駆動リファクタリング
あるコードのテストをどう書けばよいかわからなくなってくると、「red-green-refactor」というサイクルが止まってしまうこともあるでしょうし、たとえ書けたとしてもかなりつらい作業に思えることでしょう。テストを書くのがつらい部分は、しばしばコードの設計に問題があることを示します。あるいは、その部分のコードがTDDアプローチに沿って書かれていなかっただけかもしれませんが。テストコードの「匂い」は多くの場合アンチパターンと呼ぶのがふさわしく、テストとアプリコードの両方についてリファクタリングする機会であることを示します。
例として、Railsのcontroller specでの複雑なテストセットアップを見てみましょう。
describe VenuesController do
let(:leaderboard) { mock_model(Leaderboard) }
let(:leaderboard_decorator) { double(LeaderboardDecorator) }
let(:venue) { mock_model(Venue) }
describe "GET show" do
before do
Venue.stub_chain(:enabled, :find) { venue }
venue.stub(:last_leaderboard) { leaderboard }
LeaderboardDecorator.stub(:new) { leaderboard_decorator }
end
it "venueをidで検索して@venueに代入する" do
get :show, :id => 1
assigns[:venue].should eql(venue)
end
it "@leaderboardを初期化する" do
get :show, :id => 1
assigns[:leaderboard].should == leaderboard_decorator
end
context "userはpatronとしてログインしている" do
include_context "patronがログインしている"
context "patronはトップ10にいない" do
before do
leaderboard_decorator.stub(:include?).and_return(false)
end
it "leaderboardからpatronのstatsを取得" do
patron_stats = double
leaderboard_decorator.should_receive(:patron_stats).and_return(patron_stats)
get :show, :id => 1
assigns[:patron_stats].should eql(patron_stats)
end
end
end
# 簡単のため以後のテストケースは省略
end
end
このコントローラのアクションは、技術的にはさほど長くありません。
class VenuesController < ApplicationController
def show
begin
@venue = Venue.enabled.find(params[:id])
@leaderboard = LeaderboardDecorator.new(@venue.last_leaderboard)
if logged_in? and is_patron? and @leaderboard.present? and not @leaderboard.include?(@current_user)
@patron_stats = @leaderboard.patron_stats(@current_user)
end
end
end
end
ここでお気づきいただきたいのは、specセットアップのコードが長いと、たとえばVenue.enabled.findが呼び出されるというexpectationや、LeaderboardDecorator.newに正しい引数が渡されるというexpectationを開発者が書き忘れてしまいがちであるという点です。代入された@leaderboardの元は代入されたvenueであるかどうかがまったく明確になっていません。
MVCパラダイムに囚われてしまった開発者は（私も含めてですが）、ついコントローラにビジネスロジックを長々と書き連ねてしまい、よいspecを書くこともコードやspecのメンテも困難になってしまいます。この困難は、Railsのコントローラのたった1行のメソッドですら多くのことを行っていることが原因です。
def show
@venue = Venue.find(params[:id])
end
上のメソッドはこれだけの作業を行っています。
- パラメータを取り出す
- アプリ固有のメソッドを呼び出す
- ビューテンプレートで用いられる変数へ代入する
- レスポンステンプレートのレンダリング
データベース内部やビジネスルールの奥深い部分に到達するコードを書き足すと、コントローラのメソッドがカオスになるだけです。
上のコントローラには、4つの条件を持つif文が隠れています。完全なspecでは、これをカバーするためだけに15とおりの組み合わせを記述しなければなりませんが、もちろんそのようなものは書かれていません。しかし、コードがコントローラの外に置かれる場合は事情が変わってきます。
改良版のcontroller specが次のようになっているとしましょう。外部から受け付けるリクエストを処理してレスポンスを準備するという作業を実行するためにはどのようなインターフェイスが望ましいでしょうか。
describe VenuesController do
let(:venue) { mock_model(Venue) }
describe "GET show" do
before do
Venue.stub(:find_enabled) { venue }
venue.stub(:last_leaderboard)
end
it "有効なvenueをidで検索する" do
Venue.should_receive(:find_enabled).with(1)
get :show, :id => 1
end
it "見つかった@venueを代入する" do
get :show, :id => 1
assigns[:venue].should eql(venue)
end
it "venueのleaderboardをデコレーションする" do
leaderboard = double
venue.stub(:last_leaderboard) { leaderboard }
LeaderboardDecorator.should_receive(:new).with(leaderboard)
get :show, :id => 1
end
it "@leaderboardを代入する" do
decorated_leaderboard = double
LeaderboardDecorator.stub(:new) { decorated_leaderboard }
get :show, :id => 1
assigns[:leaderboard].should eql(decorated_leaderboard)
end
end
end
他のコードはどこに行ってしまったのでしょうか？ここではモデルを拡張して検索ロジックを単純化しています。
describe Venue do
describe ".find_enabled" do
before do
@enabled_venue = create(:venue, :enabled => true)
create(:venue, :enabled => true)
create(:venue, :enabled => false)
end
it "有効なスコープ内で検索する" do
Venue.find_enabled(@enabled_venue.id).should eql(@enabled_venue)
end
end
end
さまざまなif文は次のように単純化できます。
if logged_in?: 結果の違いはビューテンプレートで決定できるif @leaderboard.present?: （古いコード）falseの場合の動作はビューで決定できる- その他のコードはdecoratorクラスに移動して新しいメソッドで詳しく記述できる
describe LeaderboardDecorator do
describe "#includes_patron?" do
context "userがpatronではない" { }
context "userがpatronである" do
context "userがリストにいる" { }
context "ユーザーがリストにいない" { }
end
end
end
この新しいメソッドは、@leaderboard.patron_statsをレンダリングするかどうかをビューで決定できるようにします。この部分の変更は不要です。
# app/views/venues/show.html.erb
<%= render "venues/show/leaderboard" if @leaderboard.present? %>
# app/views/venues/show/_leaderboard.html.erb
<% if @leaderboard.includes_patron?(@current_user) -%>
<%= render "venues/show/patron_stats" %>
<% end -%>
これで、コントローラのメソッドがかなりシンプルになりました。
def show
@venue = Venue.find_enabled(params[:id])
@leaderboard = LeaderboardDecorator.new(@venue.last_leaderboard)
end
このコードを次回使うときには、LeaderboardDecoratorに与える正しい引数とは何かをコントローラ側で把握する必要がある点がちょっと残念かもしれません。venue用の新しいdecoratorを1つ導入して、デコレーションされたleaderboardを返すようにしてもよいでしょう。この部分の実装は読者の練習用に残しておきます ;)
最後に
もっと詳しくお知りになりたい方は、SemaphoreブログでMarkoのRailsアプリのテスティングアンチパターン記事をご覧ください。