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

Fix warning from Forwardable for Mock #785

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sovetnik
Copy link

@sovetnik sovetnik commented Feb 7, 2019

Ruby 2.4 warns when delegating to a private method with Forwardable: https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/56210.

When delegating to a Minitest::Mock object with Forwardable, a "forwarding to private method" warning is shown.

For example, given:

require "forwardable"

class Foo
 extend Forwardable

 attr_reader :baz

 def_delegator :baz, :bar

 def initialize(baz)
   @baz = baz
 end
end

require "minitest/autorun"

class FooTest < Minitest::Spec
 let(:baz) { MiniTest::Mock.new }
 let(:foo) { Foo.new(baz) }

 before do
   baz.expect(:bar, 'foobar')
 end

 it 'produce warnings' do
   expect(foo.bar).must_equal 'foobar'
 end
end

The test will output:

# Running:

warning.rb:26: warning: Foo#bar at warning.rb:8 forwarding to private method Minitest::Mock#bar
.

Finished in 0.000593s, 1686.3410 runs/s, 1686.3410 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips

Ideally these warnings would not be shown, because in most cases the expectation is set on a public method.

There explained why:

When you define method_missing method, you should define respond_to_missing? method too.

require "forwardable"

class Foo
 extend Forwardable

 attr_reader :baz

 def_delegator :baz, :bar

 def initialize(baz)
   @baz = baz
 end
end

require "minitest/autorun"

class MiniTest::Mock
 def respond_to_missing?(symbol, include_private = false)
   @expected_calls.key? symbol || super
 end
end

class FooTest < Minitest::Spec
 let(:baz) { MiniTest::Mock.new }
 let(:foo) { Foo.new(baz) }

 before do
   baz.expect(:bar, 'foobar')
 end

 it 'produce warnings' do
   expect(foo.bar).must_equal 'foobar'
 end
end

So, if we implement respond_to_missing? as recommended, warning disappered.

@zenspider
Copy link
Collaborator

Can you convert your example above into a test please?

@sovetnik
Copy link
Author

sovetnik commented Aug 8, 2019

Yes, I’ll try

@zenspider zenspider self-assigned this Aug 24, 2019
@zenspider
Copy link
Collaborator

anything?

@singpolyma
Copy link

In Ruby 2.5 this solution does not seem to get rid of the warning for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants