)]}'
{"nova/tests/spy.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bceb7e7fe162737734256c6489890116b1f4aa7c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        we want to spy on."},{"line_number":41,"context_line":"    :param before: A callable that will be called each time before the original"},{"line_number":42,"context_line":"        target function is called. The before callable\u0027s call signature is"},{"line_number":43,"context_line":"        before(spy, *original_arg, **original_kwargs) where spy is the spy"},{"line_number":44,"context_line":"        object decorating the original function, *original_args and"},{"line_number":45,"context_line":"        **original_kwargs are the actual parameters of the original call on the"},{"line_number":46,"context_line":"        target function. The passed in spy object can be used to store data"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_24cf872a","line":43,"range":{"start_line":43,"start_character":21,"end_line":43,"end_character":33},"updated":"2019-10-01 20:12:18.000000000","message":"original_args","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"70f510cfbfdeef001db7af5432ba37524b5b5553","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        we want to spy on."},{"line_number":41,"context_line":"    :param before: A callable that will be called each time before the original"},{"line_number":42,"context_line":"        target function is called. The before callable\u0027s call signature is"},{"line_number":43,"context_line":"        before(spy, *original_arg, **original_kwargs) where spy is the spy"},{"line_number":44,"context_line":"        object decorating the original function, *original_args and"},{"line_number":45,"context_line":"        **original_kwargs are the actual parameters of the original call on the"},{"line_number":46,"context_line":"        target function. The passed in spy object can be used to store data"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_9cba4bdf","line":43,"range":{"start_line":43,"start_character":21,"end_line":43,"end_character":33},"in_reply_to":"3fa7e38b_24cf872a","updated":"2019-10-02 10:21:17.000000000","message":"Done","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"2711d7bb4b746016e78f8686b92616cf18c8414e","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        target function is returned. The after callable\u0027s call signature is"},{"line_number":53,"context_line":"        after(spy, result, *original_arg, **original_kwargs) where spy is the"},{"line_number":54,"context_line":"        spy object decorating the original function, result is the return value"},{"line_number":55,"context_line":"        of the original call, *original_args and **original_kwargs are the"},{"line_number":56,"context_line":"        actual parameters of the original call on the target function. The"},{"line_number":57,"context_line":"        passed in spy object can be used similarly as in the before callable."},{"line_number":58,"context_line":"    :return: A context manager yielding the spy object decorating the original"},{"line_number":59,"context_line":"    target function."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_de5d2549","line":56,"range":{"start_line":55,"start_character":30,"end_line":56,"end_character":69},"updated":"2019-10-01 15:30:58.000000000","message":"This is going to bite us in the ass so hard the day we use it on a function that mutates its arguments, like all of the live migration functions that pass around migrate_data, or the update_xml stuff.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"f859a57612e113382148733cda9b292e68964db5","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        target function is returned. The after callable\u0027s call signature is"},{"line_number":53,"context_line":"        after(spy, result, *original_arg, **original_kwargs) where spy is the"},{"line_number":54,"context_line":"        spy object decorating the original function, result is the return value"},{"line_number":55,"context_line":"        of the original call, *original_args and **original_kwargs are the"},{"line_number":56,"context_line":"        actual parameters of the original call on the target function. The"},{"line_number":57,"context_line":"        passed in spy object can be used similarly as in the before callable."},{"line_number":58,"context_line":"    :return: A context manager yielding the spy object decorating the original"},{"line_number":59,"context_line":"    target function."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_8bca0e18","line":56,"range":{"start_line":55,"start_character":30,"end_line":56,"end_character":69},"in_reply_to":"3fa7e38b_39ea53ac","updated":"2019-10-01 16:55:23.000000000","message":"\u003e Is it just about the naming? So `original` is too specific? I guess\n \u003e we can rename those to simply args and kwargs and explain in the\n \u003e doc that these are the args that is sent to the original function\n \u003e in case of the `before` callable and these are the args after the\n \u003e original returned (including any mutation by the original\n \u003e function), in case of the `after` callable.\n\nYep, thanks for crystallizing my own thoughts :)","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"70f510cfbfdeef001db7af5432ba37524b5b5553","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        target function is returned. The after callable\u0027s call signature is"},{"line_number":53,"context_line":"        after(spy, result, *original_arg, **original_kwargs) where spy is the"},{"line_number":54,"context_line":"        spy object decorating the original function, result is the return value"},{"line_number":55,"context_line":"        of the original call, *original_args and **original_kwargs are the"},{"line_number":56,"context_line":"        actual parameters of the original call on the target function. The"},{"line_number":57,"context_line":"        passed in spy object can be used similarly as in the before callable."},{"line_number":58,"context_line":"    :return: A context manager yielding the spy object decorating the original"},{"line_number":59,"context_line":"    target function."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_bcbf07cf","line":56,"range":{"start_line":55,"start_character":30,"end_line":56,"end_character":69},"in_reply_to":"3fa7e38b_8bca0e18","updated":"2019-10-02 10:21:17.000000000","message":"Done","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6b5f56aa5d12a9f3edb4f2465a3c6259508a78c9","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        target function is returned. The after callable\u0027s call signature is"},{"line_number":53,"context_line":"        after(spy, result, *original_arg, **original_kwargs) where spy is the"},{"line_number":54,"context_line":"        spy object decorating the original function, result is the return value"},{"line_number":55,"context_line":"        of the original call, *original_args and **original_kwargs are the"},{"line_number":56,"context_line":"        actual parameters of the original call on the target function. The"},{"line_number":57,"context_line":"        passed in spy object can be used similarly as in the before callable."},{"line_number":58,"context_line":"    :return: A context manager yielding the spy object decorating the original"},{"line_number":59,"context_line":"    target function."}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_39ea53ac","line":56,"range":{"start_line":55,"start_character":30,"end_line":56,"end_character":69},"in_reply_to":"3fa7e38b_de5d2549","updated":"2019-10-01 16:32:38.000000000","message":"I\u0027m not following you on the biting part. Our spy passes the actual parameters sent to the original function to the `before` call and after the original function returned (and maybe mutated it\u0027s actual parameters) the spy passes the actual parameters to the `after` call. Why does it bit us? \n\n// later\n\nIs it just about the naming? So `original` is too specific? I guess we can rename those to simply args and kwargs and explain in the doc that these are the args that is sent to the original function in case of the `before` callable and these are the args after the original returned (including any mutation by the original function), in case of the `after` callable.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"2711d7bb4b746016e78f8686b92616cf18c8414e","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    original \u003d getattr(target, func_name)"},{"line_number":62,"context_line":"    _spy \u003d _generate_spy(original, before, after)"},{"line_number":63,"context_line":"    try:"},{"line_number":64,"context_line":"        # decorate the original with the _spy"},{"line_number":65,"context_line":"        setattr(target, func_name, _spy)"},{"line_number":66,"context_line":"        yield _spy"},{"line_number":67,"context_line":"    finally:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f90d9b35","line":64,"range":{"start_line":64,"start_character":10,"end_line":64,"end_character":18},"updated":"2019-10-01 15:30:58.000000000","message":"nit: not really \"decorate\", \"replace\" would be more like it.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"f859a57612e113382148733cda9b292e68964db5","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    original \u003d getattr(target, func_name)"},{"line_number":62,"context_line":"    _spy \u003d _generate_spy(original, before, after)"},{"line_number":63,"context_line":"    try:"},{"line_number":64,"context_line":"        # decorate the original with the _spy"},{"line_number":65,"context_line":"        setattr(target, func_name, _spy)"},{"line_number":66,"context_line":"        yield _spy"},{"line_number":67,"context_line":"    finally:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_6b60f236","line":64,"range":{"start_line":64,"start_character":10,"end_line":64,"end_character":18},"in_reply_to":"3fa7e38b_d9cd3f87","updated":"2019-10-01 16:55:23.000000000","message":"See, I was thinking of the spy as not just the wrapper, but the whole thing - wrapper + original. Your way of thinking about it is more technically correct though - the spy is just the wrapper.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6b5f56aa5d12a9f3edb4f2465a3c6259508a78c9","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    original \u003d getattr(target, func_name)"},{"line_number":62,"context_line":"    _spy \u003d _generate_spy(original, before, after)"},{"line_number":63,"context_line":"    try:"},{"line_number":64,"context_line":"        # decorate the original with the _spy"},{"line_number":65,"context_line":"        setattr(target, func_name, _spy)"},{"line_number":66,"context_line":"        yield _spy"},{"line_number":67,"context_line":"    finally:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_d9cd3f87","line":64,"range":{"start_line":64,"start_character":10,"end_line":64,"end_character":18},"in_reply_to":"3fa7e38b_f90d9b35","updated":"2019-10-01 16:32:38.000000000","message":"The _spy wraps the original function and we are replacing the original here with the _spy (i.e. with the wrapper), so this is technically doing the same as what a python decorator does.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bceb7e7fe162737734256c6489890116b1f4aa7c","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    _spy \u003d _generate_spy(original, before, after)"},{"line_number":63,"context_line":"    try:"},{"line_number":64,"context_line":"        # decorate the original with the _spy"},{"line_number":65,"context_line":"        setattr(target, func_name, _spy)"},{"line_number":66,"context_line":"        yield _spy"},{"line_number":67,"context_line":"    finally:"},{"line_number":68,"context_line":"        # remove the decoration"},{"line_number":69,"context_line":"        setattr(target, func_name, original)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_84cafb17","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":44},"updated":"2019-10-01 20:12:18.000000000","message":"instead of what\u0027s effectively a manual monkey patch, why aren\u0027t you using one of the patch methods from the mock library?\n\n with mock.patch.object(target, func_name, _spy):\n     yield _spy","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3c5b064e5cc89d73cf965cf3293e8e6375ccdd17","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    _spy \u003d _generate_spy(original, before, after)"},{"line_number":63,"context_line":"    try:"},{"line_number":64,"context_line":"        # decorate the original with the _spy"},{"line_number":65,"context_line":"        setattr(target, func_name, _spy)"},{"line_number":66,"context_line":"        yield _spy"},{"line_number":67,"context_line":"    finally:"},{"line_number":68,"context_line":"        # remove the decoration"},{"line_number":69,"context_line":"        setattr(target, func_name, original)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_965eaae7","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":44},"in_reply_to":"3fa7e38b_84cafb17","updated":"2019-10-02 09:43:45.000000000","message":"I tried using mocks in the implementation of the spy and failed. There is some complication mocking functions versus mocking bound methods. When we want to call the original which was a bound method then we need \u0027self\u0027 but the mock lib does not pass that to the side_effect function. But I haven\u0027t tried this specific way, especially the use of mock(new\u003d) in this context.\n\n// later\n\nunit test passes with your way so I will go and redo this based on the suggestion.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"70f510cfbfdeef001db7af5432ba37524b5b5553","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    _spy \u003d _generate_spy(original, before, after)"},{"line_number":63,"context_line":"    try:"},{"line_number":64,"context_line":"        # decorate the original with the _spy"},{"line_number":65,"context_line":"        setattr(target, func_name, _spy)"},{"line_number":66,"context_line":"        yield _spy"},{"line_number":67,"context_line":"    finally:"},{"line_number":68,"context_line":"        # remove the decoration"},{"line_number":69,"context_line":"        setattr(target, func_name, original)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_5cb4d3f2","line":69,"range":{"start_line":65,"start_character":8,"end_line":69,"end_character":44},"in_reply_to":"3fa7e38b_965eaae7","updated":"2019-10-02 10:21:17.000000000","message":"Done","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"376a723ed43fee174995bad86b5e9466f539cbb7","unresolved":false,"context_lines":[{"line_number":10,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":"from contextlib import contextmanager"},{"line_number":13,"context_line":"from functools import wraps"},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_2af50984","line":13,"range":{"start_line":13,"start_character":22,"end_line":13,"end_character":27},"updated":"2019-10-02 13:11:01.000000000","message":"import only modules (import functools and use @functools.wraps below)","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c916a57c7fd13afc794d4b6886261d153f531b9","unresolved":false,"context_lines":[{"line_number":10,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":"from contextlib import contextmanager"},{"line_number":13,"context_line":"from functools import wraps"},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_2568da1f","line":13,"range":{"start_line":13,"start_character":22,"end_line":13,"end_character":27},"in_reply_to":"3fa7e38b_2af50984","updated":"2019-10-02 13:49:18.000000000","message":"I don\u0027t like that rule but that is the rule so I will fix this.","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"376a723ed43fee174995bad86b5e9466f539cbb7","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    @wraps(original)"},{"line_number":20,"context_line":"    def _spy(*args, **kwargs):"},{"line_number":21,"context_line":"        if before:"},{"line_number":22,"context_line":"            before(_spy, *args, **kwargs)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        res \u003d original(*args, **kwargs)"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_8a299df3","line":22,"range":{"start_line":22,"start_character":19,"end_line":22,"end_character":23},"updated":"2019-10-02 13:11:01.000000000","message":"This is bizarre, and tbh I\u0027m not sure how it\u0027s even working, as _spy doesn\u0027t really exist yet. Assuming it does (it must!) is it the function or the functools.partial generated by @wraps? In either case it\u0027s an odd thing to give the caller access to.\n\nIIUC the only reason you\u0027re returning a thing to the caller is as a container for them to store stuff in, that persists from `before` to `after`. If that\u0027s the case, IMO it would be better to make a clean, empty object just for that purpose. A mock.Mock() would work, because then you can autovivify properties on it. Or even just a dict.\n\nI think scope-wise you would need to declare it within _generate_spy but outside of _spy, so it persists across multiple invocations of the wrapped method.\n\nAnd since it\u0027s important that the caller have access to the container from within the `with` context, you would have to return it along with _spy on L31 so it can be the thing you yield on L71.","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a57ee578340f091fac96764d7184affe20eef30e","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    @wraps(original)"},{"line_number":20,"context_line":"    def _spy(*args, **kwargs):"},{"line_number":21,"context_line":"        if before:"},{"line_number":22,"context_line":"            before(_spy, *args, **kwargs)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        res \u003d original(*args, **kwargs)"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_0b728f7d","line":22,"range":{"start_line":22,"start_character":19,"end_line":22,"end_character":23},"in_reply_to":"3fa7e38b_107cfa1e","updated":"2019-10-02 14:52:01.000000000","message":"\u003e\u003e\u003e def f():\n...     pass\n... \n\u003e\u003e\u003e dir(f)\n[\u0027__call__\u0027, \u0027__class__\u0027, \u0027__closure__\u0027, \u0027__code__\u0027, \u0027__defaults__\u0027, \u0027__delattr__\u0027, \u0027__dict__\u0027, \u0027__doc__\u0027, \u0027__format__\u0027, \u0027__get__\u0027, \u0027__getattribute__\u0027, \u0027__globals__\u0027, \u0027__hash__\u0027, \u0027__init__\u0027, \u0027__module__\u0027, \u0027__name__\u0027, \u0027__new__\u0027, \u0027__reduce__\u0027, \u0027__reduce_ex__\u0027, \u0027__repr__\u0027, \u0027__setattr__\u0027, \u0027__sizeof__\u0027, \u0027__str__\u0027, \u0027__subclasshook__\u0027, \u0027func_closure\u0027, \u0027func_code\u0027, \u0027func_defaults\u0027, \u0027func_dict\u0027, \u0027func_doc\u0027, \u0027func_globals\u0027, \u0027func_name\u0027]\n\u003e\u003e\u003e \n\nSo even if that is partial it does not look different from a function :)","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c747b01954c41dbce5416e18750cb14c913988d4","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    @wraps(original)"},{"line_number":20,"context_line":"    def _spy(*args, **kwargs):"},{"line_number":21,"context_line":"        if before:"},{"line_number":22,"context_line":"            before(_spy, *args, **kwargs)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        res \u003d original(*args, **kwargs)"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_107cfa1e","line":22,"range":{"start_line":22,"start_character":19,"end_line":22,"end_character":23},"in_reply_to":"3fa7e38b_4582b602","updated":"2019-10-02 14:19:27.000000000","message":"\u003e why this works:\n\nYeah, I understand.\n\n \u003e Sure I can create another object in the local scope of\n \u003e _generate_spy and pass that around but why should I do that if\n \u003e there is already a local object, the _spy function itself.\n\nBecause the _spy function is a function (actually I\u0027m still convinced it\u0027s a functools.partial masquerading as a function) which has properties and semantics the caller may not be expecting.\n\n (Pdb) dir(spy_thing)\n [\u0027__call__\u0027, \u0027__class__\u0027, \u0027__closure__\u0027, \u0027__code__\u0027, \u0027__defaults__\u0027, \n \u0027__delattr__\u0027, \u0027__dict__\u0027, \u0027__doc__\u0027, \u0027__format__\u0027, \u0027__get__\u0027, \n \u0027__getattribute__\u0027, \u0027__globals__\u0027, \u0027__hash__\u0027, \u0027__init__\u0027, \u0027__module__\u0027, \n \u0027__name__\u0027, \u0027__new__\u0027, \u0027__reduce__\u0027, \u0027__reduce_ex__\u0027, \u0027__repr__\u0027, \n \u0027__setattr__\u0027, \u0027__sizeof__\u0027, \u0027__str__\u0027, \u0027__subclasshook__\u0027, \u0027func_closure\u0027, \n \u0027func_code\u0027, \u0027func_defaults\u0027, \u0027func_dict\u0027, \u0027func_doc\u0027, \u0027func_globals\u0027, \n \u0027func_name\u0027]\n\n\nIt would take some pretty weird machinations to break it, I\u0027ll admit. But if you did do that by accident, it would be really really hard to figure out what happened.","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3659350f97ea7133f44efbfb016d4e90be27185a","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    @wraps(original)"},{"line_number":20,"context_line":"    def _spy(*args, **kwargs):"},{"line_number":21,"context_line":"        if before:"},{"line_number":22,"context_line":"            before(_spy, *args, **kwargs)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        res \u003d original(*args, **kwargs)"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_e5ee42b3","line":22,"range":{"start_line":22,"start_character":19,"end_line":22,"end_character":23},"in_reply_to":"3fa7e38b_8a299df3","updated":"2019-10-02 13:42:29.000000000","message":"\u003e clean, empty object just for that purpose. A mock.Mock() would\n \u003e work, because then you can autovivify properties on it. Or even\n \u003e just a dict.\n\nI played with this, and a mock.Mock() changes the way the caller has to use the thing. That might still be a reasonable option, but what I ended up with [1] is arguably cleaner (a Mock() has semantics that aren\u0027t necessarily desirable in this environment) and doesn\u0027t change the usage.\n\n[1] https://review.opendev.org/686167","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c916a57c7fd13afc794d4b6886261d153f531b9","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    @wraps(original)"},{"line_number":20,"context_line":"    def _spy(*args, **kwargs):"},{"line_number":21,"context_line":"        if before:"},{"line_number":22,"context_line":"            before(_spy, *args, **kwargs)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        res \u003d original(*args, **kwargs)"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_4582b602","line":22,"range":{"start_line":22,"start_character":19,"end_line":22,"end_character":23},"in_reply_to":"3fa7e38b_8a299df3","updated":"2019-10-02 13:49:18.000000000","message":"why this works:\n* When the spy module is imported the body of the module is evaluated so the _generate_spy is defined\n* When _generate_spy is called at L67 the body of _generate_spy is evaluated. First it defines a new function  called _spy then replace it with the return value of wraps(original)(_spy) which is still the _spy function just __name__ and __doc__ set based on original\n* then _spy is returned\n\n* when later _spy is called its body is evaluated in the enclosing scope the _spy function was originally defined in. So in the local scope of _generate_spy. In that scope _spy is defined so the body of _spy can happily access it. It is important that each and every _generate_spy defines a _new_ _spy function in its local scope and the body of the _spy function will see itself. So two _spy function bodies see two separate function objects. So they cannot interfere.\n\nThis is basically where the scope magic is moved. In the functional test cases we use a list in an outer scope[1] to store data between fake calls. Now this outer scope is the local scope of _generate_spy. \n\nSure I can create another object in the local scope of _generate_spy and pass that around but why should I do that if there is already a local object, the _spy function itself.\n\n[1] https://github.com/openstack/nova/blob/bf37bec80baa527ac013dfaa7480ef2761ed2cb9/nova/tests/functional/regressions/test_bug_1843090.py#L84","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d3fb4a99d3e8b44d7c0df6c776518dab3dd748f8","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    @wraps(original)"},{"line_number":20,"context_line":"    def _spy(*args, **kwargs):"},{"line_number":21,"context_line":"        if before:"},{"line_number":22,"context_line":"            before(_spy, *args, **kwargs)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        res \u003d original(*args, **kwargs)"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_8560ae37","line":22,"range":{"start_line":22,"start_character":19,"end_line":22,"end_character":23},"in_reply_to":"3fa7e38b_e5ee42b3","updated":"2019-10-02 13:48:57.000000000","message":"Another option would be to allow the caller to create its own data object. Then instead of this\n\n def spy_prep_resize(spy, _self, *args, **kwargs):\n     # awkward:\n     spy.calls_on_hosts \u003d getattr(spy, \u0027calls_on_hosts\u0027, [])\n     spy.calls_on_hosts.append(_self.host)\n     ...\n\n with spy(\n         compute_manager.ComputeManager, \u0027_prep_resize\u0027,\n         before\u003dspy_prep_resize) as prep_resize_spy:\n\nyou could do something like\n\n tracker \u003d dict(calls_on_hosts\u003d[])\n\n def spy_prep_resize(spy, _self, *args, **kwargs):\n     spy[\u0027calls_on_hosts\u0027].append(_self.host)\n     ...\n\n with spy(\n         compute_manager.ComputeManager, \u0027_prep_resize\u0027,\n         before\u003dspy_prep_resize, data\u003dtracker):","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"376a723ed43fee174995bad86b5e9466f539cbb7","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    # NOTE(gibi): cannot use side_effect here as that mysteriously breaks the"},{"line_number":69,"context_line":"    # call args of the original call if the target function is a bound method"},{"line_number":70,"context_line":"    with mock.patch.object(target, func_name, new\u003d_spy):"},{"line_number":71,"context_line":"        yield _spy"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_6a030175","line":71,"range":{"start_line":71,"start_character":14,"end_line":71,"end_character":18},"updated":"2019-10-02 13:11:01.000000000","message":"It\u0027s a little bit weird that you\u0027re yielding a functools.partial object. That could have bizarre side effects if, for example, the caller tries to set its \u0027args\u0027 attribute.\n\nSee above...","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c916a57c7fd13afc794d4b6886261d153f531b9","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    # NOTE(gibi): cannot use side_effect here as that mysteriously breaks the"},{"line_number":69,"context_line":"    # call args of the original call if the target function is a bound method"},{"line_number":70,"context_line":"    with mock.patch.object(target, func_name, new\u003d_spy):"},{"line_number":71,"context_line":"        yield _spy"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_0587deb8","line":71,"range":{"start_line":71,"start_character":14,"end_line":71,"end_character":18},"in_reply_to":"3fa7e38b_6a030175","updated":"2019-10-02 13:49:18.000000000","message":"I thin it is not a special thing I return here. It is a function. And it is the same function at every step:\n\n(\u0027before gen returns\u0027, \u003cfunction method at 0x7fe58ce58950\u003e)\n(\u0027before yield\u0027, \u003cfunction method at 0x7fe58ce58950\u003e)\n(\u0027during spying\u0027, \u003cfunction method at 0x7fe58ce58950\u003e)\n(\u0027during spying\u0027, \u003cfunction method at 0x7fe58ce58950\u003e)","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"}],"nova/tests/unit/test_spy.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"2711d7bb4b746016e78f8686b92616cf18c8414e","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        with spy.spy(TargetClass, \u0027method\u0027, before\u003dspy_before_raises):"},{"line_number":99,"context_line":"            # spy breaks the call"},{"line_number":100,"context_line":"            self.assertRaises(ValueError, t.method, \u0027foo\u0027)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # now the spy is removed so the call succeeds"},{"line_number":103,"context_line":"        self.assertEqual(1, t.method(\u0027foo\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f93b5bc5","line":100,"updated":"2019-10-01 15:30:58.000000000","message":"nit: might want to add an after_spy and assert that it\u0027s not called here.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"70f510cfbfdeef001db7af5432ba37524b5b5553","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        with spy.spy(TargetClass, \u0027method\u0027, before\u003dspy_before_raises):"},{"line_number":99,"context_line":"            # spy breaks the call"},{"line_number":100,"context_line":"            self.assertRaises(ValueError, t.method, \u0027foo\u0027)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # now the spy is removed so the call succeeds"},{"line_number":103,"context_line":"        self.assertEqual(1, t.method(\u0027foo\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_3c79d72e","line":100,"in_reply_to":"3fa7e38b_cb4fa632","updated":"2019-10-02 10:21:17.000000000","message":"Done","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6b5f56aa5d12a9f3edb4f2465a3c6259508a78c9","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        with spy.spy(TargetClass, \u0027method\u0027, before\u003dspy_before_raises):"},{"line_number":99,"context_line":"            # spy breaks the call"},{"line_number":100,"context_line":"            self.assertRaises(ValueError, t.method, \u0027foo\u0027)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # now the spy is removed so the call succeeds"},{"line_number":103,"context_line":"        self.assertEqual(1, t.method(\u0027foo\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_cb4fa632","line":100,"in_reply_to":"3fa7e38b_f93b5bc5","updated":"2019-10-01 16:32:38.000000000","message":"Good point. I will do that.","commit_id":"c9ff3d8009de9dc7c6e7732825ab8c14ae01229f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6b6008e7ad16febc857bb3b6c9d076eef81ed2e6","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        def spy_after(spy, result, *args, **kwargs):"},{"line_number":99,"context_line":"            self.fail("},{"line_number":100,"context_line":"                \u0027The after callable should not be called if before is raised\u0027)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        with spy.spy("},{"line_number":103,"context_line":"                TargetClass, \u0027method\u0027, before\u003dspy_before_raises,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa7e38b_0aa12d96","line":100,"range":{"start_line":100,"start_character":67,"end_line":100,"end_character":76},"updated":"2019-10-02 12:29:41.000000000","message":"raises","commit_id":"5f60e0494b377b22ad49e745dbca682276719f47"}]}