Make sure the Proxy being returned is actually for this object. #239

wants to merge 3 commits into


None yet
3 participants

I was getting failures in my helper specs (seems like the same problem mentioned here). I couldn't call a mocked method on my object right after defining it:

        helper_obj = helper
        helper_obj.stub(:blahblah).and_return( 'blah')
        helper_obj.blahblah.should == 'blah'

I eventually traced the issue:

  • helper is really an alias of view
  • When view is called ActionView::TestCase::TestController passes all its instance variables, including @mock_proxy, into the new view
  • Calls to stub were adding stubs to the proxy for the controller, not the object I was calling stub on.

I tried a fancier solution than this that hid the proxy in the eigenclass of the eigenclass but that resulted in a spec failure:

  1) RSpec::Mocks::Serialization marshals the same with and without stubbing
     Failure/Error: after = Marshal.dump(serializable_object)
       singleton can't be dumped
     # ./spec/rspec/mocks/serialization_spec.rb:99:in `block (2 levels) in <module:Mocks>'

This solution seems a little ugly but it works.

rake passes with these changes:

Finished in 0.34682 seconds
691 examples, 0 failures, 2 pending

47 scenarios (47 passed)
151 steps (151 passed)

I considered adding a test case for this but it seemed a little weird to me to be copying the proxy from one object to another in a spec. Do you think I should?

Make sure the Proxy being returned is actually for this object. This …
…is in case you have clode that blindly copies attributes from one object to another (like ActionView::TestCase::TestController does)

myronmarston commented Mar 15, 2013

I haven't worked in a rails app in a really long time...I don't understand the situation this is solving. @alindeman -- can you comment here?

I also won't merge this without specs....can you create an isolated spec (e.g. no rails) that simulates what rails is doing with the helper and view? That would help me understand this and ensure we don't have a regression if/when we merge this.


JonRowe commented Mar 16, 2013

I agree with @myronmarston, at the very least this seems like a problem with Rails (or Rspec/Rails) rather than RSpec, a spec would help clear up what the actual issue is.

The problem occurs because Rails blindly copies all instance variables from the controller into the view object.

See here, here, here, and here

This is the type of foot shooting that a fundamentally unsafe language like Ruby lets you do and needs to be guarded against.


JonRowe commented Mar 18, 2013

Ok, that makes things a bit clearer, however that implementation could do with some cleanup, at the very least i'd use the existing __remove_mock_proxy method, and I'd maybe refactor the initialisation out so you can reuse it rather than recursing. Thoughts @myronmarston?

I seems to me a one line recursive call is cleaner in this case that factoring out the initialisation


myronmarston commented Mar 19, 2013

Thanks for giving us the details, @jshraibman-mdsol. I actually just started an internal refactoring yesterday in preparation for implementing the new expect syntax we've discussed in #153 that removes the storage of the mock proxy from the stubbed object altogether, storing all mock proxy instances in RSpec::Mocks::Space in a hash keyed by object_id instead. I think the refactoring I'm doing will solve this problem w/o any special handling needed.

Even w/o the refactoring solution I'm working on, I still wonder if this problem is better solved in rspec-rails -- I don't particularly want to go to great lengths to work around libraries that copy instance variables between objects. I wonder if rails provides any API rspec-rails could hook into to prevent @mock_proxy from being copied over? (/cc @alindeman)

At the very least, I'm thinking it would be useful to have a spec in rspec-rails that repro's this problem as it exists (rather than manufacturing a similar situation by copying the ivar over as you have here), so that we can demonstrate that the refactoring I'm working on solves it.



myronmarston commented Mar 29, 2013

@jshraibman-mdsol -- I just merged #250, which removes the mock proxy from the mock'd object entirely -- now it is held by rspec mocks outside of the object. I think this solves the issue you've reported here, as there won't be an instance variable set on a controller when mocking a method on it, so rails won't have that extra ivar to copy to the view.

Want to give rspec-mocks master a try and see if it solves your problem?


JonRowe commented Mar 29, 2013

I did see you were working on that @myronmarston, good job!

Using the current head fixes my original problem.


myronmarston commented Mar 29, 2013

Glad to hear it!


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