Skip to content

Externalize methods and state that was previously held on all objects.

gitlab-qa-bot requested to merge externalize_methods_and_mock_state into master

Created by: myronmarston

Externalize methods and state that was previously held on all objects.

  • Remove @mock_proxy ivar
  • Remove rspec_verify, rspec_reset, __mock_proxy and __remove_mock_proxy methods.

This is a refactoring that I'm doing to pave the way for the new expect syntax (#153 (closed)). As that syntax "wraps" objects in order to mock or stub them, we need to externalize these methods and state. The methods directly on the object (e.g. stub and should_receive) will simply delegate to this external logic.

This refactoring had some nice side benefits:

  • No need for the hacky YAML fix. It was only needed because we were storing @mock_proxy on the mocked object.
  • AnyInstance no longer needs to much with dup; again, this was only needed because it dup'd the @mock_proxy ivar.
  • The Marshal extension gets significantly simpler (again, due to not storing @mock_proxy on the object anymore).

Rather than storing the mock proxies on the objects, we are now storing them in a hash, keyed by object_id. This is pretty simple and consistent, but could be problematic for objects that muck with object_id. That method seems a bit sacred, though, so I'm not too worried about it.

If there are concerns about the object_id approach I've used here, an alternate way we can consider is defining a singleton __mock_proxy method on an object when it is mocked or stubbed. I think I like the object_id approach better, though.

@dchelimsky / @alindeman -- I could really use your eyes on this. This is a pretty massive refactoring that potentially has far reaching effects that I haven't considered, so I would really love your feedback on this!

Merge request reports