Better API

[xaviershay]

Behaviour wise it is pretty stable now, but needs much better names.

A suggestion:

1. fire_double becomes instance_double
2. fire_class_double becomes class_double
3. Remove fire_replaced_class_double (stub_const is in core already).
4. .as_replaced_constant becomes .as_stubbed_const (this is orthogonal to this library, it should be in core regardless). Keep transfer_nested_constants option.
5. Keep include(RSpec::Fire) only while it is a separate gem, not required if in core.

We should make these changes over two releases: a point release with deprecation, then a major one with removed. After that has baked, look at porting it to core.

@myronmarston @alindeman @stevenharman

[myronmarston]

:+1:

I've been planning to port this to rspec-mocks in 3.0, and class_double/instance_double are the names I had in mind, so this sounds great.

[stevenharman]

The transfer_nested_constants name always struck me as an odd, but then the idea of having to copy over constants to a stubbed constant isn't something I think about every day. ;) There might be a better/alternative name out there, but I'm not sure what it is.

Overall, I am happy with the new names, and think the tactic of changing the API over two releases is spot on. :shipit:

[jmgarnier]

:+1: Shorter names but still makes a lot of sense (as a non native speaker!)

@myronmarston not sure the is the right place, but are you already working on RSpec 3 ? There is no public branch on https://github.com/rspec/rspec

[jfelchner]

Hey all, I really hope I'm not too late to the party here. @xaviershay, I have ZERO idea how rspec-fire does its business on the backend, so take my comments with that in mind. I may also be completely missing the fundamental philosophy behind rspec-fire. :smile:

One of the things that puts me off of rspec-fire is that (to me) the DSL doesn't read as well as the rest of RSpec. As @myronmarston has stated in numerous issues on rspec-mocks, rspec-fire is the unofficial official solution for verifying a double's API. However, one of the reasons why I use RSpec vs TestUnit/MiniTest is because when I read a spec, my brain doesn't have to do any translation from what my intentions were to what the spec does.

In that vein, I just wanted to ask if a DSL such as the following would be possible (and maybe someone from the RSpec team can jump in if there's an implementation detail in rspec-mocks preventing this).

I am 100% open to suggestions on the terminology of the matchers (even I'm not happy with what I've chosen), I just wanted to get the conversation started.

Contrived Example:

(please ignore the fact that you would never actually implement anything even remotely like this in real life :grinning:)

describe MyCart do
  let(:payment_method) { # Building a payment method somehow which is used other places in my specs }
  let(:gateway)        { # A gateway object built in some manner }

  it 'can purchase itself' do
    allow(gateway).to verifiably_receive(:purchase)

    my_cart.purchase(gateway)

    expect(gateway).to have_received(:purchase)
                      .with(payment_method)
  end
end

Or if it can't be done on the stubbing side, maybe it can be done on the expectation side:

describe MyCart do
  let(:payment_method) { # Building a payment method somehow which is used other places in my specs }
  let(:gateway)        { # A gateway object built in some manner }

  it 'can purchase itself' do
    allow(gateway).to receive(:purchase)

    my_cart.purchase(gateway)

    expect(gateway).to have_verifiably_received(:purchase)
                      .with(payment_method)
  end
end

Verify By Default Configuration Option:

Additionally, because when you're using rspec-fire, generally, you're going to want all of your doubles to be verified against the actual class, an option to have the default be a verifiable expectation would be nice.

RSpec.configure do |config|
  config.verify_doubles_by_default = true
end

# This would now verify implementation without explicitly saying so
describe MyCart do
  let(:payment_method) { # Building a payment method somehow which is used other places in my specs }
  let(:gateway)        { # A gateway object built in some manner }

  it 'can purchase itself' do
    allow(gateway).to receive(:purchase)

    my_cart.purchase(gateway)

    expect(gateway).to have_received(:purchase)
                      .with(payment_method)
  end
end

I would love to hear everyone's thoughts on this.

[stevenharman]

I also thought a config option to verity by default is desirable, but felt as though I were missing an obvious reason that wouldn't work. But perhaps not?

[myronmarston]

@jfelchner -- thanks for jumping in with your thoughts. I'm working on a "the plan for RSpec 3" that I'll be posting shortly after we release 2.14.0. My plan for better mock safety is one of the main features I'm planning for RSpec 3, and the blog post will go into more detail. But briefly...

• I've already been planning on adding a config option like you mentioned above, to have rspec auto-verify mocks/stubs on partial mock objects.
• I tend to favor using pure test doubles (using dependency injection to get them into the object-under-test), and I plan to port something very similar to rspec-fire over to rspec-mocks.

[jfelchner]

@myronmarston that sounds great! This may be a discussion for elsewhere, but how does using pure test doubles ensure that your object's API stays in sync with your mock? In fact, what is the use-case for rspec-fire when used with pure test doubles? A double is not backed by anything so what is the use of verifying that a method exists? Maybe I'm missing the whole point?

For example (again, it's horribly contrived):

class Gateway
  def purchase(payment_method, amount)
    # Do purchase-y stuff
  end
end

class PaymentMethod  
  # Here we're doing dependency injection of the gateway
  def purchase!(gateway, amount)
    gateway.purchase(self, amount)
  end
end

# With pure test doubles there is no way to ensure that if Gateway removes its
# #purchase method, that the spec will fail
describe PaymentMethod do
  let(:payment_method) { # Build a payment method }

  it 'can initiate a purchase' do
    gateway = double(:purchase => true)

    payment_method.purchase!(gateway, 10)

    expect(gateway).to have_received(:purchase)
                      .with(payment_method, 10)
  end
end

[jfelchner]

@myronmarston if you're too busy, I'll just wait to see what you outline in your RSpec 3 roadmap. No worries.

[myronmarston]

This may be a discussion for elsewhere, but how does using pure test doubles ensure that your object's API stays in sync with your mock? In fact, what is the use-case for rspec-fire when used with pure test doubles? A double is not backed by anything so what is the use of verifying that a method exists? Maybe I'm missing the whole point?

rspec-fire specifically helps with using pure test doubles. It doesn't do anything for partial mocks. When creating your doubles, you tell it the name of a class whose interface your double should be verified against.

In your example, here's how you would change it to use rspec-fire:

# With pure test doubles there is no way to ensure that if Gateway removes its
# #purchase method, that the spec will fail
describe PaymentMethod do
  let(:payment_method) { # Build a payment method }

  it 'can initiate a purchase' do
    gateway = instance_double("Gateway", :purchase => true)

    payment_method.purchase!(gateway, 10)

    expect(gateway).to have_received(:purchase)
                      .with(payment_method, 10)
  end
end

By creating the gateway double as an instance_double, and giving it the Gateway class name as the first argument, rspec fire will verify that :purchase stub.

[jfelchner]

@myronmarston NOW I see. I still don't think it reads very well so expect to hear from me again once you release your RSpec 3 roadmap. :wink: but at least now I understand what's going on.

rspec-fire is creating pure test doubles but it's just using the first argument to figure out what to verify against. And the reason it needs the cumbersome instance_double and class_double is because it needs to know whether to verify against instance methods or class methods.

Thank you very much for taking the time to explain it to me.

[xaviershay]

This has been addressed in RSpec 3.