Closed jcredding closed 10 years ago
@jcredding after looking at this again, I think we should add some system tests for stubbing for sure - either here or preferably in a new PR.
@jcredding I have a couple implementation questions to ask - will probably wanna wait and do those in person - cool?
@kellyredding - Yep, we can discuss in person.
@jcredding after discussing more with you I'm good with this :-1:
This fixes a rare bug with stubbing a method on a parent class and on a child class that inherits from the parent class in the same test. This caused order-dependent bugs when the stubs were tore down. If the parent was stubbed and tore down before the child, then the child tear down would fail because the parent would have already removed the method it wanted to remove. This fixes the issue by taking steps to ensure the parent and child don't share stub methods:
method_missing
. This ensures we don't alias an inherited method, which will alter the object the method is inherited from, but leaves all the existing features working.class_eval
and a string instead ofdefine_method
. This ensures the stub method is not created with an arity of1
which causes arity checks to fail. This properly creates a method with an arity of-1
.class_eval
, the stub instance needs to be stored on the object being stubbed. When the stub method is called, it will look up the stub instance and call it like it previously did. This sets and removes an ivar in the setup and teardown to support this.All of this fixes issues with stubbing inherited methods in the same test. The typical use-case for this is stubbing
new
on a parent class and a child class. Since all classes havenew
this was the most common scenario for this error to show up.@kellyredding - Ready for review. I tried to explain all the changes and why they were needed. Sorry for the lengthy commit message. I haven't added any system tests and didn't want to hold this up while I worked out what all scenarios I wanted to test (there are more scenarios, previously I hadn't considered the
method_missing
cases until it started failing on me in the test suite). I have a regression test to ensure this specific error doesn't come back.