Stubbing a method leaves behind a copy in the singleton class

[nicklewis]

Mocha is leaving behind a copy of stubbed methods in the singleton class of the object on which the method is stubbed. So a later stub to any_instance of the class isn't seen by the original object. The following rspec test case illustrates the problem:

require 'rspec'
require 'mocha'

RSpec.configure do |config|
  config.mock_with :mocha
end

class Foo 
  def bar; 10 end 
end

foo = Foo.new

describe "stubbing instances and any_instance" do
  it "stub bar" do
    foo.stubs(:bar).returns(50)
  end 

  it "bar should be equal to 20" do
    Foo.any_instance.stubs(:bar).returns(20)
    foo.bar.should == 20
  end 
end

A work-around is to add another "test" after the one that adds the stub, which will remove the method from the singleton class.

The cause appears to be at lib/mocha/class_method.rb:54, which restores the original method into the singleton class, regardless of whether that was where it came from in the first place. I'll work on a patch if I get the time.

Incidentally, the same line of code seems to be the perpetrator of #3.

Thanks.

[floehopper]

I think there may well be some short-comings in what Mocha is doing, but in trying to reproduce the error, I noticed that by creating the instance of Foo outside the test class, you are introducing global state i.e. both tests are operating on the same instance of Foo. This is generally best avoided and although I'm not an Rspec expert, I think that if you change your code along the following lines (i.e. introducing a before method and an instance variable), you can avoid the error :-

require 'rspec'
require 'mocha'

RSpec.configure do |config|
  config.mock_with :mocha
end

class Foo 
  def bar; 10 end 
end

describe "stubbing instances and any_instance" do
  before do
    @foo = Foo.new
  end

  it "stub bar" do
    @foo.stubs(:bar).returns(50)
  end 

  it "bar should be equal to 20" do
    Foo.any_instance.stubs(:bar).returns(20)
    @foo.bar.should == 20
  end 
end

Hopefully this gives you a workaround for the moment. Let me know whether this helps and in the meantime I'll try and do some more investigation.

Thanks, James.

[nicklewis]

Thanks for the quick response.

In the simplified case given here, it is indeed easy to move the global state into the tests. Unfortunately, the object in question in our code isn't created in the test setup, and is also expensive to re-create after every test. In any case, we've worked around the problem by removing the only other place in our code where we create an instance of the class, eliminating the need for an any_instance stub at all.

[floehopper]

Hi Nick.

I've done a little bit more looking into this, although not as much as I would like. I'm not sure that your explanation for this issue is correct :-

Mocha is leaving behind a copy of stubbed methods in the singleton class of the object on which the method is stubbed.

I think that StubInstanceMethodTest and StubAnyInstanceTest both already check that this is not the case. If you can see something I'm missing, please write a similar test that demonstrates a method being left behind.

Following on from your first statement, you say :-

So a later stub to any_instance of the class isn't seen by the original object.

It is certainly true that if you stub the same method via an instance and via any_instance, then the instance stub will take precedence. But I think this makes sense from a Ruby point of view. I have added a Lighthouse ticket to improve the documentation for Mocha::ClassMethods.any_instance.

Lastly, you say :-

The cause appears to be at lib/mocha/class_method.rb:54, which restores the original method into the singleton class, regardless of whether that was where it came from in the first place.

The behaviour of this line depends on the value of stubbee. In the case of stubbing an instance method, stubbee is the instance itself; whereas in the case of stubbing via any_instance, stubbee is the class. The call to __metaclass__ is there to allow us to call alias_method and is not an indication of where the method ends up. I hope that makes some kind of sense.

Having said all the above, there does still seem to be something amiss. I'm just not sure that your explanation is correct. I'll try and find some time to do some more digging.

Thanks again for reporting the issue.

Regards, James.

[nicklewis]

I think that StubInstanceMethodTest and StubAnyInstanceTest both already check that this is not the case. If you can see something I'm missing, please write a similar test that demonstrates a method being left behind.

I have a test at the head of my fork (nicklewis/mocha@06889b5) that demonstrates this. I see there is a test in test/acceptance/github_issue_20_test.rb, but it doesn't properly characterize the bug, because it creates multiple instances of the class.

It is certainly true that if you stub the same method via an instance and via any_instance, then the instance stub will take precedence. But I think this makes sense from a Ruby point of view. I have added a Lighthouse ticket to improve the documentation for Mocha::ClassMethods.any_instance.

I agree that this behavior is correct if the instance itself and any_instance are both stubbed at the same time. In this case, however, the instance itself has been unstubbed, and the any_instance stub is still not seen.

The behaviour of this line depends on the value of stubbee. In the case of stubbing an instance method, stubbee is the instance itself; whereas in the case of stubbing via any_instance, stubbee is the class. The call to metaclass is there to allow us to call alias_method and is not an indication of where the method ends up. I hope that makes some kind of sense.

In the case where stubbee is an instance, the alias_method is called on the singleton class, and therefore an alias to the method is created in the singleton class, thus adding the method to the object's ancestry lower in the tree than the class, shadowing the version from the class. Here's a gist demonstrating what seems to be going on:

https://gist.github.com/804871

Nick

[floehopper]

Hi Nick. That looks really useful. I have a vague recollection that at one point InstanceMethod had its own implementation of restore_original_method. It might have been the victim of some over-zealous refactoring. I'll try and find some time to look at this over the next few days. Cheers, James.

[floehopper]

Hi Nick,

As you can see on the Github page for this issue, I've attempted a fix in f1d182e. It's a fairly naive and simplistic fix and has highlighted other problems in my mind, but it would be great if you can confirm that it fixes the problem you were having.

I'd rather not do a release until I've done some more work, so let me know if you need help trying out the "edge" version of Mocha. Please close the issue if you think it's fixed.

Thanks for your patience. Cheers, James.

[floehopper]

Hi Nick,

Given that I fixed the failing test (although it's now been subsumed by different tests), I'm going to assume the original problem is fixed and close this issue. Please re-open it if I'm wrong. Thanks again for reporting the problem.

Cheers, James.