Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix strict kwargs matching when method doesn't accept kwargs #605

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Fix strict kwargs matching when method doesn't accept kwargs #605

wants to merge 2 commits into from

Commits on Dec 31, 2022

  1. Implicitly set responder on partial mocks 

    We now automatically set a responder on mock object which are used for
partial mocks.

Having made the change above, I had to set include_all to true for the
call to Object#respond_to? in Mock#check_responder_responds_to in order
to fix a load of broken tests.

The legacy_behaviour_for_array_flatten condition in
Mock#check_responder_responds_to is needed to avoid a regression of #580
in Ruby < v2.3.

Hopefully this is a small step towards having
Configuration.prevent(:stubbing_non_existent_method) check Method#arity
and/or Method#parameters (#149) and rationalising
Configuration.stubbing_non_existent_method= & Mock#responds_like (#531).
    @floehopper
    floehopper committed Dec 31, 2022
    Configuration menu
    Copy the full SHA
    42da834 View commit details
    Browse the repository at this point in the history

  2. WIP: Relax keyword argument matching 

    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)
    @floehopper
    floehopper committed Dec 31, 2022
    Configuration menu
    Copy the full SHA
    906e106 View commit details
    Browse the repository at this point in the history