rspec / rspec-mocks

RSpec's 'test double' framework, with support for stubbing and mocking
https://rspec.info
MIT License
1.16k stars 357 forks source link

Don't convert keywords to hash in #send for verifying doubles on Ruby 3 #1485

Closed honigc closed 1 year ago

honigc commented 1 year ago

The current re-implementation of send on verifying doubles uses *args by itself. Everything works fine on main for Ruby 2 or lower, but on Ruby 3 this converts keyword arguments to a Hash element inside the args array, which then gets passed to __send__ and then super and (correctly) causes a failure when the method gets a Hash instead of keywords.

Without this patch, the new test passes on Ruby 2.7.6 and fails on Ruby 3.1.2:

Failures:

  1) Expected argument verification (when `#with` is called) when doubling a loaded class for a method with keyword args when using `send` matches against keyword arguments
     Failure/Error: dbl.send(:m, :k => 1)

       #<InstanceDouble(#<Class:0x00007fbd16cf1ba8>) (anonymous)> received :m with unexpected arguments
         expected: ({:k=>1}) (keyword arguments)
              got: ({:k=>1}) (options hash)
       Diff:

With this patch, all tests pass on all Ruby versions.

honigc commented 1 year ago

@JonRowe Done

honigc commented 1 year ago

Updated again to use eval to set up the kwarg method in tests, to avoid syntax errors in old Rubies

JacobEvelyn commented 1 year ago

Hi @JonRowe! We're eager to see this merged—anything I can do to help move it forward?

JonRowe commented 1 year ago

Can this get rebased? That might trigger the build again

pirj commented 1 year ago

Yesterday I rebased, added a separate changelog commit and force-pushed, no build :thinking: On the Actions tab, it says "Workflow runs completed with no jobs".

JonRowe commented 1 year ago

Theres no changelog or seperate commit here, so no build... You must have pushed it to a different branch?

pirj commented 1 year ago

You must have pushed it to a different branch?

Apparently, yes :facepalm:

pirj commented 1 year ago

ERROR: Permission to panorama-ed/rspec-mocks.git denied to pirj

Might be "allow edits for maintainers" is turned off for this PR?

@honigc Do you mind ticking that checkbox please, or rebase and push again for the build to trigger?

JonRowe commented 1 year ago

Thanks for the work on this @honigc sorry it took so long to merge.