Stubbing a return value in RSpec - correct usage of doubles and stubs

Question

I have run into confusion tying together a test double and stubbing it. My question is - what is the most appropriate way to test the confirm_purchase_order and create_order methods in class PurchaseOrder?

I have included the relevant code following code:

class PurchaseOrder
  attr_reader :customer, :products

  def initialize(customer)
    @products = {}
    @customer = customer
  end

  ....some other methods

  def add_product(product, quantity = 1)
    @products[product] = (@products[product] ? @products[product] + quantity :  quantity )
    puts "You haved added #{quantity} #{product.title}'s to your purchase order"
  end

  def confirm_purchase_order
    purchase_order_total
    raise "Your PO appears to be empty! Add some products and try again." unless self.total.to_f.round(2) > 0

    create_order
    create_invoice

    return "We have generated an Invoice and created an order."
  end

  def create_order
    order = Order.new(customer)
    order.products = @products.clone
  end

  def create_invoice
    invoice = Invoice.new(customer)
    invoice.products = @products.clone
  end
end

---

class Order
  attr_reader :customer
  attr_accessor :status, :total, :products

  def initialize(customer)
    @products = {}
    @status = :pending
    @customer = customer
  end

---

class Customer
  attr_reader :name, :type

  def initialize(name, type)
    @name = name.to_s
    @type = type.to_sym
  end

---

class Invoice

  attr_reader :customer, :products
  attr_accessor :total

  def initialize(customer, products)
    @products = {}
    @customer = customer
    @payment_recieved = false
  end
end

---

I want to test the confirm_purchase_order method as well as the create_order method in class PurchaseOrder. My approach so far:

I need some object doubles and an actual PurchaseOrder object

describe PurchaseOrder do
  let(:product) { double :product, title: "guitar", price: 5 }
  let(:order) { instance_double(Order) }
  let(:customer) { double :customer, name: "Bob", type: :company }
  let(:products) { {:product => 1} }

  let(:purchase_order) { PurchaseOrder.new(customer) }

  describe "#create_order" do

    it "returns an order" do
      expect(Order).to receive(:new).with(customer).and_return(order)
      allow(order).to receive(products).and_return(???products??!)

      purchase_order.add_product(product, 1)
      purchase_order.create_order
      expect(order.products).to eq (products)
    end
  end
end

I have also looked at the use of:

 # order.stub(:products).and_return(products_hash)
 # allow_any_instance_of(Order).to receive(:products) { products_hash }
 # order.should_receive(:products).and_return(products_hash)

To setup the order double to return a products hash when order.products is called, but these all feel like they are 'rigging' the test too much. What is the most appropriate way to test the confirm_purchase_order and create_order methods in class PurchaseOrder?

Answer 1

It seems to me that perhaps you're giving PurchaseOrder too much responsibility. It now has intimate knowledge about Order and Invoice.

I'd perhaps test the current implementation like this:

it "returns an order with the same products" do
  expect_any_instance_of(Order).to receive(:products=).with(products: 1)

  purchase_order.add_product(product, 1)
  expect(purchase_order.create_order).to be_a(Order)
end

But maybe it could make sense to decouple PurchaseOrder from Order and Invoice a little bit and do something like this:

class Invoice
  def self.from_purchase_order(purchase_order)
    new(purchase_order.customer, purchase_order.products.clone)
  end
end

class Order
  def self.from_purchase_order(purchase_order)
    new.tap(purchase_order.customer) do |invoice|
      invoice.products = purchase_order.products.clone
    end
  end
end

class PurchaseOrder
  # ...
  def create_order
    Order.from_purchase_order(self)
  end

  def create_invoice
    Invoice.from_purchase_order(self)
  end
end

describe PurchaseOrder do
  let(:customer) { double('a customer')}
  let(:purchase_order) { PurchaseOrder.new(customer) }
  describe "#create_order" do
    expect(Order).to receive(:from_purchase_order).with(purchase_order)
    purchase_order.create_order
  end

  describe "#create_invoice" do
    expect(Order).to receive(:from_purchase_order).with(purchase_order)
    purchase_order.create_order
  end
end

describe Order do
  describe '.from_purchase_order' do
    # test this
  end
end

describe Order do
  describe '.from_purchase_order' do
    # test this
  end
end

This way you let the Order and Invoice classes know how to build themselves from a PurchaseOrder. You can test these class methods separately. The tests for create_order and create_invoice become simpler.

Some other things I thought of:

For products, try using a Hash with a default proc:

@products = Hash.new { |hash, unknown_key| hash[unknown_key] = 0 }

This way, you can always safely do @products[product] += 1.