Add and_invoke for sequential mixed (return/raise) responses.

[askreet]

In tests I occasionally encounter things like this:

called = false
allow(mock_api).to receive(:put_something) do
  # raise first call, second call succeeds
  unless called
    called = true
    raise ApiError, "Some Failure!"
  end
  :some_useful_value
end

It's commonly suggested to do similar things on StackOverflow, for example. It seemed a common enough problem to try to generalize a solution that is a bit more readable. This patch allows the following alternative:

allow(mock_api).to receive(:put_something).and_invoke(-> { raise ApiError, "Some Failure!" },
                                                      -> { :some_useful_value })

and_invoke behaves like and_return with regards to sequential calls, matching expected call counts, and repeating the last call indefinitely in cases where allow is used vs. expect with exactly(n).times.

I alternatively considered some more magical approaches like:

allow(mock_api).to receive(:put_something).and_return_or_raise(ApiError.new("Some Failure!"), true)

allow(mock_api).to receive(:put_something).chain_response
                                          .and_raise(RuntimeError, "Some Failure!")
                                          .and_return(true)

Ultimately, I thought Procs were more "native", if slightly ugly.

[askreet]

Ah, did not realize rspec-mocks kept compatibility with Ruby 1.8.7 -- impressive! Will fix the syntax to be compatible. (I assume this is just a "stabby lambda" issue.)

[pirj]

How do you feel about accumulating configured responses?
Now, e.g. and_return accepts several arguments, but the following:

allow(foo)
  .to receive(:bar)
  .and_return(1)
  .and_return(2)

won't work.

In the same way, it could be possible to use and_raise.

It's very similar to what you suggest with chain_response.

Side note: it's kind of confusing that it is possible to configure several responses at the moment, but only one, the last defined, actually comes into play.

[askreet]

I'm surprised this works, too, @pirj. I'm a bit worried about changing it because in all liklihood there are people who have done something like:

def default_fixture
  instance_double(Foo).tap |dbl|
    expect(dbl).to receive(:color).and_return(:blue)
  end
end

it "bar" do
  expect(baz(default_fixture)).to eq(4)
end

it "bars slightly differently under certain circumstances" do
  fixture = default_fixture.and_return(:aqua)

  expect(baz(fixture)).to eq(5)
end

However, your example raises a neat idea that would require a bit more careful architecture change:

allow(foo)
  .to receive(:bar)
  .and_return(1) # <== sets terminal action to AndReturnImplementation.new([1])
  .and_then_return(2) # <== replaces terminal action with ChainedTerminalAction.new([ existing_action, AndReturnImplementation([2]) ])
  .and_then_raise(RuntimeError, "failure!") # <== calls ChainedTerminalAction.append(AndRaiseImplementation.new(...))

The trick here would be the earlier chained implementations need to be extends to support not repeating the last element in their list, or track if they have been 'consumed' or not so that the chained action can move onto the next item. Likewise, if we want to keep existing behavior, we'd have to repeat the last action of the last chain element. Alternatively we could say that once you opt into this chaining construct, you must build an explicit chain of response elements.

Overall, I quite like this approach, but I'll hold off on making the change until I hear from more maintainers about the direction.

Note that my original #chained_responses implementation is actually easier to pull off because it can set the terminal implementation to a new ChainedResponses instance and then return that, allowing all further calls to go to the new object (although that would mean you couldn't use more RSpec DSL at that point).

[pirj]

I believe that chaining configured response qualifiers would imply those responses being called in order.

It needs a second thought on how countable qualifiers should work though.

receive(:foo)
  .and_return(1).twice
  .and_return(2).once # return 1, 1, then 2 

vs

receive(:foo)
  .twice
  .and_return(1)
  .and_return(2) # ok

receive(:foo)
  .thrice
  .and_return(1)
  .and_return(2) # the number of configured responses doesn't add up. Should we return `nil` on the third call?

receive(:foo)
  .twice
  .and_return(1)
  .and_return(2)
  .and_return(3) # Error! The number of configured responses exceeds the expected number of calls

Semi-related rspec/rspec-expectations#1252

[askreet]

I don't know, are you saying that in the case where we've qualified the counts with expectations that the responses should switch from being overridden to appending in a chained fashion? The inconsistency with the behavior when you call and_return twice (override) is pretty surprising. The nice thing about introducing a new method (and_then... or chained_responses) is that it's clear you're altering the default behavior.

Even in qualified cases where expectation counts have been specified, people may still be relying on the override behavior (like my example from above). It seems backwards compatibility is a reasonable top priority for this gem.

[JonRowe]

Initially I was going to suggest that as and_return already allows multiple static responses[1], it could be re-written to allow this scenario too, as having another name for configuring return responses seems sub-ideal, but I think I'm convinced this specific scenario needs its own name, to distinguish between a list of callables and others.

1

[benoittgt]

Thanks for this proposal. I love the idea. Last version of code looks good to me. Thanks for the clarity of the PR.

[askreet]

@benoittgt Great, glad to hear it. I'm good with where this is at whenever we have quorum to merge. Thanks!

[JonRowe]

Thanks!

[pirj]

Oh, I'm late for the party 🐌

It looks amazing, thanks for the contribution!

[pirj]

The feature description says "if the message is received additional times", maybe add an additional line repeating this one to prove/indicate that?