rspec / rspec-expectations

Provides a readable API to express expected outcomes of a code example
https://rspec.info
MIT License
1.26k stars 397 forks source link

Extending existing matchers #1267

Closed mockdeep closed 3 years ago

mockdeep commented 3 years ago

Somewhat similar to rspec/rspec-expectations#1219, I'm writing a custom matcher that adds a small twist to the change matcher. All over our tests we have something like this:

expect { post(:update, params: { id: user.id, user: { login: 'b' } }
  .to change { user.reload.login }.from('a').to('b')

I want a matcher that encapsulates the reload bit, something like:

expect { post(:update, params: { id: user.id, user: { loginl: 'b' } }) }
  .to change_record(user, :login).from('a').to('b')

I know it's not encouraged to inherit from existing matchers, but we want basically the same behavior as the change matcher, with a reload added after calling the event_proc. So our options are to either rebuild the change matcher from scratch, or to extend from the existing matcher and a handful of its private subsidiaries. Neither option feels great, but the second one is way cleaner, as there's a lot built into the change matcher. The messaging in particular. Here's the extended version:

extending matcher ```rb module Ik12 module Matchers class ChangeRecord < RSpec::Matchers::BuiltIn::Change def by(expected) ChangeRecordRelatively.new(change_details, expected, :by) do |actual| values_match?(expected, actual) end end def by_at_least(min) ChangeRecordRelatively.new(change_details, min, :by_at_least) do |actual| actual >= min end end def by_at_most(max) ChangeRecordRelatively.new(change_details, max, :by_at_most) do |actual| actual <= max end end def to(value) ChangeRecordToValue.new(change_details, value) end def from(value) ChangeRecordFromValue.new(change_details, value) end def matches?(event_proc) my_proc = lambda { event_proc.call change_details.instance_variable_get(:@receiver).reload } super(my_proc) end end class ChangeRecordRelatively < RSpec::Matchers::BuiltIn::ChangeRelatively def matches?(event_proc) my_proc = lambda { event_proc.call @change_details.instance_variable_get(:@receiver).reload } super(my_proc) end end class ChangeRecordToValue < RSpec::Matchers::BuiltIn::ChangeToValue def matches?(event_proc) my_proc = lambda { event_proc.call @change_details.instance_variable_get(:@receiver).reload } super(my_proc) end end class ChangeRecordFromValue < RSpec::Matchers::BuiltIn::ChangeFromValue def matches?(event_proc) my_proc = lambda { event_proc.call @change_details.instance_variable_get(:@receiver).reload } super(my_proc) end end end end ```

So I'm wondering if there are any alternatives I'm not thinking of. Or if there's any potential to make the existing matchers more extensible in some way. In my case all I really want to do is wrap event_proc, which seems simple in principle, but in practice it's pretty tricky.

mockdeep commented 3 years ago

I'm not sure if this is necessarily specific to Rails, since we like to write custom matchers for a variety of other cases as well. This is the first where we wanted to extend the change matcher, though.

JonRowe commented 3 years ago

The matcher internals are private API because it allows us to change them without major versions, so what you are doing is not technically supported, and I am not going to change that policy.

Instead I would suggest you delegate to change using the public api instead, (that is when you create an instance of your matcher you create an instance of change and use it directly wrapping the parts you wish to change).

JonRowe commented 3 years ago

I'm happy to continue discussion here, but I'm closing this as "not supported"

mockdeep commented 3 years ago

@JonRowe I'd love to do that, but I'm not sure how to make that happen. We want to wrap event_proc, but the change matcher can return several other matchers which might be handling it depending on the situation. Do you have any suggestions for how to do this cleanly?

JonRowe commented 3 years ago

Instantiate or pass the matcher to yourself, keep it internally within that class, interact with it only through your class. Then you can wrap the block when it goes in and compose it, and you can delegate to the methods you need.