最近思うrspecをシンプルにする書き方（オレ流）

rspec書き方指南ではない

最近自分でやってみて、テストが意外に楽に書けるのではないかと思うやり方をまとめてみた。
ベストだとか唯一の書き方だとかそういう事を言いたいわけではない。各自自分の好きなやり方で書けばいいと思う。最終的には自分の書いたクラスがちゃんとテスト出来ていれば目的は達成している。

個人的な主観としてテストは出来るだけシンプルにしたい。そうじゃないと個人的にテストを書くのが嫌になるし、テストが間違ってて、そもそも何をテストしてるのか？という事になりたくない。
と言うことを解決できるのではないかという投稿。

要点

• クラス内のメソッド（処理）は出来るだけシンプルにしたい
• 一度しか呼ばれないようなロジックでもメソッドに落とし込みたい
• メソッドが増える分、内部のメソッドと外部向けのメソッドをちゃんと意識する
• itに説明は要らない
• テスト中に使用する変数内容は実際の条件を意識しない
• subjectとallowをよく使う
• expect(...).to have_received(...)をよく使う
• expect{...}.to change{...}をよく使う
• 条件により動作が変わる場合、正常系 + context "...という場合" ... のようにまとめる

例えばこういうモデルを考える

• 自動販売機である
• 商品は複数持てる
• 商品は個別の金額を持てる
• 商品は個別の在庫を持てる
• 返金できる
• 代金が不足すれば売れない
• 在庫がなくなれば売れない
• 客としては在庫が後何個あるかは関係ない（買えるのかどうかだけ）

machine = VendingMachine.new({
  "コーラ 350ml" => {
    price: 130,
    inventory: 10,
  },
  "コーラ 500ml" => {
    price: 160,
    inventory: 10,
  },
  "水 500ml" => {
    price: 110,
    inventory: 10,
  },
  "烏龍茶 500ml" => {
    price: 160,
    inventory: 10,
  },
})

machine.product_names # => ["コーラ 350ml", "コーラ 500ml", "水 500ml", "烏龍茶 500ml"]
machine.availables # => []
machine.insert 1000
machine.availables # => ["コーラ 350ml", "コーラ 500ml", "水 500ml", "烏龍茶 500ml"]
machine.select "烏龍茶 500ml" # => true
machine.credit # => 840
machine.refund # => 840
machine.credit # => 0

machine.insert 5000
9.times{ machine.select "烏龍茶 500ml" }
machine.availables.include? "烏龍茶 500ml" # => false
machine.credit # => 3560
machine.refund # => 3560
machine.credit # => 0

自分だとこう書く

class VendingMachine
  attr_reader :credit

  class InvalidAmountError < StandardError
  end

  class InvalidNameError < StandardError
  end

  def initialize(
    products = {
      # "商品名A" => {price: 999, inventory: 999},
      # "商品名B" => {price: 999, inventory: 999},
      # "商品名C" => {price: 999, inventory: 999},
    }
  )
    @products = products
    @credit = 0
  end

  def insert(amount)
    value = amount.to_i
    @credit += value if valid_amount?(value)
  end

  def select(name)
    available?(name).tap do |ok|
      next unless ok

      vend!(name)
    end
  end

  def refund
    @credit.dup.tap { |amount| @credit -= amount }
  end

  def product_names
    products.keys
  end

  def availables
    products.select { |_, product| product[:price] <= credit and 0 < product[:inventory] }
      .keys
  end

  private

  attr_reader :products

  def available?(name)
    availables.include? name
  end

  def valid_amount?(amount)
    raise InvalidAmountError if amount <= 0
    true
  end

  def product(name)
    products[name.to_s].tap { |product| raise InvalidNameError unless product }
  end

  def price(name)
    product(name)[:price]
  end

  def inventory(name)
    product(name)[:inventory]
  end

  def vend!(name)
    @credit -= price(name)
    product(name)[:inventory] -= 1
  end
