Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow waiting on a mock #61215

Open
pitrou opened this issue Jan 22, 2013 · 18 comments
Open

Allow waiting on a mock #61215

pitrou opened this issue Jan 22, 2013 · 18 comments
Labels
3.13 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented Jan 22, 2013

BPO 17013
Nosy @jcea, @pitrou, @rbtcollins, @cjw296, @ezio-melotti, @voidspace, @Kentzo, @lisroach, @mariocj89, @pablogsal, @Kentzo, @tirkarthi
PRs
  • [WIP] bpo-17013: Implement WaitableMock to create Mock objects that can wait until called #12818
  • gh-61215: New mock to wait for multi-threaded events to happen #16094
  • bpo-17013: Extend Mock.called to allow waiting for calls #17133
  • gh-61215: Add Mock.call_event to allow waiting for calls #20759
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2013-01-22.12:15:47.799>
    labels = ['type-feature', 'library', '3.9']
    title = 'Allow waiting on a mock'
    updated_at = <Date 2020-06-10.14:37:15.926>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2020-06-10.14:37:15.926>
    actor = 'Kentzo'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2013-01-22.12:15:47.799>
    creator = 'pitrou'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 17013
    keywords = ['patch']
    message_count = 18.0
    messages = ['180377', '181398', '247777', '340146', '340153', '340158', '340164', '352211', '356898', '367446', '371133', '371165', '371175', '371193', '371195', '371196', '371202', '371207']
    nosy_count = 13.0
    nosy_names = ['jcea', 'pitrou', 'rbcollins', 'cjw296', 'ezio.melotti', 'michael.foord', 'Ilya.Kulakov', 'lisroach', 'mariocj89', 'lkollar', 'pablogsal', 'Kentzo', 'xtreak']
    pr_nums = ['12818', '16094', '17133', '20759']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17013'
    versions = ['Python 3.9']

    Linked PRs

    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 22, 2013

    In non-trivial tests, you may want to wait for a method to be called in another thread. This is a case where unittest.mock currently doesn't help. It would be nice to be able to write:

      myobj.some_method = Mock(side_effect=myobj.some_method)
      # launch some thread
      myobj.some_method.wait_until_called()

    And perhaps

      myobj.some_method.wait_until_called_with(...)

    (with an optional timeout?)

    If we don't want every Mock object to embed a threading.Event, perhaps there could be a ThreadedMock subclass?
    Or perhaps even:

      WaitableMock(..., event_class=threading.Event)

    so that people can pass multiprocessing.Event if they want to wait on the mock from another process?

    @pitrou pitrou added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jan 22, 2013
    @voidspace
    Copy link
    Contributor

    There is a similar feature request on the mock issue tracker: http://code.google.com/p/mock/issues/detail?id=189

    I prefer this proposal to the other one though. (Although technically allowing a wait for multiple calls is more flexible.)

    @rbtcollins
    Copy link
    Member

    Now at testing-cabal/mock#189

    @csabella csabella added the 3.8 only security fixes label Dec 12, 2017
    @tirkarthi
    Copy link
    Member

    Is there still sufficient interest in this? I gave an initial try at this based on my limited knowledge of mock and Antoine's idea. I created a new class WaitableMock that inherits from Mock and accepts an event_class with threading.Event as default and stores an event object. When call is made then via CallableMixin _mock_call is called which I have overridden to set the event object. I have wait_until_called that waits on this event object for a given timeout to return whether it's called or not.

    I am not sure of implementing wait_until_called_with since I set the event object for any call in _mock_call irrespective of argument and since the call params for which the event has to be set are passed to wait_until_called_with. Perhaps allow passing a call object to wait_until_called_with and and during _mock_call set event object only if the call is the same as one passed to wait_until_called_with ?

    See also bpo-26467 to support asyncio with mock

    Initial implementation

    class WaitableMock(Mock):
    
        def __init__(self, *args, **kwargs):
            event_class = kwargs.pop('event_class', threading.Event)
            _safe_super(WaitableMock, self).__init__(*args, **kwargs)
            self.event = event_class()
    
        def _mock_call(self, *args, **kwargs):
            _safe_super(WaitableMock, self)._mock_call(*args, **kwargs)
            self.event.set()
    
        def wait_until_called(self, timeout=1):
            return self.event.wait(timeout=timeout)

    Sample program :

    import multiprocessing
    import threading
    import time
    from unittest.mock import WaitableMock, patch
    
    def call_after_sometime(func, delay=1):
        time.sleep(delay)
        func()
    
    def foo():
        pass
    
    def bar():
        pass
    
    with patch('__main__.foo', WaitableMock(event_class=threading.Event)):
        with patch('__main__.bar', WaitableMock(event_class=threading.Event)):
            threading.Thread(target=call_after_sometime, args=(foo, 1)).start()
            threading.Thread(target=call_after_sometime, args=(bar, 5)).start()
            print("foo called ", foo.wait_until_called(timeout=2)) # successful
            print("bar called ", bar.wait_until_called(timeout=2)) # times out
            foo.assert_called_once()
            bar.assert_not_called()
            # Wait for the bar's thread to complete to verify call is registered
            time.sleep(5)
            bar.assert_called_once()

    # foo is called but waiting for call to bar times out and hence no calls to bar are registered though bar is eventually called in the end and the call is registered at the end of the program.

    ➜ cpython git:(master) ✗ time ./python.exe ../backups/bpo17013_mock.py
    foo called True
    bar called False
    ./python.exe ../backups/bpo17013_mock.py 0.40s user 0.05s system 7% cpu 5.765 total

    @mariocj89
    Copy link
    Mannequin

    mariocj89 mannequin commented Apr 13, 2019

    I think this is REALLY interesting!, there are many situations where this has been useful, it would greatly improve multithreading testing in Python.

    Q? I see you inherit from Mock, should it inherit from MagicMock?
    I'd say send the PR and the discussion can happen there about the implementation, seems there is consensus in this thread.

    I would find it OK to start with wait_until_called if you want and can be discussed in another issue/PR if you don't have the time/think it might not be worth.

    @tirkarthi
    Copy link
    Member

    Thanks Mario for the feedback.

    I see you inherit from Mock, should it inherit from MagicMock?

    yes, it can inherit from MagicMock to mock magic methods and to wait on their call.

    I thought some more about waiting for function call with arguments. One idea would be to have a dictionary with args to function as key mapping to an event object and to set and wait on that event object. To have wait_until_called that is always listening on a per mock object and to have wait_until_called_with listening on event object specific to that argument. Below is a sample implementation and an example to show it working with wait_until_called and wait_until_called_with. wait_until_called_with is something difficult to model since it needs per call event object and also need to store relevant key mapping to event object that is currently args and doesn't support keyword arguments. But wait_until_called is little simpler waiting on a per mock event object.

    I will open up a PR with some tests.

    class WaitableMock(MagicMock):
    
        def __init__(self, *args, **kwargs):
            _safe_super(WaitableMock, self).__init__(*args, **kwargs)
            self.event_class = kwargs.pop('event_class', threading.Event)
            self.event = self.event_class()
            self.expected_calls = {}
    
        def _mock_call(self, *args, **kwargs):
            ret_value  = _safe_super(WaitableMock, self)._mock_call(*args, **kwargs)
    
            for call in self._mock_mock_calls:
                event = self.expected_calls.get(call.args)
                if event and not event.is_set():
                    event.set()
    
            # Always set per mock event object to ensure the function is called for wait_until_called.
            self.event.set()
    
            return ret_value
    
        def wait_until_called(self, timeout=1):
            return self.event.wait(timeout=timeout)
    
        def wait_until_called_with(self, *args, timeout=1):
            # If there are args create a per argument list event object and if not wait for per mock event object.
            if args:
                if args not in self.expected_calls:
                    event = self.event_class()
                    self.expected_calls[args] = event
                else:
                    event = self.expected_calls[args]
            else:
                event = self.event
        return event.is_set() or event.wait(timeout=timeout)
    

    # Sample program to wait on arguments, magic methods and validating wraps

    import multiprocessing
    import threading
    import time
    from unittest.mock import WaitableMock, patch, call
    
    def call_after_sometime(func, *args, delay=1):
        time.sleep(delay)
        func(*args)
    
    def wraps(*args):
        return 1
    
    def foo(*args):
        pass
    
    def bar(*args):
        pass
    
    with patch('__main__.foo', WaitableMock(event_class=threading.Event, wraps=wraps)):
        with patch('__main__.bar', WaitableMock(event_class=threading.Event)):
            # Test normal call
            threading.Thread(target=call_after_sometime, args=(foo, 1), kwargs={'delay': 1}).start()
            threading.Thread(target=call_after_sometime, args=(bar, 1), kwargs={'delay': 5}).start()
            print("foo called ", foo.wait_until_called(timeout=2))
            print("bar called ", bar.wait_until_called(timeout=2))
            foo.assert_called_once()
            bar.assert_not_called()
        # Test wraps works
        assert foo() == 1
    
        # Test magic method
        threading.Thread(target=call_after_sometime, args=(foo.__str__, ), kwargs={'delay': 1}).start()
        print("foo.__str__ called ", foo.__str__.wait_until_called(timeout=2))
        print("bar.__str__ called ", bar.__str__.wait_until_called(timeout=2))
    
            foo.reset_mock()
            bar.reset_mock()
        # Test waiting for arguments
        threading.Thread(target=call_after_sometime, args=(bar, 1), kwargs={'delay': 1}).start()
        threading.Thread(target=call_after_sometime, args=(bar, 2), kwargs={'delay': 5}).start()
        print("bar called with 1 ", bar.wait_until_called_with(1, timeout=2))
        print("bar called with 2 ", bar.wait_until_called_with(2, timeout=2))
        time.sleep(5)
        print("bar called with 2 ", bar.wait_until_called_with(2))
    
    $ ./python.exe ../backups/bpo17013_mock.py
    foo called  True
    bar called  False
    foo.__str__ called  True
    bar.__str__ called  False
    bar called with 1  True
    bar called with 2  False
    bar called with 2  True

    @mariocj89
    Copy link
    Mannequin

    mariocj89 mannequin commented Apr 13, 2019

    Kooning great! I would Add a test for the following (I think both fails with the proposed impl):

    • The mock is called BEFORE calling wait_until_called_with
    • I call the mock with arguments and then I call wait for call without arguments.
    • using keyword arguments

    Also, I don’t have a great solution for it but it might be worth prefixing time-out with something in the wait_untill_called_with. In situations where the mocked object has a timeout parameter (which is a common argument for multithreaded apps).

    @mariocj89
    Copy link
    Mannequin

    mariocj89 mannequin commented Sep 12, 2019

    Spoke offline with @XTreak, I'll be sending a PR for this to take over the existing one.

    @lisroach proposed a new name, EventMock to differentiate it from any awaitable async notation.

    @michael.foord suggested using mock_timeout as the argument.

    Discussed also to change the naming of the method to await_until_any_call to make the semantics similar to the one that Mock provides. As await_until_called_with might mislead the user that it applies only to the last call.

    @Kentzo
    Copy link
    Mannequin

    Kentzo mannequin commented Nov 18, 2019

    I have submitted an alternative implementation of this feature heavily inspired by _AwaitEvent I wrote for asynctest [0].

    There was recently an interest from the community towards asynctest to the point that got some of its measures merged into CPython [1].

    The key features of my implementation [2]:

    • Gives meaning to the existing Mock.called property, otherwise not much useful
    • Does not require end users to do anything: change is automatically available in every Mock (and any subclass of mock that does not override called)
    • Use of conditionals provides API that allows much richer extension: instead of hardcoding conditions as events it allows to wait until arbitrary predicate becomes true
    • Utilizes existing semantics of python conditionals (both asyncio and threading)

    Accepting this PR will allow me to bring _AwaitEvent thereby completing mock.py with waiting mechanics with identical semantics for both threading-based and asyncio-based cases.

    0: https://github.com/Martiusweb/asynctest/blob/4b1284d6bab1ae90a6e8d88b882413ebbb7e5dce/asynctest/mock.py#L428
    1: #9296
    2: #17133

    @csabella csabella added 3.9 only security fixes and removed 3.8 only security fixes labels Jan 10, 2020
    @mariocj89
    Copy link
    Mannequin

    mariocj89 mannequin commented Apr 27, 2020

    For the record, I have no strong preference over either implementation. @voidspace preferred offline the new mock class, but sadly the rationale is lost in the physical word.

    @pablogsal
    Copy link
    Member

    isn't PR 20759 backwards incompatible? If I understand correctly, this would break anyone that is checking for identity as :

    assert mymock.called is True

    @Kentzo
    Copy link
    Mannequin

    Kentzo mannequin commented Jun 10, 2020

    Correct, it is not backward compatible in that respect. I did not check thoroughly, but a quick lookup shown no such use among public repos on GitHub.

    I can instead add the called_event property and make the CallEvent “public”.

    Best Regards
    Ilya Kulakov

    On Jun 9, 2020, at 11:56 PM, Pablo Galindo Salgado <report@bugs.python.org> wrote:

    
    Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

    isn't PR 20759 backwards incompatible? If I understand correctly, this would break anyone that is checking for identity as :

    assert mymock.called is True

    ----------
    nosy: +pablogsal


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue17013\>


    @pablogsal
    Copy link
    Member

    Correct, it is not backward compatible in that respect.

    Unfortunately, we take backwards compatibility very seriously in the core team and this is a big downside of this proposal.

    I can instead add the called_event property

    Wouldn't that also break any mock that is mocking an object with a "called_event" attribute?

    @Kentzo
    Copy link
    Mannequin

    Kentzo mannequin commented Jun 10, 2020

    Unfortunately, we take backwards compatibility very seriously in the core team and this is a big downside of this proposal.

    Current implementation relies on that:

    1. called is almost never used in practice (people just use .assert*)
    2. The is True / False is discouraged and is rarely used by itself, let alone in combination with .called

    Wouldn't that also break any mock that is mocking an object with a "called_event" attribute?

    It should break them in the same way as "called" breaks them now.

    @pablogsal
    Copy link
    Member

    1. called is almost never used in practice (people just use .assert*)

    Sorry but saying "almost never used" is not good enough. Not only because you hold incomplete data but because backwards compatibility is mainly binary: it breaks or it does not, and this breaks.

    1. The is True / False is discouraged and is rarely used by itself, let alone in combination with .called

    That is not true, is actually encouraged to check for singletons like True, False and None.

    @pablogsal
    Copy link
    Member

    Current implementation relies on that:

    Notice that this is a clear disadvantage over a subclass-based approach, which is backwards compatible and preserves the semantics of mock.

    @Kentzo
    Copy link
    Mannequin

    Kentzo mannequin commented Jun 10, 2020

    As far as I understand it introduces 3 methods that may clash. It's unlikely but (I speculate)
    is still more likely than an identity check with called.

    That being said, the PR can be redone as a subclass. But that implementation will not play
    as nicely with the planned awaited property for asyncio mocks (see description in the PR).

    @Kentzo
    Copy link
    Mannequin

    Kentzo mannequin commented Jun 10, 2020

    That is not true, is actually encouraged to check for singletons like True, False and None.

    You're right, just never used it as I never needed an identity check against True / False

    The PR is re-done to use an additional property call_event instead of extending called for backwards compatibility.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @arhadthedev arhadthedev added 3.13 new features, bugs and security fixes and removed 3.9 only security fixes labels Jun 10, 2023
    cjw296 pushed a commit that referenced this issue Jul 3, 2023
    mock: Add `ThreadingMock` class
    
    Add a new class that allows to wait for a call to happen by using
    `Event` objects. This mock class can be used to test and validate
    expectations of multithreading code.
    
    It uses two attributes for events to distinguish calls with any argument
    and calls with specific arguments.
    
    The calls with specific arguments need a lock to prevent two calls in
    parallel from creating the same event twice.
    
    The timeout is configured at class and constructor level to allow users
    to set a timeout, we considered passing it as an argument to the
    function but it could collide with a function parameter. Alternatively
    we also considered passing it as positional only but from an API
    caller perspective it was unclear what the first number meant on the
    function call, think `mock.wait_until_called(1, "arg1", "arg2")`, where
    1 is the timeout.
    
    Lastly we also considered adding the new attributes to magic mock
    directly rather than having a custom mock class for multi threading
    scenarios, but we preferred to have specialised class that can be
    composed if necessary. Additionally, having added it to `MagicMock`
    directly would have resulted in `AsyncMock` having this logic, which
    would not work as expected, since when if user "waits" on a
    coroutine does not have the same meaning as waiting on a standard
    call.
    
    Co-authored-by: Karthikeyan Singaravelan <tir.karthi@gmail.com>
    carljm added a commit to carljm/cpython that referenced this issue Jul 3, 2023
    * main: (167 commits)
      pythongh-91053: make func watcher tests resilient to other func watchers (python#106286)
      pythongh-104050: Add more type hints to Argument Clinic DSLParser() (python#106354)
      pythongh-106359: Fix corner case bugs in Argument Clinic converter parser (python#106361)
      pythongh-104146: Remove unused attr 'parameter_indent' from clinic.DLParser (python#106358)
      pythongh-106320: Remove private _PyErr C API functions (python#106356)
      pythongh-104050: Annotate Argument Clinic DSLParser attributes (python#106357)
      pythongh-106320: Create pycore_modsupport.h header file (python#106355)
      pythongh-106320: Move _PyUnicodeWriter to the internal C API (python#106342)
      pythongh-61215: New mock to wait for multi-threaded events to happen (python#16094)
      Document PYTHONSAFEPATH along side -P (python#106122)
      Replace the esoteric term 'datum' when describing dict comprehensions (python#106119)
      pythongh-104050: Add more type hints to Argument Clinic DSLParser() (python#106343)
      pythongh-106320: _testcapi avoids private _PyUnicode_EqualToASCIIString() (python#106341)
      pythongh-106320: Add pycore_complexobject.h header file (python#106339)
      pythongh-106078: Move DecimalException to _decimal state (python#106301)
      pythongh-106320: Use _PyInterpreterState_GET() (python#106336)
      pythongh-106320: Remove private _PyInterpreterState functions (python#106335)
      pythongh-104922: Doc: add note about PY_SSIZE_T_CLEAN (python#106314)
      pythongh-106217: Truncate the issue body size of `new-bugs-announce-notifier` (python#106329)
      pythongh-104922: remove PY_SSIZE_T_CLEAN (python#106315)
      ...
    cjw296 pushed a commit that referenced this issue Jul 4, 2023
    …106414)
    
    mock: Rename `wait_until_any_call` to `wait_until_any_call_with`
    
    Rename the method to be more explicit that it expects the args and
    kwargs to wait for.
    carljm added a commit to carljm/cpython that referenced this issue Jul 5, 2023
    * main: (39 commits)
      pythongh-102542 Remove unused bytes object and bytes slicing (python#106433)
      Clarify state of CancelledError in doc (python#106453)
      pythongh-64595: Fix regression in file write logic in Argument Clinic (python#106449)
      pythongh-104683: Rename Lib/test/clinic.test as Lib/test/clinic.test.c (python#106443)
      tp_flags docs: fix indentation (python#106420)
      pythongh-104050: Partially annotate Argument Clinic CLanguage class (python#106437)
      pythongh-106368: Add tests for formatting helpers in Argument Clinic (python#106415)
      pythongh-104050: Annotate Argument Clinic parameter permutation helpers (python#106431)
      pythongh-104050: Annotate toplevel functions in clinic.py (python#106435)
      pythongh-106320: Fix specialize.c compilation by including pycore_pylifecycle.h (python#106434)
      Add some codeowners for `Tools/clinic/` (python#106430)
      pythongh-106217: Truncate the issue body size of `new-bugs-announce-notifier` (python#106423)
      pythongh-61215: Rename `wait_until_any_call` to `wait_until_any_call_with` (python#106414)
      pythongh-106162: array: suppress warning in test_array (python#106404)
      pythongh-106320: Remove _PyInterpreterState_HasFeature() (python#106425)
      pythonGH-106360: Support very basic superblock introspection (python#106422)
      pythongh-106406: Fix _Py_IsInterpreterFinalizing() in _winapi.c (python#106408)
      pythongh-106396: Special-case empty format spec to gen empty JoinedStr node (python#106401)
      pythongh-106368: Add tests for permutation helpers in Argument Clinic (python#106407)
      pythonGH-106008: Fix refleak when peepholing `None` comparisons (python#106367)
      ...
    cjw296 pushed a commit that referenced this issue Jul 10, 2023
    threadingmock: Remove unused branch for `timeout`
    
    This is no longer needed as the mock does not hold a "timeout"
    parameter, the timeout is stored in `_mock_wait_timeout`.
    cjw296 pushed a commit that referenced this issue Jul 17, 2023
    …106822)
    
    threadingmock: Improve test suite to avoid race conditions
    
    Simplify tests and split them into multiple tests to prevent assertions
    from triggering race conditions.
    Additionally, we rely on calling the mocks without delay to validate the
    functionality of matching calls.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.13 new features, bugs and security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    Status: No status
    Development

    No branches or pull requests

    7 participants