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

Kernel.warn when using Forwardable delegators with a null object #1441

Open
bannable opened this issue Oct 27, 2021 · 4 comments
Open

Kernel.warn when using Forwardable delegators with a null object #1441

bannable opened this issue Oct 27, 2021 · 4 comments

Comments

@bannable
Copy link

Subject of the issue

When using Forwardable delegators, if the destination object is a spy, a kernel warning will be emitted of the form:

warning: [Class]#[method name] at [file]:[line] forwarding to private method RSpec::Mocks::InstanceVerifyingDouble#[method name]

Your environment

  • Ruby version:
  • rspec-mocks version:

Steps to reproduce

require 'forwardable'
require 'rspec/core'
require 'rspec/mocks'

class ForwardableTest
  extend Forwardable
  def_delegator :@dest, :bar
  def initialize(dest)
    @dest = dest
  end

  def foo
    bar
  end
end

class Injected
  def bar; true; end
end

RSpec.describe 'fowardable behavior' do
  subject { ForwardableTest.new(thing).bar }

  context 'with an object instance' do
    let(:thing) { Injected.new }

    specify { expect { subject }.not_to output.to_stderr }
  end

  context 'with a null object (spy)' do
    let(:thing) { instance_spy('Injected') }

    specify { expect { subject }.not_to output.to_stderr }
  end
end

Expected behavior

Both examples will pass.

Actual behavior

fowardable behavior with a null object (spy) is expected not to output to stderr fails.

@pirj
Copy link
Member

pirj commented Oct 27, 2021

It seems that this:

  def foo
    bar
  end

can be removed, and the error still reproduces.

@JonRowe
Copy link
Member

JonRowe commented Oct 30, 2021

Adding an expected result solves this, such as:

instance_spy('Injected', bar: true)

So I think this is an artefact from the forward, as instance doubles only verify methods which are stubbed, where as the spy mode calls as_null_object where everything returns self, so is more like a method missing call, I think forward is complaining about that

@pirj
Copy link
Member

pirj commented Oct 31, 2021

By looking at the implementation, it doesn't look like Forwardable's warning is fair ([1], [2]). I'm not certain if it's Forwardable or instance_spy to blame.

I guess that spy is accepting and records every method call with method_missing. But does it define respond_to_missing? to declare that it does so? Is this sufficient for Forwardable to work and skip this warning?
Are you interested in getting to the bottom of this?

I'd start with an attempt to reproduce the warning with a class defining dynamic methods used as a delegator target.

@JonRowe
Copy link
Member

JonRowe commented Nov 2, 2021

Our doubles respond_to? true for this case, it seems Forwardable checks theres an actual method with this warning, we don't define that unless you specifically mock out the method, I don't think we want to change that behaviour, and given there is an easy alternative, I'm inclined to close this as a "not something we can fix"

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

No branches or pull requests

3 participants