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.

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

Keyword argument warning on Ruby 2.7 for #and_call_original #1306

Closed
jaynetics opened this issue Dec 27, 2019 · 16 comments
Closed

Keyword argument warning on Ruby 2.7 for #and_call_original #1306

jaynetics opened this issue Dec 27, 2019 · 16 comments

Comments

@jaynetics
Copy link

Subject of the issue

#and_call_original with keyword arguments causes the following warning:

~/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/rspec-mocks-3.9.0/lib/rspec/mocks/message_expectation.rb:101: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call

Your environment

  • Ruby version: 2.7.0
  • rspec-expectations version: 3.9.0

Steps to reproduce

require 'rspec'

class Foo
  def initialize(bar: nil); end
end

RSpec.describe Foo do
  it 'works' do
    expect(Foo).to receive(:new).with(bar: :qux).and_call_original
    Foo.new(bar: :qux)
  end
end

RSpec::ExampleGroups::Foo.run

Expected behavior

No warning is emitted.

Actual behavior

A warning is emitted.

@JonRowe
Copy link
Member

JonRowe commented Apr 5, 2020

@jaynetics sorry it took so long but #1324 should take of this warning

@jonspalmer
Copy link

Any news on this one?

@pirj
Copy link
Member

pirj commented Jul 28, 2020

@jonspalmer Thanks for the reminder!

@G-Rath
Copy link

G-Rath commented Sep 13, 2020

Any further news on this? We'd like to get on Ruby 2.7.1 and this is the only "blocker" (not a showstopper, but would be nice to have resolved).

@bestwebua
Copy link

The same issue, any updates?

@JonRowe
Copy link
Member

JonRowe commented Sep 21, 2020

Its on the list of things the team are working on and as always PRs are welcome.

@ioquatix
Copy link

It looks like this is now broken on Ruby 3. I'm happy to take a look, but I guess I don't know what the current status is.

@pirj
Copy link
Member

pirj commented Dec 25, 2020

@ioquatix No work is done on this front yet, we're focusing on preparing 4.0 release. Would be happy to include kwargs delegation fixes along with it.

If you can spare some time, here are notable things to get you going. Most importantly, it seems that using ruby2_keywords is the only choice. See:

@JonRowe
Copy link
Member

JonRowe commented Dec 25, 2020

Work has been done on this but it slipped through the cracks, in part because we don't have specs for a lot of the keyword argument usage. Any keyword argument specs that can be provided which now fail I'll happily fix and take priority over releasing RSpec 4 so people can use Ruby 3 while we work on it.

@pirj
Copy link
Member

pirj commented Dec 29, 2020

@ioquatix Have you had a chance to take a look? No stress, we're going to pair on this with @benoittgt, just to avoid doing duplicate work.

@JonRowe
Copy link
Member

JonRowe commented Dec 29, 2020

Applying this demonstrates the issue:

--- a/spec/rspec/mocks/and_call_original_spec.rb
+++ b/spec/rspec/mocks/and_call_original_spec.rb
@@ -12,6 +12,10 @@ RSpec.describe "and_call_original" do
           yield x, :additional_yielded_arg
         end

+        if RSpec::Support::RubyFeatures.kw_args_supported?
+          class_eval("def meth_3(a: ''); a; end")
+        end
+
         def self.new_instance
           new
         end
@@ -59,6 +63,12 @@ RSpec.describe "and_call_original" do
       expect(value).to eq([:submitted_arg, :additional_yielded_arg])
     end

+    it 'supports keyword arguments' do
+      expect(instance).to receive(:meth_3).and_call_original
+      value = instance.meth_3(a: :a)
+      expect(value).to eq(:a)
+    end
+

@JonRowe
Copy link
Member

JonRowe commented Dec 29, 2020

