Remove ModuleMethod#method_exists?
#295
Conversation
|
I'm fairly confident this can just be merged but I don't have time to look at it right now so I'm opening this PR as a reminder that I need to do something with it. |
|
This looks good to me. I'm going to remove the |
floehopper
force-pushed
the
remove-module-method-exists
branch
from
f7072eb
to
4b73f1f
Compare
|
Travis reported a failing test. I suspect it was an intermittent failure so I've requested the build to be restarted. |
@floehopper. I've had a look at this change and am fairly confident it's safe to merge if/when the Travis build passes. I'll rebase it again and get it merged. |
This is similar to the change in commit 8f58edd. Commit e87c03b made `ModuleMethod#method_exists?` redundant by changing the implementation of `ClassMethod#hide_original_method` so that it uses `ClassMethod#method_visibility` instead. This introduced a subtle change in behaviour due to the difference in `ModuleMethod#method_exists?` and `ClassMethod#method_visibility`. The former would return false if the method being stubbed was on `Kernel` or `Object` while the latter will return true in those cases. This gist illustrates the difference - https://gist.github.com/chrisroos/12a1b032b95664448c9e987132f33988. Practically, this meant that _some_ stubbed module methods (those defined on Kernel and Object) wouldn't have had the correct visibility set. The tests added in this commit ensure we don't introduce regressions in future. The tests for the visibility of the protected and private methods failed prior to commit e87c03b. I confirmed this by checking our the commit before that (`git co e87c03b~`) and running these new tests.
chrisroos
force-pushed
the
remove-module-method-exists
branch
from
4b73f1f
to
fe1d49d
Compare
|
@chrisroos Sounds good. Please go ahead and merge it. |
|
Released in v1.6.0. |
This is similar to the change in this commit.
This commit made
ModuleMethod#method_exists?redundant by changing the implementation ofClassMethod#hide_original_methodso that it usesClassMethod#method_visibilityinstead.This introduced a subtle change in behaviour due to the difference in
ModuleMethod#method_exists?andClassMethod#method_visibility. The former would return false if the method being stubbed was onKernelorObjectwhile the latter will return true in those cases. This gistillustrates the difference.
Practically, this meant that some stubbed module methods (those defined on Kernel and Object) wouldn't have had the correct visibility set. The tests added in this commit ensure we don't introduce
regressions in future. The tests for the visibility of the protected and private methods failed prior to this commit. I confirmed this by checking our the commit before that (
git co e87c03b068efc48267fbcd5a295514077c52b901~) and running these new tests.