Always use code injection for spies and remove the ".spy" requirement.

Review Request #9517 — Created Jan. 22, 2018 and submitted

Information

kgb
master
ac20500...

Reviewers

kgb

From the beginning, kgb has had two ways of spying on functions. Bound
methods (methods on an instance, or classmethods on a class) and unbound
methods (methods on a class not yet bound to an instance) would be
temporarily replaced by the FunctionSpy, which would intercept calls
and forward things on to the destination function. This was the easiest
to handle, and was possible due to methods referencing their owner.
Standard functions don't have an owner, and any module importing them
had a reference to the function, so there was no way to just replace it.
The solution there was injecting new bytecode into the function that
recorded and forwarded calls.

With kgb 1.1 and Python 3 support, code injection changed to require
compatible function signatures, but the old method replacement support
wasn't so strict, leading to an inconsistency. Worse, though, is that
unbound methods on Python 3 could not be spied upon without breaking
calls to those methods on instances. This is due to those methods
appearing as standard functions on Python 3, and issues with code
injection on unbound methods causing contamination on bound methods on
instances.

There was also an inconsistency with how you interacted with spies. For
methods, you could call functions like method.called_with(), but for
functions you had to use func.spy.called_with().

This change reworks all of our spying to always use code injection for
methods and functions of all types, to infer whether we're working with
unbound methods on Python 3 (through the __qualname__ attribute), and
to remove the need for .spy.

By always performing code injection, we have consistency in behavior and
requirements between functions and methods. Signatures now always have
to be compatible. Callers now always see a proper function/method, and
don't see a spy.

To do this, we did have to make some changes to how we treat bound vs.
unbound methods. In both Python 2 and 3, both of these share the same
actual function behind the scenes (methods are proxies for the
function that store state and alter the call). Since we don't want code
injection on one instance to affect others, we now have to replace the
method with a copy of the method. This is similar to what we used to
have to do, but it still looks and feels like the original method,
preventing interested code from getting confused.

We also now inject spied functions with the state and methods used to
check on the results from a spy. Both methods and standard functions now
own .called_with(), .calls, etc., making for nicer testing code.

Overall, this really simplifies kgb, simplifies the expectations of
consumers, and creates better compatibility between Python 2 and 3.

Unit tests pass on Python 2.7 and 3.4.

Ran test suites for several other large projects (beanbag-tools, Djblets,
Review Board, RBCommons), and everything passed (after fixing some
function signatures).

Description From Last Updated

F841 local variable 'orig_do_math' is assigned to but never used

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
  1. Aside from the thing Review Bot found, this looks OK to me. It's big enough that I'm only maybe 80% confident in my reading of the patch, though.

  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to master (e0222d6)