end

クラス内のメソッド（処理）は出来るだけシンプルにしたい

どのメソッドも10行程度に収まってる。10行以内でなければならないとかそういう制約ではなく、それぐらい短いよ、という話。
メリットとしては、メソッド単位が短いので読む時に上から下まで変数が30個もあるようなメソッドと比べ、頭の中で覚えることが極端に減る、ので1年後の自分でも理解が楽。

一度しか呼ばれないようなロジックでもメソッドに落とし込みたい

上のこととも関連があるが、これはテスト時に大きく楽にできるメリットだと思う（一番下で説明）。

メソッドが増える分、内部のメソッドと外部向けのメソッドをちゃんと意識する

privateなメソッドを意識して、のちの自分や他の人がこのクラスを使う場合、表に出ているメソットだけを見て動かせるような構造にしたい（要は不要なメソッドは公開しない）。

自分でテストを書くとしたらこう書く

require "rails_helper"

RSpec.describe VendingMachine do
  subject { VendingMachine.new }

  describe :initialize do
    let(:product) { spy :product }
    subject { VendingMachine.new({ "商品名" => product }) }
    it { expect(subject.instance_eval { @products }).to eq({ "商品名" => product }) }
    it { expect(subject.credit).to be_zero }
  end

  describe :insert do
    let(:amount) { 100 }
    it do
      expect(subject.insert(amount)).to eq 100
      expect(subject.insert(amount)).to eq 200
    end
    it do
      expect {
        subject.insert(amount)
        subject.insert(amount)
      }.to change { subject.credit }.from(0).to(200)
    end

    context "不正な値の場合" do
      let(:amount) { 0 }
      it { expect { |b| subject.insert(amount, &b) }.to raise_error }

      it do
        expect {
          begin
            subject.insert(amount)
          rescue StandardError
            nil
          end
        }.not_to change { subject.credit }
      end
    end
  end

  describe :select do
    let(:available) { true }
    before do
      allow(subject).to receive(:available?).and_return available
      allow(subject).to receive(:vend!)
    end
    it do
      expect(subject.select "商品名").to eq true
      expect(subject).to have_received(:vend!).with "商品名"
    end

    context "選択可能ではない場合" do
      let(:available) { false }
      it do
        expect(subject.select "商品名").to eq false
        expect(subject).not_to have_received(:vend!)
      end
    end
  end

  describe :refund do
    before { subject.instance_eval { @credit = 1000 } }
    it { expect(subject.refund).to eq 1000 }
    it { expect { subject.refund }.to change { subject.credit }.from(1000).to(0) }
  end

  describe :product_names do
    let(:product) { spy :product }
    it do
      allow(subject).to receive(:products).and_return(
        { "商品名A" => product, "商品名B" => product },
      )
      expect(subject.product_names).to eq %w[商品名A 商品名B]
    end
  end

  describe :availables do
    let(:credit) { 100 }
    let(:inventory) { 10 }
    before do
      allow(subject).to receive(:credit).and_return credit
      allow(subject).to receive(:products).and_return(
        { "商品名" => { price: 100, inventory: inventory } },
      )
    end
    it { expect(subject.availables).to eq ["商品名"] }

    context "入金額が足りない場合" do
      let(:credit) { 99 }
      it { expect(subject.availables).to be_empty }
    end

    context "在庫が足りない場合" do
      let(:inventory) { 0 }
      it { expect(subject.availables).to be_empty }
    end
  end

  describe :private do
    describe :available? do
      before { allow(subject).to receive(:availables).and_return %w[商品名A 商品名B] }
      it { expect(subject.send :available?, "商品名A").to eq true }
      it { expect(subject.send :available?, "商品ではない").to eq false }
    end

    describe :valid_amount? do
      let(:amount) { 1 }
      it { expect(subject.send :valid_amount?, amount).to eq true }

      context "不正な場合" do
        let(:amount) { 0 }
        it do
          expect { |b|
            subject.send :valid_amount?, amount, &b
          }.to raise_error VendingMachine::InvalidAmountError
        end
      end
    end

    describe :product do
      let(:product) { spy :product }
      before { allow(subject).to receive(:products).and_return({ "商品名A" => product }) }
      it { expect(subject.send :product, "商品名A").to eq product }

      it do
        expect { |b|
          subject.send :product, "商品ではない", &b
        }.to raise_error VendingMachine::InvalidNameError
      end
    end

    describe :price do
      it do
        allow(subject).to receive(:product).and_return({ price: 100 })
        expect(subject.send :price, "商品名").to eq 100
      end
    end

    describe :inventory do
      it do
        allow(subject).to receive(:product).and_return({ inventory: 10 })
        expect(subject.send :inventory, "商品名").to eq 10
      end
    end

    describe :vend! do
      let(:product) { { inventory: 10 } }

      before do
        subject.instance_eval { @credit = 1000 }
        expect(subject).to receive(:price).with("商品名").and_return(100)
        expect(subject).to receive(:product).with("商品名").and_return(product)
      end

      it { expect { subject.send :vend!, "商品名" }.to change { subject.credit }.to 900 }
      it do
        expect { subject.send :vend!, "商品名" }.to change { product[:inventory] }.to 9
      end
    end
  end