(Or at least it generates the warning, I can't run rspec-mocks on Ruby 3.0.0 locally at the moment due to a executable-hooks 🤷 issue)

@ioquatix
Copy link

I have not had time to look into it yet so thank you for your efforts.

mpraglowski added a commit to RailsEventStore/rails_event_store that referenced this issue Dec 30, 2020
rspec-mocks is having some trouble with regard to keyword arguments
when using and_call_original: [rspec/rspec-mocks#1306](rspec/rspec-mocks#1306)

There was just 1 failing spec due to this, and we actually do
not need to call orginals in this cases.
mpraglowski added a commit to RailsEventStore/rails_event_store that referenced this issue Dec 30, 2020
rspec-mocks is having some trouble with regard to keyword arguments
when using and_call_original: [rspec/rspec-mocks#1306](rspec/rspec-mocks#1306)

There was just 1 failing spec due to this, and we actually do
not need to call orginals in this cases.
mostlyobvious pushed a commit to RailsEventStore/rails_event_store that referenced this issue Dec 30, 2020
rspec-mocks is having some trouble with regard to keyword arguments
when using and_call_original: [rspec/rspec-mocks#1306](rspec/rspec-mocks#1306)

There was just 1 failing spec due to this, and we actually do
not need to call orginals in this cases.
@swiknaba
Copy link

swiknaba commented Jan 7, 2021

double .as_null_object seems to also suffer from this problem under Ruby 3.0 --haven't had time to dig deeper yet though. Example spec: https://github.com/WeTransfer/zip_tricks/blob/v5.5.0/spec/zip_tricks/streamer_spec.rb#L526 and here the CI failure: https://github.com/WeTransfer/zip_tricks/pull/103/checks?check_run_id=1649814176#step:7:434

@JonRowe
Copy link
Member

JonRowe commented Jan 7, 2021

This specific issue is fixed by #1385.

@swiknaba can you open a new issue for anything with .as_null_object ideally with an isolated reproduction (e.g. a snippet thats just RSpec and plain Ruby) if you have the time, I don't have the time to go read your build / project source right now.

@swiknaba
Copy link

swiknaba commented Jan 7, 2021

This specific issue is fixed by #1385.

@swiknaba can you open a new issue for anything with .as_null_object ideally with an isolated reproduction (e.g. a snippet thats just RSpec and plain Ruby) if you have the time, I don't have the time to go read your build / project source right now.

Done! #1396 --for the history: the bad guy is receive, not as_null_object. sorry.

julik pushed a commit to WeTransfer/zip_tricks that referenced this issue Jan 9, 2021
✅   Locally, for Ruby <= 2.7.2 specs are all passing
❌   For Ruby 3.0 some specs are failing. All errors seem to be related to the changed behavior for kwargs
  -> **update**: see comments below!

```
Finished in 19.31 seconds (files took 0.67243 seconds to load)
123 examples, 7 failures

Failed examples:

rspec ./spec/zip_tricks/block_deflate_spec.rb:46 # ZipTricks::BlockDeflate deflate_in_blocks_and_terminate uses deflate_in_blocks
rspec ./spec/zip_tricks/block_deflate_spec.rb:58 # ZipTricks::BlockDeflate deflate_in_blocks_and_terminate passes a custom compression level
rspec ./spec/zip_tricks/block_deflate_spec.rb:87 # ZipTricks::BlockDeflate.deflate_in_blocks honors the block size
rspec ./spec/zip_tricks/rails_streaming_spec.rb:4 # ZipTricks::RailsStreaming calls the requisite controller methods
rspec ./spec/zip_tricks/streamer_spec.rb:276 # ZipTricks::Streamer writes the correct archive elements when using data descriptors
rspec ./spec/zip_tricks/streamer_spec.rb:420 # ZipTricks::Streamer prevents duplicates in the stored files
rspec ./spec/zip_tricks/streamer_spec.rb:525 # ZipTricks::Streamer writes the specified modification time
```

-> I've added a new issue to address the actual fixes for Ruby 3 in a separated PR: #104


**update**
the actual problem (for the first spec at least) is `rspec-mocks`, in particular this line: https://github.com/rspec/rspec-mocks/blob/6ab343ca479c606e892c82ce0245e7410e3f0eac/lib/rspec/mocks/message_expectation.rb#L101

```ruby
      def and_call_original
        wrap_original(__method__) do |original, *args, &block|
          original.call(*args, &block)
        end
      end
```

which will receive the following `args`:
```ruby
[#<StringIO:0x00007ff8d713bfc8>, #<StringIO:0x00007ff8d713be88>, {:level=>-1, :block_size=>65536}]
```
hence this is expanded to 3 params, 2x a string, and a hash, which is then not splatted into kwargs.
Further research shows, there is already an Issue: rspec/rspec-mocks#1306 with a fixing PR: rspec/rspec-mocks#1324
julik pushed a commit to julik/zip_kit that referenced this issue Feb 29, 2024
✅   Locally, for Ruby <= 2.7.2 specs are all passing
❌   For Ruby 3.0 some specs are failing. All errors seem to be related to the changed behavior for kwargs
  -> **update**: see comments below!

```
Finished in 19.31 seconds (files took 0.67243 seconds to load)
123 examples, 7 failures

Failed examples:

rspec ./spec/zip_tricks/block_deflate_spec.rb:46 # ZipTricks::BlockDeflate deflate_in_blocks_and_terminate uses deflate_in_blocks
rspec ./spec/zip_tricks/block_deflate_spec.rb:58 # ZipTricks::BlockDeflate deflate_in_blocks_and_terminate passes a custom compression level
rspec ./spec/zip_tricks/block_deflate_spec.rb:87 # ZipTricks::BlockDeflate.deflate_in_blocks honors the block size
rspec ./spec/zip_tricks/rails_streaming_spec.rb:4 # ZipTricks::RailsStreaming calls the requisite controller methods
rspec ./spec/zip_tricks/streamer_spec.rb:276 # ZipTricks::Streamer writes the correct archive elements when using data descriptors
rspec ./spec/zip_tricks/streamer_spec.rb:420 # ZipTricks::Streamer prevents duplicates in the stored files
rspec ./spec/zip_tricks/streamer_spec.rb:525 # ZipTricks::Streamer writes the specified modification time
```

-> I've added a new issue to address the actual fixes for Ruby 3 in a separated PR: WeTransfer/zip_tricks#104


**update**
the actual problem (for the first spec at least) is `rspec-mocks`, in particular this line: https://github.com/rspec/rspec-mocks/blob/6ab343ca479c606e892c82ce0245e7410e3f0eac/lib/rspec/mocks/message_expectation.rb#L101

```ruby
      def and_call_original
        wrap_original(__method__) do |original, *args, &block|
          original.call(*args, &block)
        end
      end
```

which will receive the following `args`:
```ruby
[#<StringIO:0x00007ff8d713bfc8>, #<StringIO:0x00007ff8d713be88>, {:level=>-1, :block_size=>65536}]
```
hence this is expanded to 3 params, 2x a string, and a hash, which is then not splatted into kwargs.
Further research shows, there is already an Issue: rspec/rspec-mocks#1306 with a fixing PR: rspec/rspec-mocks#1324
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants