[RSpec] テストが書きやすいコードは良いコード

概要

classと単体テストを用いて、テストを書きやすくしたいと考えるうちにプロダクトコードが綺麗になっていく様子を解説したいと思います。

ふざけたmodelとテストコードを用意して、徐々に機能追加していきます

ore.rb

class Ore
  def self.firstname(lastname)
    lastname == 'Matsuyama' ? 'Hideyuki' : '(Unknown)'
  end
end

ore_spec.rb

describe Ore, type: :model do
  describe '.firstname' do
    subject { Ore.firstname lastname }

    context 'when lastname is "Matsuyama"' do
      let(:lastname) { 'Matsuyama' }
      it { is_expected.to eq 'Hideyuki' }
    end

    context 'when lastname is "Foo"' do
      let(:lastname) { 'Foo' }
      it { is_expected.to eq '(Unknown)' }
    end
  end
end

和名にも対応してみましょう

ore.rb

class Ore
  def self.firstname(lastname)
    case lastname
    when 'Matsuyama'
      'Hideyuki'
    when '松山'
      '秀行'
    else
      '(Unknown)'
    end
  end
end

ore_spec.rb

describe Ore, type: :model do
  describe '.firstname' do
    subject { Ore.firstname lastname }

    context 'when lastname is "Matsuyama"' do
      let(:lastname) { 'Matsuyama' }
      it { is_expected.to eq 'Hideyuki' }
    end

    context 'when lastname is "松山"' do
      let(:lastname) { '松山' }
      it { is_expected.to eq '秀行' }
    end

    context 'when lastname is "Foo"' do
      let(:lastname) { 'Foo' }
      it { is_expected.to eq '(Unknown)' }
    end
  end
end

もぅ外出しのファイルに持っちゃいましょう（テストコードは変わらずグリーン）

ore.rb

require 'yaml'

class Ore
  def self.firstname(lastname)
    ore = YAML.load_file 'ore.yml'
    find_ore = ore.find { |row| row['lastname'] == lastname }
    find_ore ? find_ore['firstname'] : '(Unknown)'
  end
end

ore.yml

---
-
  firstname: 'Hideyuki'
  lastname: 'Matsuyama'
-
  firstname: '秀行'
  lastname: '松山'

例外処理と、オレオレ詐欺も追加します

ore.rb

require 'yaml'

class Ore
  def self.firstname(lastname)
    begin
      filename = 'ore.yml'
      ore = YAML.load_file filename
      find_ore = ore.find { |row| row['lastname'] == lastname }
      if find_ore
        find_ore['firstname']
      elsif lastname == 'オレオレ'
        '詐欺かも'
      else
        '(Unknown)'
      end
    rescue Errno::ENOENT
      puts "'#{filename}' is not found."
    rescue 
      raise
    end
  end
end

ore_spec.rb

describe Ore, type: :model do
  describe '.firstname' do
    subject { Ore.firstname lastname }

    context 'when lastname is "Matsuyama"' do
      let(:lastname) { 'Matsuyama' }
      it { is_expected.to eq 'Hideyuki' }
    end

    context 'when lastname is "松山"' do
      let(:lastname) { '松山' }
      it { is_expected.to eq '秀行' }
    end

    context 'when lastname is "Foo"' do
      let(:lastname) { 'Foo' }
      it { is_expected.to eq '(Unknown)' }
    end

    context 'when lastname is "オレオレ"' do
      let(:lastname) { 'オレオレ' }
      it { is_expected.to eq '詐欺かも' }
    end
  end
end

リファクタリング

ここまで、テストのグリーンを保ちつつmodelに機能を追加しました。ここで、Ore.firstnameメソッドの処理を列挙してみます。

• ore.ymlを読み込みRuby Objectに変換する
• 引数lastnameで検索しfirstnameを返す
• もしlastnameが'オレオレ'だったら詐欺を疑う
• その他の場合、'(Unknown)'を返す
• ore.ymlへのアクセスに失敗した場合、メッセージを出力し正常終了する。
• その他エラーの場合、異常終了する

結構な頑張り屋さんになりました。ここでポイントとなるのは、次の点でテストが書きにくいということです。

• このメソッド名にしてはいろいろやりすぎている
• 戻り値が何になるのか、散在していて分かりにくい
• 2つのrescueに到達するのが難しい
• パッと見、if elseで何がしたいのか分かりにくい
• ダサい

なので、以下の方針でリファクタリングを行います