end

itに説明は要らない

it "..." doとすると、テスト内容が変わる度に"..."を併せて更新しないといけないのでメンテナンスコストが上がる。
そもそもitブロック内の内容が少なければ「読めば分かる」程度なので説明は要らない。

テスト中に使用する変数内容は実際の条件を意識しない

今回の場合だと自動販売機なので商品名はコーラとかにしがち。コーラは現実に存在するのでテスト対象の自動販売機に現在入ってるのかどうかはパッと見分からない。

describe :available? do
  before { allow(subject).to receive(:availables).and_return %w[ジョルトコーラ ペプシ] }
  it { expect(subject.send :available?, "ペプシ").to eq true }
  it { expect(subject.send :available?, "コーラ").to eq false }
end

こうしておけば、絶対的に"商品ではない"は商品ではないと分かりやすい。

describe :available? do
  before { allow(subject).to receive(:availables).and_return %w[商品名A 商品名B] }
  it { expect(subject.send :available?, "商品名A").to eq true }
  it { expect(subject.send :available?, "商品ではない").to eq false }
end

subjectとallowをよく使う

subjectは一応トップに標準的なものを置いておいて、各テスト条件によっては上書きするような感じにしておくとシンプルになるのではないかと考える。

subject内のメソッドをallowしてテストを楽にする。
allowする場合、対象のオブジェクトがそういうメソッドを持っているかどうかのチェックが入るので、闇雲に値を突っ込むことはできない。ので安全。

例え一度しか使わないような場合でも、メソッドを細切れにしておくことで、各メソッド単位でテストが通ることを事前に確認さえしておけば、細切れのメソッドを他の部分で使う時はallowで期待値を設定出来る（stub）ので、今テストしているメソッドだけに集中してテストできる。

expect(...).to have_received(...)をよく使う

allowしたものがちゃんと呼ばれたのか？、ちゃんと期待したパラメータを受けたのか？は確認したい。

expect{...}.to change{...}をよく使う

これをこうしたらこれがこう変わる。という意味で読みやすい。

条件により動作が変わる場合、正常系 + context "...という場合" ... のようにまとめる

事前条件をbeforeにまとめておいて、contextで一部の値だけを変えて"...という場合"という意味で表現しやすいので、見た目スッキリするし分かり易い気がする。

だけれどもcontextが、Aという場合で且つBという場合のように条件がネストしだすと多分切り出せるメソットがあるはずなので、テストしているメソットが簡略化できないか？（細切れにして各単位をallowする方向にできないか）を考える。