Skip to content

Commit

Permalink
WIP: Relax keyword argument matching
Browse files Browse the repository at this point in the history
When a method doesn't accept keyword arguments.

In this scenario keyword or Hash-type arguments are assigned as a single
Hash to the last argument without any warnings and strict keyword
matching should not have any effect.

This is an exploratory spike on fixing #593.

* This has highlighted a significant problem with partial mocks in #532.
  The method obtained from the responder is the stub method defined by
  Mocha and not the original. This effectively makes it useless!

* I'm not sure the method_accepts_keyword_arguments? belongs on
  Invocation, but that's the most convenient place for now. It feels as
  if we need to have a bit of a sort out of where various things live
  and perhaps introduce some new classes to make things clearer.

* We might want to think ahead a bit at what we want to do in #149 to
  decide the best way to go about this.

* I'm not sure it's sensible to re-use the Equals matcher; we could
  instead parameterize PositionalOrKeywordHash, although the logic in
  there is already quite complex. Conversely if this is a good approach,
  it might make more sense to do something similar when creating a hash
  matcher for a non-last parameter to further simplify the code.

* I haven't yet introduced any acceptance tests for this and I suspect
  there might be some edge cases yet to come out of the woodwork. In
  particular, I think it's worth exhaustively working through the
  various references mentioned in this comment [1].

[1]: #593 (comment)
  • Loading branch information
floehopper committed Dec 31, 2022
1 parent 42da834 commit 906e106
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 14 deletions.
2 changes: 1 addition & 1 deletion lib/mocha/expectation.rb
Expand Up @@ -639,7 +639,7 @@ def matches_method?(method_name)

# @private
def match?(invocation)
@method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation.arguments) && @block_matcher.match?(invocation.block) && in_correct_order?
@method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation) && @block_matcher.match?(invocation.block) && in_correct_order?
end

# @private
Expand Down
9 changes: 8 additions & 1 deletion lib/mocha/invocation.rb
Expand Up @@ -8,11 +8,12 @@ module Mocha
class Invocation
attr_reader :method_name, :block

def initialize(mock, method_name, arguments = [], block = nil)
def initialize(mock, method_name, arguments = [], block = nil, responder = nil)
@mock = mock
@method_name = method_name
@arguments = arguments
@block = block
@responder = responder
@yields = []
@result = nil
end
Expand Down Expand Up @@ -62,6 +63,12 @@ def full_description
"\n - #{call_description} #{result_description}"
end

def method_accepts_keyword_arguments?
return true unless @responder

@responder.method(@method_name).parameters.any? { |k, v| %i(keyreq key keyrest).include?(k) }
end

private

def argument_description
Expand Down
2 changes: 1 addition & 1 deletion lib/mocha/mock.rb
Expand Up @@ -321,7 +321,7 @@ def method_missing(symbol, *arguments, &block)
def handle_method_call(symbol, arguments, block)
check_expiry
check_responder_responds_to(symbol)
invocation = Invocation.new(self, symbol, arguments, block)
invocation = Invocation.new(self, symbol, arguments, block, @responder)
if (matching_expectation_allowing_invocation = all_expectations.match_allowing_invocation(invocation))
matching_expectation_allowing_invocation.invoke(invocation)
elsif (matching_expectation = all_expectations.match(invocation)) || (!matching_expectation && !@everything_stubbed)
Expand Down
10 changes: 7 additions & 3 deletions lib/mocha/parameter_matchers/instance_methods.rb
Expand Up @@ -6,7 +6,7 @@ module ParameterMatchers
# @private
module InstanceMethods
# @private
def to_matcher(_expectation = nil)
def to_matcher(_expectation = nil, _method_accepts_keyword_arguments = true)
Mocha::ParameterMatchers::Equals.new(self)
end
end
Expand All @@ -21,7 +21,11 @@ class Object
# @private
class Hash
# @private
def to_matcher(expectation = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation)
def to_matcher(expectation = nil, method_accepts_keyword_arguments = true)
if method_accepts_keyword_arguments
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation)
else
Mocha::ParameterMatchers::Equals.new(self)
end
end
end
13 changes: 5 additions & 8 deletions lib/mocha/parameters_matcher.rb
Expand Up @@ -9,26 +9,23 @@ def initialize(expected_parameters = [ParameterMatchers::AnyParameters.new], exp
@matching_block = matching_block
end

def match?(actual_parameters = [])
def match?(invocation)
actual_parameters = invocation.arguments || []
if @matching_block
@matching_block.call(*actual_parameters)
else
parameters_match?(actual_parameters)
matchers(invocation).all? { |matcher| matcher.matches?(actual_parameters) } && actual_parameters.empty?
end
end

def parameters_match?(actual_parameters)
matchers.all? { |matcher| matcher.matches?(actual_parameters) } && actual_parameters.empty?
end

def mocha_inspect
signature = matchers.mocha_inspect
signature = signature.gsub(/^\[|\]$/, '')
"(#{signature})"
end

def matchers
@expected_parameters.map { |p| p.to_matcher(@expectation) }
def matchers(invocation)
@expected_parameters.map { |p| p.to_matcher(@expectation, invocation.method_accepts_keyword_arguments?) }
end
end
end

0 comments on commit 906e106

Please sign in to comment.