はじめに
みなさん、DRY原則はご存知でしょうか?
DRY = Don't repeat yourselfの略で「繰り返しを避けること」という意味ですよね。
良いコードを書くための重要かつ基本的な原則なので、みなさんよくご存知だと思います。
ですが、DRY原則はテストコードを書く場合は必ずしも最善にはならない場合があります。
他の人が書いたテストコードを見ていると、テストコードにDRY原則を適用したために、かえって悪いコードになっているケースをときどき見かけます。
この記事ではなぜテストコードをDRYにすると良くないのか、ということを説明します。
追記:タイトルを変更しました
@t_wada さんのコメントを受けて、タイトルを見直しました。
「テストコードはDRYを捨ててベタ書きする」
=> 「テストコードの期待値はDRYを捨ててベタ書きする」
【注意】この記事は画一的なテストコードの書き方を勧めているわけではありません!
記事タイトルには少し強い言葉を使っていますが、僕は決して「いつでもベタ書きしろ」と言いたいわけではありません。
当然トレードオフを検討する余地はあります。
記事の後半ではそういった内容も説明しているので、記事タイトルだけで判断せず、本文を最後まで読んでもらえると幸いです。
テストコードの重要な役割
まず、テストコードの役割について認識をあわせておきましょう。
テストコードの重要な役割には以下の2つがあります。
- 対象のコードの仕様がひと目でわかること(仕様書になっていること)
- 確実にバグを検出できること
テストコードにおいては、この役割とDRY原則は両立しにくいケースが多いです。
具体例を以下に示します。
例1:半額の値段を計算するメソッドのテストを書く
たとえば以下のようなCloth(服)クラスがあったとします。
class Cloth
attr_reader :name, :price
def initialize(name, price)
@name = name
@price = price
end
def half_price
price / 2
end
end
half_price
は半額の値段を計算するメソッドです。
Aさんはこのメソッドをテストするために、次のようなテストコードを書きました。
describe Cloth do
describe '#half_price' do
it '半額の値段を計算する' do
cloth = Cloth.new('RSpec Tシャツ', 1000)
expect(cloth.half_price).to eq cloth.price / 2
end
end
end
どうでしょう?値段の1000円は1回しか出てきてないので、ここはDRYですね。
テストパターンが増えても変更する箇所は値段の部分だけで済みます。
describe Cloth do
describe '#half_price' do
it '半額の値段を計算する' do
cloth = Cloth.new('RSpec Tシャツ', 1000)
expect(cloth.half_price).to eq cloth.price / 2
cloth = Cloth.new('RSpec Tシャツ', 2000)
expect(cloth.half_price).to eq cloth.price / 2
end
end
end
ここまでは問題ないように見えますが、3つ目のテストケースを増やしたらどうでしょう?
2で割ったら端数が出るケース(999円の場合)です。
describe Cloth do
describe '#half_price' do
it '半額の値段を計算する' do
cloth = Cloth.new('RSpec Tシャツ', 1000)
expect(cloth.half_price).to eq cloth.price / 2
cloth = Cloth.new('RSpec Tシャツ', 2000)
expect(cloth.half_price).to eq cloth.price / 2
# 端数が出る場合の半額はいくら??
cloth = Cloth.new('RSpec Tシャツ', 999)
expect(cloth.half_price).to eq cloth.price / 2
end
end
end
あなたは999円の服の半額がいくらになるか、ぱっと理解できますか??
Rubyのエキスパートであれば瞬時にわかるかもしれませんが、少なくとも僕は確信をもって「いくら」と答えることができません。
この時点でテストコードの重要な役割の一つである「対象のコードの仕様がひと目でわかること(仕様書になっていること)」を満たせなくなります。
改善策:期待値はベタ書きする
というわけで、こういったケースは半額の値段もベタ書きしてしまう方が望ましいです。
describe Cloth do
describe '#half_price' do
it '半額の値段を計算する' do
cloth = Cloth.new('RSpec Tシャツ', 1000)
expect(cloth.half_price).to eq 500
cloth = Cloth.new('RSpec Tシャツ', 2000)
expect(cloth.half_price).to eq 1000
cloth = Cloth.new('RSpec Tシャツ', 999)
expect(cloth.half_price).to eq 499
end
end
end
こうすれば999円のように端数が出る場合の計算結果もひと目でわかるようになります。
例2:実装コードのバグに気づく?気づかない?
テストコードと検証ロジックが実装コードに依存していると、実装コードのバグをテストコード側で検出できない恐れがあります。
非常にいじわるな例ですが、price
が元の値段のマイナスの値を返すようにしてみます。
class Cloth
attr_reader :name
def initialize(name, price)
@name = name
@price = price
end
# わざとマイナスの値を返す
def price
@price * -1
end
def half_price
price / 2
end
end
先ほど示した、DRYなテストコードの例ではこの変化に気づくことはできません(つまりテストはパスします)。
なぜなら、cloth.half_price
の結果も、cloth.price / 2
の結果も、両方-500や-1000になるからです。
describe Cloth do
describe '#half_price' do
it '半額の値段を計算する' do
cloth = Cloth.new('RSpec Tシャツ', 1000)
expect(cloth.half_price).to eq cloth.price / 2
cloth = Cloth.new('RSpec Tシャツ', 2000)
expect(cloth.half_price).to eq cloth.price / 2
cloth = Cloth.new('RSpec Tシャツ', 999)
expect(cloth.half_price).to eq cloth.price / 2
end
end
end
# 実行結果
1 example, 0 failures, 1 passed
Finished in 0.00271 seconds
Process finished with exit code 0
しかし、期待値をベタ書きしている場合はテストが失敗するので、実装コードの変化に気づくことができます。
describe Cloth do
describe '#half_price' do
it '半額の値段を計算する' do
cloth = Cloth.new('RSpec Tシャツ', 1000)
expect(cloth.half_price).to eq 500
cloth = Cloth.new('RSpec Tシャツ', 2000)
expect(cloth.half_price).to eq 1000
cloth = Cloth.new('RSpec Tシャツ', 999)
expect(cloth.half_price).to eq 499
end
end
end
# 実行結果
expected: 500
got: -500
(compared using ==)
./spec/cloth_spec.rb:5:in `block (3 levels) in <top (required)>'
-e:1:in `load'
-e:1:in `<main>'
1 example, 1 failure, 0 passed
Finished in 0.016279 seconds
Process finished with exit code 1
price
が突然マイナスになるというのは極端な例ですが、複雑な実装コードであればあるほど、思いがけないバグが混入しやすくなります。
なので、実装側のロジックは極力テストコードに持ち込まないようにする方が望ましいです。
ベタ書きには壊れやすくなるというデメリットもある
ところで、上の例ではprice
がマイナスの値を返すことを「バグ」と定義しました。
それにゆえに、DRYなテストコードは「バグを検出できなかった」と見なされました。
しかし、マイナスの値を返すことが「仕様変更」だとしたらどうでしょうか?
DRYなテストコードは「テストコードを修正せずに済んだ」となりますし、ベタ書きしたテストコードは「テストが壊れてしまい、テストコードを修正する必要が出てきた」と見なされます。
そうです、ベタ書きするテストコードは仕様変更の際に壊れやすくなるのです。
ベタ書きとDRYは、(当然ながら)正反対のメリットとデメリットを持っています。
とはいえ、壊れやすさと、仕様の理解のしやすさ・バグの検出のしやすさを天秤にかけると、後者の方がテストコードにとって重要です。
状況によっては壊れやすさがメリットになるケースもある
テストコードが壊れることは必ずしも悪いことではありません。
思いがけないテストコードが壊れると、自分では気づいていなかった仕様変更の影響範囲に気づく、という効果も見込めます。
仕様変更によってテストコードが壊れ、そのためにテストコードを修正するのは面倒ですし、時間がかかる作業になります。
ですが、それはある程度やむを得ないものとして、「テストコードの役割」を受け入れてやりましょう。
条件分岐やループ処理が大量に出てくるテストコードも「臭うテストコード」
「対象のコードの仕様がひと目でわかること」「確実にバグを検出できること」という観点からすると、条件分岐やループ処理が大量に出てくるテストコードもたいていの場合、悪いテストコードです。
たとえば以下はPersonの生年月日を和暦で表示するformat_date_of_birth
メソッドの 悪いテストコード例 です。
describe Person do
describe '#format_date_of_birth' do
it '生年月日を和暦で表示すること' do
%w(1977-06-06 1988-06-06 1989-06-06 2016-06-06).each do |date_text|
date = Date.parse(date_text)
person = Person.new('Taro', date)
jp_year = date.year >= 1989 ? "平成#{date.year - 1988}年" : "昭和#{date.year - 1925}年"
expected = "#{jp_year}#{date.month}月#{date.day}日"
expect(person.format_date_of_birth).to eq expected
end
end
end
end
上のコードのように、テストコードがロジックロジックしていると、仕様がぱっと見てわかりにくいです。
それだけでなく、 テストコード自体のバグ により、本来パスすべきでないテストケースがパスしてしまう恐れもあります。
(そもそもテストコードがバグっているかどうかすら、コードを見ても瞬時に判別できません)
それよりも条件分岐やループ処理などを使わず、上から下に素直に読めるテストコードの方が 良いテストコード です。
describe Person do
describe '#format_date_of_birth' do
it '生年月日を和暦で表示すること' do
person = Person.new('Taro', Date.parse('1977-06-06'))
expect(person.format_date_of_birth).to eq '昭和52年6月6日'
person = Person.new('Taro', Date.parse('1988-06-06'))
expect(person.format_date_of_birth).to eq '昭和63年6月6日'
person = Person.new('Taro', Date.parse('1989-06-06'))
expect(person.format_date_of_birth).to eq '平成1年6月6日'
person = Person.new('Taro', Date.parse('2016-06-06'))
expect(person.format_date_of_birth).to eq '平成28年6月6日'
end
end
end
これであれば西暦がどのように和暦に変換されるのかが一目瞭然ですし、テストコード自体のバグが混入する可能性もほとんどなくなります。
(弁明:これはあくまでテストコードの善し悪しを語るための例ですので、「和暦変換のロジックに異議あり!」という意見はナシということでお願いします。。)
何事も機械的に適用しない。目的を考えて最適な選択肢を選ぶ
さて、こんなふうに書くと「なるほど、じゃあこれからはテストコードは何が何でもベタ書きにするぞ!!」と思い込んでしまう人が出てくるかもしれません。
しかし、極端から極端へ走ってしまうのもよくありません。
先ほど述べたように、ベタ書きはベタ書きでコード量が増えたり、テストコードが壊れやすくなったりする原因になります。
大事なことは「対象のコードの仕様がひと目でわかること」と「確実にバグを検出できること」です。
この2点をちゃんと満たせるのなら、ある程度テストコードをDRYにしていくのも問題ありません。
たとえば、RSpecにはdescribe/contextブロックのネストや、let、shared examplesといった機能があります。
こういった機能をうまく使えば、「対象のコードの仕様がひと目でわかること」と「確実にバグを検出できること」を満たしつつ、テストコードの重複を減らすことができます。
また、場合によっては実装コード側のメソッドをヘルパー的にテストコードに持ち込んだ方が、読みやすくて分かりやすいテストコードになるケースもあります。
つまり、テストコードを書くときはバランス感覚が重要だということです。
目的をちゃんと理解しつつ、メリットとデメリットを天秤にかけて、その場その場で最適な選択肢を選べるようになりましょう。
また、自分一人で決められない場合は他のメンバーを巻き込んでテストコードの善し悪しを議論するのがいいと思います。
FAQ:そもそもこの話って、本当にDRYと関係してるの??
タイトルや本文では「DRY」という言葉を多用していますが、「いや、これって別にDRYとは直接関係ないんじゃ?」と思われる方も多いと思います。
この点については実は僕自身もちょっと迷いながら、DRYという言葉を使っています。
詳しい理由や背景は以下の個人ブログに書いたので、DRYという言葉にしっくりこない人は一度目を通してみてください。
また、本記事には登場していない別のサンプルコードも出てきます。
Qiitaに「テストコードの期待値はDRYを捨ててベタ書きする」という記事を書きました - give IT a try
まとめ
というわけで、この記事ではテストコードはDRYを捨ててベタ書きする方が(原則として)望ましい、という話を書きました。
この考え方はRubyやRSpecに限らず、どんな言語やどんなテストフレームワークにも通じる話だと思います。
この記事を読んでから、自分の書いたテストコードを眺めてみてください。
ベタ書きした数値や文字列ではなく、変数やメソッドがアサーションの期待値になってたりしませんか?
if文やループ処理が大量にテストコードに紛れ込んだりしていませんか?
良いテストコードを書く上で、この記事がみなさんの参考になれば幸いです。
あわせて読みたい
この記事で説明したテストコードをMinitestで書き直したバージョンと、よりRSpecらしく書き直したバージョンを紹介しています。
「テストコードの期待値はDRYを捨ててベタ書きする」のテストコードをMinitestで書く・RSpecらしく書く - Qiita
RSpecの書き方、読み方がわからない、という方は以下の記事をご覧ください。
使えるRSpec入門・その1「RSpecの基本的な構文や便利な機能を理解する」 - Qiita
Railsプログラマの方には電子書籍「Everyday Rails」もオススメです。
Everyday Rails - RSpecによるRailsテスト入門