Feature suggestion: Custom expectation callbacks to default matchers like receive

[alexevanczuk]

Subject of the issue

I would like to customize the receive matcher to add custom behavior. We use the Contracts gem (runtime type checking) where I work, and we built out a custom matcher that verifies when you use this custom matcher, we effectively call super (that is, use the receive matcher), and also verify that the input and output parameters match the specified contract of the ExpectationTarget's method. This works great, but:

1. Users have to know to use this matcher.
2. The matcher does not support things like at_least, and_yield, exactly and other chains that receive supports.

I want everything in our test suite to use this custom matcher by default. I tried something kind of hacky to make this happen:

RSpec.configure do |config|
  config.before(:each) do |example|
    allow_any_instance_of(RSpec::Expectations::ExpectationTarget).
      to receive(:to).and_wrap_original do |m, *args|
        actual_or_proxy_double = m.receiver
        object = actual_or_proxy_double.target
        matcher = args.first
        method_name = matcher.instance_variable_get(:@message)

        contract_object = ContractObject.new(object, method_name)

        # Not all matchers implement name
        using_contractually_receive = matcher.try(:name) == :contractually_receive || caller.any? { |f| f.include?('contractually_receive') }

        contracts_enabled = !example.metadata[:no_contracts]

        if contracts_enabled && contract_object.has_contract? && !using_contractually_receive
          # While we could raise here, or just automatically use the new matecher, unfortunately
          # `contractually_receive` is not yet mature enough to handle all of the things `RSpec::Mocks::Matchers::Receive`
          # can handle, such as block syntax, `and_raise`, `and_yield`, `at_least`, and other
          # add on matchers.
          message = "[WARNING] Could be using `contractually_receive` matcher for:\n#{contract_object}"
          puts Rainbow(message).yellow
          # new_matcher = self.contractually_receive(*args)
          # m.call(new_matcher)
        end

        m.call(*args)
      end
  end
end

The main problem is that I can't force people to use it until it supports more chains.

Suggested Changes

Allowing configuration of default matchers

One thought is that we can configure default matchers such as RSpec::Mocks::Matchers::Receive to allow for custom behavior. So when someone passes a receive matcher into the to method of an ExpectationTarget using the normal syntax expect(target).to receive(:method), it will call our callbacks too.

This could look like this, but could probably use a lot of work. The only thing I know about how rspec-mocks works under the hood is through debugging to try to make the hack above work, so I'm not sure what's considered public/private API and what would be a solid interface.

RSpec::Mocks::Matchers::Receive.configure do |matcher|
  matcher.expectation_callback do |expectation_target, with_matcher_params, return_matcher_params, ...|
    expect(expectation_target).to obey_contract_of(:method).with(parameters).and_return(return)
  end
end

I see that the matcher has a recorded_customizations instance variable, which looks like it could make this possible.

Allowing inheriting from default matchers

It would be great to define custom matchers by inheriting from default matchers (if those matcher methods are or could be public interface).

So I could have a class that looks like this:

class ContractMatcher < RSpec::Mocks::Matchers::Receive
  # Not sure what the interface could look like for this
end

Would be happy to contribute after hearing your thoughts. Thanks for reading!

Your environment

• Ruby version: ruby 2.3.5p376 (2017-09-14 revision 59905) [x86_64-darwin16]
• rspec-mocks version: rspec-mocks (3.6.0)

Steps to reproduce

N/A

Expected behavior

N/A

Actual behavior

N/A

[JonRowe]

Why don't you just override the receive matcher?

module ContractExpectation
  def receive(*args, &block)
    super(*args) do
      # custom logic
      block.call unless block.nil
    end
  end
end
RSpec.configuration.include ContractExpectation

Or if you want to customise the receive matcher, vendor it into your code base and customise it. All matchers follow the "matcher protocol" and thus you wouldn't really be at risk from any underlying changes, you could probably simplify the matcher for your use case. You could then publish this as your own extension gem/

[myronmarston]

I was going to suggest something similar, but using a decorator based on SimpleDelegator:

require 'delegate'

class ReceiveWithContracts < SimpleDelegator
  # Put your custom logic here; all method calls will be delegated to the underlying `receive` matcher.
  # Just override the methods you need and use `super` to delegate as needed
end

module ReceiveWithContractsExtension
  def receive(*args, &block)
    ReceiveWithContracts.new(super)
  end
end

RSpec.configure do |config|
  config.include ReceiveWithContractsExtension
end

You can customize it to your hearts content, with no changes needed in RSpec itself. In general decorating the matcher with SimpleDelegator is a very robust approach; no monkey patching is involved and you can't accidentally mess up the internals as you're wrapping the matcher and don't have access to its internals.

[myronmarston]

(Whether @JonRowe's approach or mine is better largely depends on your needs, but I believe either would work).

[alexevanczuk]

This is great! I will give these strategies a try. Thanks for all of your hard work on RSpec!

[myronmarston]

Great; let us know if you have further questions. Closing.

[alexevanczuk]

I got things in a state that is mostly working, but just wanted to point out some issues I ran into with the above approach when it came to making sure I could use the with and and_return matchers.

At the bottom is what I ended up doing.

@JonRowe @myronmarston I went with a combination of both of your approaches. I couldn't simply override the existing matcher, because I needed to override other things such as with, and_return, etc. in order to get the full set of data I need to verify the types of the input and output parameters.

There were some nuances with passing around the blocks correctly, since some of the functions take a block and others don't (e.g. and_return). I needed to not allow this custom matcher for and_wrap_original, and eventually any case that takes a block, because of the issue here: #774

I wasn't able to use the SimpleDelegator unfortunately, because of:

     def matcher_allowed?(matcher)
      Matchers::Matcher === matcher
    end

Maybe there is a way to compose the delegation logic and ensure the proper === behavior. In the end I just inherited from the rspec receive matcher, which may not be kosher, but I think it is almost identical anyways to taking in a matcher of any type and delegating to it.

Lastly, I needed to raise an error in my custom logic for matches?, does_not_match? and setup_allowance, since it appears these are the methods called on the receive matcher that is the equivalent of the dsl match method. It would be nice to not raise an error and conform to whatever matcher API would let me tell callers that the match is invalid. Seems like matches? should just return a boolean, but I'm not sure if it's possible to fail in the setup_allowance and other methods as it stands.

Let me know what you think of this approach!

Thank you,
Alex

require 'delegate'

module ContractuallyReceiveExtension
  def receive(*args, &block)
    ContractuallyReceive.new(*args, block)
  end
end

RSpec.configure do |config|
  config.include ContractuallyReceiveExtension
end

require 'delegate'

class ContractuallyReceive < RSpec::Mocks::Matchers::Receive
  attr_reader :object, :message, :not_supported_by_custom_matcher

  def matches?(subject, &block)
    @object = subject
    if !not_supported_by_custom_matcher
      # Custom logic -- I have to raise here rather than fail in the same way I can fail using a DSL created matcher
    end
    super(subject, &block)
  end

  def does_not_match?(subject, &block)
    @object = subject
    if !not_supported_by_custom_matcher
      # Custom logic -- I have to raise here rather than fail in the same way I can fail using a DSL created matcher
    end
    super(subject, &block)
  end

  def setup_allowance(subject, &block)
    if !not_supported_by_custom_matcher
      # Custom logic -- I have to raise here rather than fail in the same way I can fail using a DSL created matcher
    end
    super(subject, &block)
  end

  def with(*args, &block)
    @params = *args
    super(*args, &block)
  end

  def and_return(*args, &block)
    @stubbed_value = args.first
    super(*args, &block)
  end

  def and_wrap_original(*args, &block)
    @not_supported_by_custom_matcher = true
    super(*args, &block)
  end
end

[myronmarston]

Maybe there is a way to compose the delegation logic and ensure the proper === behavior

Matchers::Matcher is just a module you can include in your class to "tag" it as a matcher intended for use with rspec-mocks:

require 'delegate'

class ReceiveWithContracts < SimpleDelegator
  include RSpec::Mocks::Matchers::Matcher

  # Put your custom logic here; all method calls will be delegated to the underlying `receive` matcher.
  # Just override the methods you need and use `super` to delegate as needed
end

That said, the matcher_allowed? method you mentioned only comes into play in a specialize circumstance that may not apply to you: the code that uses it is there for when rspec-mocks is used but rspec-expectations isn't. Normally rspec-expectations defines the expect(object).to matcher API; then, when rspec-mocks is also loaded, it hooks into that API by defining an object for receive that implements the rspec-expectations matcher API. But we wanted expect(object).to receive to be usable when rspec-mocks is used w/o rspec-expectations, and in that situation, there's a simplified alternate version of expect(object).to matcher that gets defined that only supports rspec-mocks features and not other matchers. As long as you use rspec-expectations and rspec-mocks you shouldn't have to deal with that issue at all, but including the RSpec::Mocks::Matchers::Matcher module should solve it regardless.

In the end I just inherited from the rspec receive matcher, which may not be kosher, but I think it is almost identical anyways to taking in a matcher of any type and delegating to it.

It's almost identical in terms of the behavior you've able to achieve, but it's more brittle. Consider, for example, that your custom logic and the Receive class you inherit from share the same namespace for instance variables. You've defined some for your class, including @object and @params. A future RSpec release might start using those same instance variables in the Receive matcher, and once we start doing that, your matcher will likely start having subtle bugs. The great thing about delegation is it limits what you're able to do to the public API of the object your wrapping so there's no chance of accidentally screwing up the internals of the object like there is when you subclass.

It would be nice to not raise an error and conform to whatever matcher API would let me tell callers that the match is invalid. Seems like matches? should just return a boolean, but I'm not sure if it's possible to fail in the setup_allowance and other methods as it stands.

receive is a weird hybrid object. It quacks like a matcher, but it's not really a normal matcher. Consider:

• Normal matchers only work with expect(...).to matcher, but receive has to work with allow(...).to receive, expect_any_instance_of(...).to receive and allow_any_instance_of(...).to receive. It has to do some weird things that normal matchers don't have to do to work correctly in those other contexts.
• Normal matchers just return true/false to indicate a successful match or not, but receive isn't like that at all. allow(...).to receive doesn't have any pass/fail notion (it's stubbing a method), while expect(...).to receive doesn't pass or fail now, but instead has to configure things to verify the expected message when the example completes and pass or fail at that time.

So basically, receive is nothing like a matcher, and only looks like one and "implements" the matcher API in order to let it be used with a consistent expect(...).to receive syntax so that there's API consistency for RSpec users.

[alexevanczuk]

Your points are valid -- wrapping and delegating to the existing matcher rather than pretending to be the existing matcher is definitely less brittle. However it appears the rspec matcher calls certain methods, such as setup_expectation, from within itself. This makes SimpleDelegator not work out of the box (as described in this issue: https://www.ruby-forum.com/topic/115975). This is not really an rspec related issue at this point though, and I'll try to investigate ways to handle this while getting around the delegator issue.

Good education regarding matcher's unique behavior. Sounds like there was a bit of challenge getting receive to function as a matcher and still handle all these other requirements.

Thanks again, this was all super helpful.

[Envek]

Thanks everyone for awesome discussion–the only place to get info on how to add some custom logic to existing matchers. Hope that something on this topic will be added to the documentation.

In my example I tried to add new expectation customization for additional check on when mocked object can be called. So I want to restrict calling some methods on some objects when database transaction is in progress (using gem isolator to detect it), like this:

allow(ExternalApi::GetUser).to receive(:call).outside_of_transaction.and_return({ "Name" => "Vasya" })

I need it because in our tests we're actively mocking calls to our objects wrapping some external APIs instead of mocking raw http requests (e.g. with webmock), so isolator can't detect transaction violations out of the box.

Thanks to examples posted here I was able to construct something like this (and it works):

# spec/support/matchers/outside_of_transaction.rb

module RSpec
  module Mocks
    module Matchers
      class Receive
        def outside_of_transaction
          self
        end
      end

      module NotInTransaction
        def matches?(subject, *args, &block)
          if Isolator.within_transaction?
            # We doesn't raise Isolator::UnsafeOperationError because it may be
            # rescued somewhere in subject, while MockExpectationError may not.
            @error_generator.send(:__raise, <<~MSG.squish)
              #{@error_generator.intro} received #{subject.inspect}
              while in database transaction, but it is unsafe to call it so.
            MSG
          end

          super
        end
      end
    end

    class MessageExpectation
      def outside_of_transaction
        raise_already_invoked_error_if_necessary(__method__)
        extend Matchers::NotInTransaction
        self
      end
    end
  end
end

Can it be done better? Am I missing something? Thanks!

[JonRowe]

@Envek

The recommended way is to create a custom matcher yourself, not monkey patching ours. See:
https://relishapp.com/rspec/rspec-expectations/v/3-8/docs/custom-matchers/define-a-custom-matcher

You can delegate as much or as little to our matchers within your own as you like, you can also inherit from our matchers if you must. In particular the additional "chain" style you are looking for is here:

https://relishapp.com/rspec/rspec-expectations/v/3-8/docs/custom-matchers/define-matcher-with-fluent-interface

[rahul404]

@Envek

The recommended way is to create a custom matcher yourself, not monkey patching ours. See: https://relishapp.com/rspec/rspec-expectations/v/3-8/docs/custom-matchers/define-a-custom-matcher

You can delegate as much or as little to our matchers within your own as you like, you can also inherit from our matchers if you must. In particular the additional "chain" style you are looking for is here:

https://relishapp.com/rspec/rspec-expectations/v/3-8/docs/custom-matchers/define-matcher-with-fluent-interface

The links are not working anymore, can you help me to the updated documentation?

[pirj]

https://rspec.info

[JonRowe]

e.g https://rspec.info/features/3-13/rspec-expectations/custom-matchers/define-matcher-with-fluent-interface/