From 78527ff5ab5452b5d91c361d040f3f31469b37ec Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Thu, 7 Apr 2016 14:26:45 -0400 Subject: [PATCH] Update the semantics on _fixtures.MonkeyPatch A previous change added some logic so that when monkeypatching a staticmethod the old_value was restored as a staticmethod. There was an issue where the determination of whether or not a method should be static was incorrectly checking the new function not the one to be replaced. That caused an unintended side-effect that in order to patch an instance method of a class the new function needed to be an instance method of a class. This change now reworks the semantics of MonkeyPatch to address that issue and at the same time be explicit about how it should work in a large number of different cases. The rule is simple and provides great flexibility. Given a callable bar to be patched on to foo bar will be called with any bound arguments first and then arguments of foo appended. This is easier to visualize. Given: class C(object): @classmethod def foo(cls, arg): pass class D(object): @classmethod def bar(cls, tgtcls, arg): pass def baz(cls, arg): pass MonkeyPatch('...C.foo', D.bar) will result in C.foo(1) calling bar like bar(D, C, 1) because cls on bar was already bound to D when patching. And MonkeyPatch('...C.foo', baz) will result in baz being called with baz(C, 1). --- fixtures/_fixtures/monkeypatch.py | 54 ++- fixtures/tests/_fixtures/test_monkeypatch.py | 373 ++++++++++++++++++- 2 files changed, 408 insertions(+), 19 deletions(-) diff --git a/fixtures/_fixtures/monkeypatch.py b/fixtures/_fixtures/monkeypatch.py index 4198b4f..e63f7be 100644 --- a/fixtures/_fixtures/monkeypatch.py +++ b/fixtures/_fixtures/monkeypatch.py @@ -23,21 +23,52 @@ from fixtures import Fixture -def _setattr(obj, name, value): - """Handle some corner cases when calling setattr. +def _coerce_values(obj, name, new_value, sentinel): + """Handle value coercion for static and classmethods. - setattr transforms a function into instancemethod, so where appropriate - value needs to be wrapped with staticmethod(). + setattr transforms a function into an instancemethod when set on a class. + This checks if the attribute to be replaced is either and wraps new_value + if necessary. This also checks getattr(obj, name) and wraps it if necessary + since the staticmethod wrapper isn't preserved. """ + old_value = getattr(obj, name, sentinel) + if sys.version_info[0] == 2: class_types = (type, types.ClassType) else: # All classes are in Python 3 class_types = type - if (isinstance(obj, class_types) and - isinstance(value, types.FunctionType)): - value = staticmethod(value) - setattr(obj, name, value) + + if not isinstance(obj, class_types): + # Nothing special to do here + return (new_value, old_value) + + # getattr() returns a function, this access pattern will return a + # staticmethod/classmethod if the name method is defined that way + old_attribute = obj.__dict__.get(name) + if old_attribute is not None: + old_value = old_attribute + + # If new_value is not callable it is potentially wrapped with staticmethod + # or classmethod so grab the underlying function. If it has no underlying + # callable thing the following coercion can be skipped, just return. + if not callable(new_value): + if hasattr(new_value, '__func__'): + new_value = new_value.__func__ + else: + return (new_value, old_value) + + if isinstance(old_value, staticmethod): + new_value = staticmethod(new_value) + if isinstance(old_value, classmethod): + new_value = classmethod(new_value) + if isinstance(old_value, types.FunctionType): + captured_method = new_value + def func_wrapper(*args, **kwargs): + return captured_method(*args, **kwargs) + new_value = func_wrapper + + return (new_value, old_value) class MonkeyPatch(Fixture): @@ -72,16 +103,17 @@ def _setUp(self): for component in components[1:]: current = getattr(current, component) sentinel = object() - old_value = getattr(current, attribute, sentinel) + new_value, old_value = _coerce_values(current, attribute, + self.new_value, sentinel) if self.new_value is self.delete: if old_value is not sentinel: delattr(current, attribute) else: - _setattr(current, attribute, self.new_value) + setattr(current, attribute, new_value) if old_value is sentinel: self.addCleanup(self._safe_delete, current, attribute) else: - self.addCleanup(_setattr, current, attribute, old_value) + self.addCleanup(setattr, current, attribute, old_value) def _safe_delete(self, obj, attribute): """Delete obj.attribute handling the case where its missing.""" diff --git a/fixtures/tests/_fixtures/test_monkeypatch.py b/fixtures/tests/_fixtures/test_monkeypatch.py index 81927c8..d1e0af7 100644 --- a/fixtures/tests/_fixtures/test_monkeypatch.py +++ b/fixtures/tests/_fixtures/test_monkeypatch.py @@ -13,6 +13,8 @@ # license you chose for the specific language governing permissions and # limitations under that license. +import functools + import testtools from fixtures import MonkeyPatch, TestWithFixtures @@ -20,9 +22,42 @@ reference = 23 class C(object): + def foo(self, arg): + return arg + @staticmethod + def foo_static(): pass + @classmethod + def foo_cls(cls): pass + +class D(object): + def bar(self): pass + def bar_two_args(self, arg): + return (self, arg) + @classmethod + def bar_cls(cls): + return cls + @classmethod + def bar_cls_args(cls, *args): + return tuple([cls] + [arg for arg in args]) + @staticmethod + def bar_static(): + pass @staticmethod - def foo(): pass -def bar(): pass + def bar_static_args(*args): + return args + def bar_self_referential(self, *args, **kwargs): + self.bar() + +INST_C = C() + +def fake(*args): + return args +def fake2(arg): pass +def fake_no_args(): pass +def fake_no_args2(): pass +@staticmethod +def fake_static(): pass + class TestMonkeyPatch(testtools.TestCase, TestWithFixtures): @@ -72,14 +107,336 @@ def test_delete_missing_attribute(self): fixture.cleanUp() self.assertFalse('new_attr' in globals()) - def test_patch_staticmethod(self): - oldfoo = C.foo + def _check_restored_static_or_class_method(self, oldmethod, oldmethod_inst, + klass, methodname): + restored_method = getattr(klass, methodname) + restored_method_inst = getattr(klass(), methodname) + self.assertEqual(oldmethod, restored_method) + self.assertEqual(oldmethod, restored_method_inst) + self.assertEqual(oldmethod_inst, restored_method) + self.assertEqual(oldmethod_inst, restored_method_inst) + restored_method() + restored_method_inst() + + def test_patch_staticmethod_with_staticmethod(self): + oldmethod = C.foo_static + oldmethod_inst = C().foo_static + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_static', + D.bar_static) + with fixture: + C.foo_static() + C().foo_static() + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_static') + + def test_patch_staticmethod_with_classmethod(self): + oldmethod = C.foo_static + oldmethod_inst = C().foo_static + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_static', + D.bar_cls) + with fixture: + C.foo_static() + C().foo_static() + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_static') + + def test_patch_staticmethod_with_function(self): + oldmethod = C.foo_static + oldmethod_inst = C().foo_static + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_static', + fake_no_args) + with fixture: + C.foo_static() + C().foo_static() + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_static') + + def test_patch_staticmethod_with_boundmethod(self): + oldmethod = C.foo_static + oldmethod_inst = C().foo_static + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_static', + D().bar) + with fixture: + C.foo_static() + C().foo_static() + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_static') + + def test_patch_classmethod_with_staticmethod(self): + oldmethod = C.foo_cls + oldmethod_inst = C().foo_cls + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_cls', + D.bar_static_args) + with fixture: + (cls,) = C.foo_cls() + self.assertTrue(issubclass(cls, C)) + (cls,) = C().foo_cls() + self.assertTrue(issubclass(cls, C)) + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_cls') + + def test_patch_classmethod_with_classmethod(self): + oldmethod = C.foo_cls + oldmethod_inst = C().foo_cls + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_cls', + D.bar_cls_args) + with fixture: + cls, tgtcls = C.foo_cls() + self.assertTrue(issubclass(cls, D)) + self.assertTrue(issubclass(tgtcls, C)) + cls, tgtcls = C().foo_cls() + self.assertTrue(issubclass(cls, D)) + self.assertTrue(issubclass(tgtcls, C)) + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_cls') + + def test_patch_classmethod_with_function(self): + oldmethod = C.foo_cls + oldmethod_inst = C().foo_cls + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_cls', + fake) + with fixture: + (cls,) = C.foo_cls() + self.assertTrue(issubclass(cls, C)) + (cls, arg) = C().foo_cls(1) + self.assertTrue(issubclass(cls, C)) + self.assertEqual(1, arg) + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_cls') + + def test_patch_classmethod_with_boundmethod(self): + oldmethod = C.foo_cls + oldmethod_inst = C().foo_cls + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_cls', + D().bar_two_args) + with fixture: + slf, cls = C.foo_cls() + self.assertTrue(isinstance(slf, D)) + self.assertTrue(issubclass(cls, C)) + slf, cls = C().foo_cls() + self.assertTrue(isinstance(slf, D)) + self.assertTrue(issubclass(cls, C)) + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_cls') + + def test_patch_function_with_staticmethod(self): + oldmethod = fake_no_args + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.fake_no_args', + D.bar_static) + with fixture: + fake_no_args() + + self.assertEqual(oldmethod, fake_no_args) + + def test_patch_function_with_classmethod(self): + oldmethod = fake_no_args + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.fake_no_args', + D.bar_cls) + with fixture: + fake_no_args() + + self.assertEqual(oldmethod, fake_no_args) + + def test_patch_function_with_function(self): + oldmethod = fake_no_args + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.fake_no_args', + fake_no_args2) + with fixture: + fake_no_args() + + self.assertEqual(oldmethod, fake_no_args) + + def test_patch_function_with_partial(self): + oldmethod = fake_no_args + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.fake_no_args', + functools.partial(fake, 1)) + with fixture: + (ret,) = fake_no_args() + self.assertEqual(1, ret) + + self.assertEqual(oldmethod, fake_no_args) + + def test_patch_function_with_boundmethod(self): + oldmethod = fake_no_args + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.fake_no_args', + D().bar) + with fixture: + fake_no_args() + + self.assertEqual(oldmethod, fake_no_args) + + def test_patch_boundmethod_with_staticmethod(self): + oldmethod = INST_C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.INST_C.foo', + D.bar_static) + with fixture: + INST_C.foo() + + self.assertEqual(oldmethod, INST_C.foo) + + def test_patch_boundmethod_with_classmethod(self): + oldmethod = INST_C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.INST_C.foo', + D.bar_cls) + with fixture: + INST_C.foo() + + self.assertEqual(oldmethod, INST_C.foo) + + def test_patch_boundmethod_with_function(self): + oldmethod = INST_C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.INST_C.foo', + fake_no_args) + with fixture: + INST_C.foo() + + self.assertEqual(oldmethod, INST_C.foo) + + def test_patch_boundmethod_with_boundmethod(self): + oldmethod = INST_C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.INST_C.foo', + D().bar) + with fixture: + INST_C.foo() + + self.assertEqual(oldmethod, INST_C.foo) + + def test_patch_unboundmethod_with_staticmethod(self): + oldmethod = C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo', + D.bar_static_args) + with fixture: + tgtslf, arg = C().foo(1) + self.assertTrue(isinstance(tgtslf, C)) + self.assertEqual(1, arg) + + self.assertEqual(oldmethod, C.foo) + + def test_patch_unboundmethod_with_classmethod(self): + oldmethod = C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo', + D.bar_cls_args) + with fixture: + cls, tgtslf, arg = C().foo(1) + self.assertTrue(issubclass(cls, D)) + self.assertTrue(isinstance(tgtslf, C)) + self.assertEqual(1, arg) + + self.assertEqual(oldmethod, C.foo) + + def test_patch_unboundmethod_with_function(self): + oldmethod = C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo', + fake) + with fixture: + tgtslf, arg = C().foo(1) + self.assertTrue(isinstance(tgtslf, C)) + self.assertTrue(1, arg) + + self.assertEqual(oldmethod, C.foo) + + def test_patch_unboundmethod_with_boundmethod(self): + oldmethod = C.foo + fixture = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo', + D().bar_two_args) + with fixture: + slf, tgtslf = C().foo() + self.assertTrue(isinstance(slf, D)) + self.assertTrue(isinstance(tgtslf, C)) + + self.assertEqual(oldmethod, C.foo) + + def test_double_patch_instancemethod(self): + oldmethod = C.foo + oldmethod_inst = C().foo + fixture1 = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo', + fake) + fixture2 = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo', + fake2) + with fixture1: + with fixture2: + C().foo() + + self.assertEqual(oldmethod, C.foo) + # The method address changes with each instantiation of C, and method + # equivalence just tests that. Compare the code objects instead. + self.assertEqual(oldmethod_inst.__code__, C().foo.__code__) + + def test_double_patch_staticmethod(self): + oldmethod = C.foo_static + oldmethod_inst = C().foo_static + fixture1 = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_static', + fake_no_args) + fixture2 = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_static', + fake_static) + with fixture1: + with fixture2: + C.foo_static() + C().foo_static() + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_static') + + def test_double_patch_classmethod(self): + oldmethod = C.foo_cls + oldmethod_inst = C().foo_cls + fixture1 = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_cls', + fake) + fixture2 = MonkeyPatch( + 'fixtures.tests._fixtures.test_monkeypatch.C.foo_cls', + fake2) + with fixture1: + with fixture2: + C.foo_cls() + C().foo_cls() + + self._check_restored_static_or_class_method(oldmethod, oldmethod_inst, + C, 'foo_cls') + + def test_patch_c_foo_with_instance_d_bar_self_referential(self): + oldmethod = C.foo + oldmethod_inst = C().foo fixture = MonkeyPatch( 'fixtures.tests._fixtures.test_monkeypatch.C.foo', - bar) + D().bar_self_referential) with fixture: - C.foo() C().foo() - self.assertEqual(oldfoo, C.foo) - self.assertEqual(oldfoo, C().foo) + self.assertEqual(oldmethod, C.foo) + # The method address changes with each instantiation of C, and method + # equivalence just tests that. Compare the code objects instead. + self.assertEqual(oldmethod_inst.__code__, C().foo.__code__)