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

await_args_list in AsyncMock always refers to the last awaited call object #84096

Closed
MadsSejersen mannequin opened this issue Mar 9, 2020 · 6 comments
Closed

await_args_list in AsyncMock always refers to the last awaited call object #84096

MadsSejersen mannequin opened this issue Mar 9, 2020 · 6 comments
Labels
3.8 only security fixes 3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@MadsSejersen
Copy link
Mannequin

MadsSejersen mannequin commented Mar 9, 2020

BPO 39915
Nosy @cjw296, @voidspace, @asvetlov, @1st1, @lisroach, @mariocj89, @miss-islington, @tirkarthi
PRs
  • bpo-39915: Ensure await_args_list is updated according to the order in which coroutines were awaited #18924
  • [3.8] bpo-39915: Ensure await_args_list is updated according to the order in which coroutines were awaited (GH-18924) #18927
  • Files
  • fail.py
  • 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 = <Date 2020-03-14.07:17:26.326>
    created_at = <Date 2020-03-09.16:29:08.544>
    labels = ['type-bug', '3.8', '3.9', 'expert-asyncio']
    title = 'await_args_list in AsyncMock always refers to the last awaited call object'
    updated_at = <Date 2020-03-14.07:17:26.320>
    user = 'https://bugs.python.org/MadsSejersen'

    bugs.python.org fields:

    activity = <Date 2020-03-14.07:17:26.320>
    actor = 'xtreak'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-03-14.07:17:26.326>
    closer = 'xtreak'
    components = ['asyncio']
    creation = <Date 2020-03-09.16:29:08.544>
    creator = 'Mads Sejersen'
    dependencies = []
    files = ['48963']
    hgrepos = []
    issue_num = 39915
    keywords = ['patch']
    message_count = 6.0
    messages = ['363748', '363815', '363817', '363928', '364147', '364148']
    nosy_count = 9.0
    nosy_names = ['cjw296', 'michael.foord', 'asvetlov', 'yselivanov', 'lisroach', 'mariocj89', 'miss-islington', 'xtreak', 'Mads Sejersen']
    pr_nums = ['18924', '18927']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39915'
    versions = ['Python 3.8', 'Python 3.9']

    @MadsSejersen
    Copy link
    Mannequin Author

    MadsSejersen mannequin commented Mar 9, 2020

    When calling asyncio.gather the await_args_list is not correct. In the attached minimal example it contains only the latest call and not each of the three actual calls.

    Expected output:
    [call(0), call(1), call(2)]
    [call(1), call(2), call(3)] # or any permutation hereof

    Actual output:
    [call(0), call(1), call(2)]
    [call(3), call(3), call(3)]

    @MadsSejersen MadsSejersen mannequin added 3.8 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error labels Mar 9, 2020
    @MadsSejersen
    Copy link
    Mannequin Author

    MadsSejersen mannequin commented Mar 10, 2020

    It can actually be boiled down to an even more minimal example. It looks like the problem is that the function call is stored for later when called, then overwritten by other subsequent calls. Then, once awaited, the latest registered call is added to the await_args_list instead of the call that actually happened.

    import asyncio
    from unittest.mock import AsyncMock
    
    
    async def main():
        foo = AsyncMock()
    
        foo1 = foo(1)
        foo2 = foo(2)
        await foo1
        await foo2
        print(foo.await_args_list)
    
    
    asyncio.run(main())
    

    @tirkarthi
    Copy link
    Member

    Thanks for the report. At [0] self.call_args is used. This value is always the last call. So when the object is awaited it will be always the last call object and gets appended. The id of the call objects remain the same. The fix would be to construct the call object with args and kwargs in asynchronous definition of _execute_mock_call like [1]. This would need tests too. It seems there is only test for one await call and assertion over await_args_list. Having different args and kwargs with multiple await to assert await_args_list would help here. In synchronous case the call object is eagerly appended so it won't be an issue.

    [0]

    _call = self.call_args

    [1]
    _call = _Call((args, kwargs), two=True)

    A potential patch would be as below :

    diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
    index 9e692981a2..20daf724c1 100644
    --- a/Lib/unittest/mock.py
    +++ b/Lib/unittest/mock.py
    @@ -2171,7 +2171,7 @@ class AsyncMockMixin(Base):
             # This is nearly just like super(), except for special handling
             # of coroutines
     
    -        _call = self.call_args
    +        _call = _Call((args, kwargs), two=True)
             self.await_count += 1
             self.await_args = _call
             self.await_args_list.append(_call)

    @tirkarthi tirkarthi changed the title AsyncMock doesn't work with asyncio.gather await_args_list in AsyncMock always refers to the last awaited call object Mar 10, 2020
    @tirkarthi tirkarthi changed the title AsyncMock doesn't work with asyncio.gather await_args_list in AsyncMock always refers to the last awaited call object Mar 10, 2020
    @cjw296
    Copy link
    Contributor

    cjw296 commented Mar 11, 2020

    New changeset e553f20 by Karthikeyan Singaravelan in branch 'master':
    bpo-39915: Ensure await_args_list is updated according to the order in which coroutines were awaited (GH-18924)
    e553f20

    @cjw296
    Copy link
    Contributor

    cjw296 commented Mar 14, 2020

    New changeset f6bdac1 by Miss Islington (bot) in branch '3.8':
    bpo-39915: Ensure await_args_list is updated according to the order in which coroutines were awaited (GH-18927)
    f6bdac1

    @tirkarthi
    Copy link
    Member

    Thanks Mads Sejersen for the report. Closing this as fixed as it's merged to 3.9 and 3.8.

    @tirkarthi tirkarthi added the 3.9 only security fixes label Mar 14, 2020
    @tirkarthi tirkarthi added the 3.9 only security fixes label Mar 14, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants