Too strong restriction for matchers

[nepalez]

Seems like the requirement only the 'receive', 'have_received' and 'receive_messages' matchers are supported with 'allow(...).to' is too restrictive.

For example, I provide the matcher, that is sort of semi-transparent extension for the receive

I believe it could work, but it doesn't because of the restriction above. Do we really need it?

Surely, I could just reload the receive method (and actually I did) and then monkey-patch the whole RSpec::Mocks::Matchers::Receive class. But also I do need to reload both the allow and expect to switch client to client.connection. It works, but seems far too messy, not to mention dark sides of monkey patching by itself.

I think, it would be far better way to allow developer to shoot himself 🔫 in the foot, not restricting him so strongly.

[nepalez]

Actually I understood that a proposed solution is not sufficient. What I do need is a way to extend receive matcher to keep it accessible to both expect and allow (my solution doesn't do this, because it explicitly calls the expect)

[myronmarston]

Exactly. Matchers in general don't work with allow. allow doesn't even use the matcher protocol, because it has to act differently than expect does. It actually calls setup_allowance on the matcher, but that's not a published public API.

I think you could get allow(client).to receive_request(:get, url) to work if you define receive_request as a method that delegates to receive internally -- which would just return the same kind of object that receive returns which allow would understand.

[nepalez]

Hi, @myronmarston, and thank you for the suggestion!

Yes, this is what I've done eventually.

Though this works, still there is one more issue. Because I've intentionally kept myself from monkey-patching the Receive class, I could not reload the initializer. It works but not as clean as it might be.

Seems like the restriction requires exact classes (RSpec::Mocks::Matchers::Receive and so on) for the allowance matchers. If it allowed subclassing too, I could remove all this mess from the #receive_request and just define my own receiver, and do something like this:

module Receiver
  def initialize(method, path = nil)
    # ...
    super # ...
  end

  def where(&block)
    # ...
  end
end

class ReceiveRequest < RSpec::Mocks::Matchers::Receive
  include Receiver
end

class HaveReceivedRequest < RSpec::Mocks::Matchers::HaveReceived
  include Receiver
end

def receive_request(*args)
  ReceiveRequest.new(*args)
end

def have_received_request(*args)
  HaveReceivedRequest.new(*args)
end

[myronmarston]

Seems like the restriction requires exact classes (RSpec::Mocks::Matchers::Receive and so on) for the allowance matchers. If it allowed subclassing too

Looking at the implementation, it looks like it does allow subclasses (using classes in a case statement will match on instances of that class or any subclass).

What makes you say it does not allow subclassing?

[nepalez]

In the example as shown above (where Evil::Client::RSpec::ReceiveRequest is a subclass of RSpec::Mocks::Matchers::Receive) I've got an exception:

only the `receive`, `have_received` and `receive_messages` matchers are supported with `allow(...).to`, but you have provided: #<Evil::Client::RSpec::ReceiveRequest:0x00000003de14f0>

[myronmarston]

Can you push up a repo containing a reproducible example?

[nepalez]

I'll try doing this a bit later, because its a part of private repo. Need some time to carefully extract it from there

[nepalez]

Well, i've decided to provide a simplest one :)

require "rspec"

describe "inheriting" do
  def receive_request(method)
    Class.new(RSpec::Mocks::Matchers::Receive).new(method, nil)
  end

  it "works" do
    client = double

    allow(client).to  receive_request(:call)
    expect(client).to receive_request(:call)

    client.call
  end
end

[myronmarston]

Thanks for the example. I see the problem now. The case statement I referred to before is used for a different situation. The check for this situation is here:

rspec-mocks/lib/rspec/mocks/targets.rb

Lines 40 to 42 in 42ef957

	def matcher_allowed?(matcher)
	matcher.class.name.start_with?("RSpec::Mocks::Matchers".freeze)
	end

...which just looks at the matcher class name (it's a simple way to ask, "is this one of the rspec-mocks matchers?")

That can certainly be changed to a more robust mechanism that allows subclasses. Also, for now you can just define your subclass as a constant nested under RSpec::Mocks::Matchers:: to work around this issue.

[myronmarston]

Also, I meant to ask: are you willing to work up a PR to fix this? It's generally hard for us to find time for things like this that only one user has asked for.