Add stubbing test helpers for the event bus

[waiting-for-dev]

Description

This commit adds a Spree::TestingSupport::EventHelpers module which
adds stubbing helpers for Spree::Event when included in an RSpec file.

It adds a RSpec matcher, have_been_fired, which can be used after the
event bus has been stubbed through stub_events.

Internally, it leans on the have_received built-in rspec-mocks
matcher. Unfortunately, RSpec's matcher lacks good extensibility, so we
can't provide OOTB with all its bells and whistles (like once,
exactly(2).times), and the effort is not worth it at this point.
However, we do support a with modifier for have_been_fired to match
the published payload.

Example:

require 'rails_helper'
require 'spree/testing_support/event_helpers'

RSpec.describe MyClass do
  include Spree::TestingSupport::EventHelpers

  it 'does stuff with events' do
    stub_events

    described_class.new.do_stuff_with_events

    expect('foo').to have_been_fired
    expect('bar').to have_been_fired.with(a_hash_containing(foo: :bar))
  end
end

Checklist:

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have updated Guides and README accordingly to this change (if needed)
• I have added tests to cover this change (if needed)
• I have attached screenshots to this PR for visual changes (if needed)

[kennyadsl]

Sad that we can't use once or similar, but this is a great starting point. Thanks @waiting-for-dev!

[adammathys]

Very cool. Might be nice to have it print out the difference in arguments when they're provided, but this is a great initial implementation.

[waiting-for-dev]

Very cool. Might be nice to have it print out the difference in arguments when they're provided, but this is a great initial implementation.

Thanks, @adammathys. Yeah, I agree. Unfortunately, there's no straightforward way to reuse the logic in RSpec's have_received matcher. So, it's easy to show the payload given in the expectation, but not the actual one 😞 For now, I feel that it's not worth it to make the code more complex...

[spaghetticode]

@waiting-for-dev this looks so convenient! I was thinking that we could also use rspec metadata so we don't need to explicitly write stub_events in the example body, something like this:

it 'fires foo event', :stub_events do
  Spree::Event.fire 'foo'

  expect('foo').to have_been_fired
end

what do you think?

[waiting-for-dev]

@waiting-for-dev this looks so convenient! I was thinking that we could also use rspec metadata so we don't need to explicitly write stub_events in the example body, something like this:

it 'fires foo event', :stub_events do
  Spree::Event.fire 'foo'

  expect('foo').to have_been_fired
end

what do you think?

@spaghetticode, I think it's a great idea to use it that way! However, I'm not particularly eager to modify RSpec configuration from a library. I think that's something that we could address from the docs point of view. How do you feel about that?

[spaghetticode]

@spaghetticode, I think it's a great idea to use it that way! However, I'm not particularly eager to modify RSpec configuration from a library. I think that's something that we could address from the docs point of view. How do you feel about that?

I think I get what you mean, but I'm not sure I would consider that addition something that intrusive, the possible clash with other metadata names is unlikely, but if this is a concern then the name could be changed to :stub_spree_events (actually, while I am writing this I am starting to think it may be a good idea to change also the helper method name to that 🤔)... then Solidus is a Rails engine, not a simple library so some level of convenience at the price of some obtrusive configuration doesn't seem unreasonable to me. But in the end I don't have strong opinions, I think it's reasonable both ways!

[waiting-for-dev]

actually, while I am writing this I am starting to think it may be a good idea to change also the helper method name to that

I think you're right, and it makes it consistent with the approach used elsewhere in the codebase. I just renamed the method to stub_spree_events

then Solidus is a Rails engine, not a simple library so some level of convenience at the price of some obtrusive configuration doesn't seem unreasonable to me. But in the end I don't have strong opinions, I think it's reasonable both ways!

Thanks for bringing up those points. That's entirely a valid reason! I think my hesitance grounds in two points:

• As I said, if possible, I always try to avoid changing user configuration (also from an engine)
• I'm not particularly fond of controlling a test context through metadata:
  • It adds indirection
  • Encourages sharing state between tests
  • It makes debugging more difficult as we're using something that is no actual code (so to speak)

Considering that the whole stubbing approach is not my favorite one (I'm more in favor of using #4204), I wouldn't like to make it too easy to use if you get my point. For instance, if someone adds stub_spree_event: true on the top describe block, it'd encourage using it elsewhere in the file.

On top of that, it'll be a one-liner to add it the user application. Otherwise, before(:each) blocks can be used. It's only a few keystrokes longer and at least is actual code (again, so to speak).

Now, I understand that those are all opinable reasons. But, if you don't have strong opinions about it, I'd prefer to leave it as it is for now. It'll always be easier to add it than to remove it 🙂