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

@@ -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 <class 'type'> 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."""