Prevent error formatting from causing recursive errors

[jamesdabbs-procore]

This comes from tracking down https://github.com/rspec/rspec-rails/issues/2049 wherein

it 'should show only 2 times' do
  expect(@student).to     receive(:email).once.and_call_original
  @student.email
  @student.email
end

overflows. There Devise has defined an inspect method which calls email, so when the second call to email errors, the error generator's inspect call goes into a loop.

I've added a spec that I think describes the desired behavior, but which is currently failing against master.

The included lib change fixes the spec, but this is my first time digging into the RSpec codebase, and it's entirely possible there's a better place to make that change.

Please let me know if I've mis-understood the desired behavior, or if there's anything else you'd like to see changed. Thanks!

[fables-tales]

@jamesdabbs-procore would you be able to finish this one off and close it out?

[jamesdabbs-procore]

Ah, sorry. Lost this thread. Definitely, although it might be sometime this weekend.

[pirj]

@jamesdabbs-procore ping!

[jamesdabbs-procore]

Oh, sorry. I posted a threaded response earlier - https://github.com/rspec/rspec-mocks/pull/1275#discussion_r295124926 - and could use some feedback there. (I'm not entirely sure what change is being requested.)

[jamesdabbs-procore]

@JonRowe - I think that last implements the changes you were requesting. I also don't love the proliferation of flags / method-name combinations. Are the existing safe_invoke and invoke_without_incrementing_received_count intended to be part of the publicly supported API here? (Is invoke itself?) If not, we could standardize on something like

def invoke(parent_stub, *args, increment: 1, allow_failure: !@invoking, &block)

and update the existing callers.

What were you thinking in terms of refactoring the existing invoke?

[benoittgt]

I find the current implementation pretty clean and readable.

I don't think we want this to be accessible to the public API. Do you see a reason to have it public? I am not against your proposal but it is little bit more complicated. :)

Could you rebase master for the CI?

[JonRowe]

Ok I'm happy to merge this once CI is green, if I feel that strongly about the method names I'll refactor it myself later 😂, can you give it a rebase?

[JonRowe]

I'm not sure why this never got merged, I think it just needed a rebase and then got forgotten, sorry about that! I've migrated it to the monorepo rspec/rspec#116 and will aim to get it merged.