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 two RSpec matchers, have_been_fired and have_subscription,
which can be used after the event bus has been stubbed through
stub_events.

Internally, they lean 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))
    expect('baz').to have_subscription
  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)

[aldesantis]

@waiting-for-dev I don't know if I'm a huge fan of stubbing the entire Spree::Event module to be honest. It prevents people from mixing and matching the helpers we provide (e.g., performing_only with have_been_fired), and it can lead to very confusing behavior.

What if, instead of stubbing the entire module, we simply stored the fired events in a global variable somewhere, and we matched against that? That would allow people to mix the behavior however they want and it removes any confusion. (Also, you wouldn't need to call stub_events at the beginning of the test.)

We could do this through the new observability hooks you have implemented, by storing all firings globally, for example!

[waiting-for-dev]

Thanks for your feedback, @aldesantis, and sorry for taking so long to get back to you!

I don't know if I'm a huge fan of stubbing the entire Spree::Event module to be honest.

I'm neither a fan of this. But I'd say there're two main approaches when it comes to testing: stubbing and injection. I always try to go with the latter when I can, but some people rightfully prefer the former. I wanted the event bus to support both main approaches. We can put it on hold if you prefer.

It prevents people from mixing and matching the helpers we provide

Yeah, it was intentional as part of the support for both approaches. #performing_only falls within the injection category, and I wanted to make the difference evident in the docs.

What if, instead of stubbing the entire module, we simply stored the fired events in a global variable somewhere

I'm not a fan of storing things globally, as it always ends up causing problems. But I see it as an extension of the injection strategy. We can add the option to inject a different bus for a given block (it's what #performing_only is doing) and keep the fired events within the state of the bus instance. It should be straightforward to implement!

[aldesantis]

@waiting-for-dev I've given this some thought, and I can't really come up with a better and equally elegant alternative. This seems like it could work well, and we can always revisit the implementation down the road if needed without touching the public API too much, so let's just go ahead with what you have and see how it plays out!

[aldesantis]

I'm not completely sure, but I suspect this might not work properly in a real app, where the event subscribers are subscribed during boot (rather than after the stub_events call). Or have you already checked?

[waiting-for-dev]

Yeah, you're right. It's useless for module-based subscribers. My idea was to use it for subscriptions made through the raw Spree::Event.subscribe method. But maybe it would be something confusing for users. Do you think we should remove it altogether?

[aldesantis]

@waiting-for-dev I think the standard will be module-based subscriber, so yeah, I don't think it adds a lot of value right now. 😢

[waiting-for-dev]

✂️ ✂️ done! The less code to maintain, the best! 🙂 I forced a push removing have_subsription and updated the commit message.

[waiting-for-dev]

Moved to solidusio#4214