• 処理ごとにメソッドを分ける
• 1つのメソッドが負う責任をシンプルにする
• if elseは、その目的を直感的に表すメソッド名にして切り出す
• AメソッドがBメソッドを呼び出す場合「Bメソッドを呼んだ」ことのみ確認する（戻り値を処理していないので）

すると以下のコードになりました。

ore.rb

require 'yaml'

class Ore
  def self.firstname(lastname)
    ore = load_file 'ore.yml'
    find_ore = ore.find { |row| row['lastname'] == lastname }
    find_ore ? find_ore['firstname'] : 怪しむ(lastname)
  end

  def self.load_file(filepath)
    begin
      YAML.load_file filepath
    rescue Errno::ENOENT
      print_message filepath
    rescue
      raise
    end
  end

  def self.怪しむ(lastname)
    lastname == 'オレオレ' ? '詐欺かも' : '(Unknown)'
  end

  def self.print_message(filepath)
    puts "'#{filepath}' is not found."
  end
end

ore_spec.rb

describe Ore, type: :model do
  describe '.firstname' do
    subject { Ore.firstname lastname }

    context 'when lastname is "Matsuyama"' do
      let(:lastname) { 'Matsuyama' }
      it { is_expected.to eq 'Hideyuki' }
    end

    context 'when lastname is "松山"' do
      let(:lastname) { '松山' }
      it { is_expected.to eq '秀行' }
    end

    context 'when lastname is "Foo"' do
      let(:lastname) { 'Foo' }
      it { is_expected.to eq '(Unknown)' }
      it 'should receive .怪しむ' do
        expect(Ore).to receive(:怪しむ).with(lastname); subject
      end
    end
  end

  describe '.load_file' do
    let(:target_filepath) { 'ore.yml' }

    subject { Ore.load_file target_filepath }

    it { is_expected.to be_present }

    context 'when the loading filepath is nil' do
      let(:target_filepath) { nil }

      it 'should does not printing message, and raise error' do
        expect do
          expect(Ore).not_to receive(:print_message)
          subject
        end.to raise_error
      end
    end

    context 'when the loading filepath does not exist' do
      let(:target_filepath) { 'not_exist_filepath.yml' }

      it 'should printing message, and does not raise error' do
        expect do
          expect(Ore).to receive(:print_message).with(target_filepath)
          subject
        end.not_to raise_error
      end
    end
  end

  describe '.怪しむ' do
    subject { Ore.怪しむ lastname }

    context 'when lastname is "オレオレ"' do
      let(:lastname) { 'オレオレ' }
      it { is_expected.to eq '詐欺かも' }
    end

    context 'when lastname is "だれだれ"' do
      let(:lastname) { 'だれだれ' }
      it { is_expected.to eq '(Unknown)' }
    end
  end
end

実行結果.sh

Ore
  .firstname
    when lastname is "Matsuyama"
      should eq "Hideyuki"
    when lastname is "松山"
      should eq "秀行"
    when lastname is "Foo"
      should eq "(Unknown)"
      should receive .怪しむ
  .load_file
    should be present
    when the loading filepath is nil
      should does not printing message, and raise error
    when the loading filepath does not exist
      should printing message, and does not raise error
  .怪しむ
    when lastname is "オレオレ"
      should eq "詐欺かも"
    when lastname is "だれだれ"
      should eq "(Unknown)"

Finished in 0.04318 seconds (files took 226 minutes 55 seconds to load)
9 examples, 0 failures

テスト内容は次の通りです。

Ore.firstname

• lastnameがore.ymlに存在しない場合、怪しもうとするところまで確認

Ore.load_file

• ore.ymlの正常な読み込みを確認
• 引数がnilだった場合の異常系処理にてメッセージが出力されないこと、エラーのraiseを確認
• 引数が存在しないファイルパスだった場合は、メッセージが出力されること、エラーがraiseされないことを確認

Ore.怪しむ

• 詐欺かも

まとめ

以上です。如何だったでしょうか。ついでになりますが、後で直すと分かっていても最初からテストを書くことは重要であることも表現できたと思います。
このように単体テストの書きやすさを求めることで、プロダクトコードが読みやすく、かつメンテしやすく改善されていく方向へ自然と導かれるのです。

さらに予想できることとして、エラーメッセージの出力先をloggerに変えたらバグが発生したとします。そのときOre.firstnameが疑われて可哀想という事態も回避できます。アタリを小さくできるわけですね。有事の際、疑うべき箇所を極小化でき、更にテストが書きやすい。ステキです。

小さく小さく、故に早く＆速く。開発はこうでなければ。