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

Remove AsyncMock.assert_awaited_* #82317

Closed
lisroach opened this issue Sep 12, 2019 · 6 comments
Closed

Remove AsyncMock.assert_awaited_* #82317

lisroach opened this issue Sep 12, 2019 · 6 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@lisroach
Copy link
Contributor

BPO 38136
Nosy @cjw296, @ezio-melotti, @voidspace, @asvetlov, @1st1, @lisroach, @mariocj89, @tirkarthi
PRs
  • bpo-38136: Updates await_count and call_count to be different things #16192
  • [3.8] bpo-38136: Updates await_count and call_count to be different things (GH-16192) #16431
  • 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 = 'https://github.com/lisroach'
    closed_at = <Date 2019-09-30.04:21:49.191>
    created_at = <Date 2019-09-12.12:40:20.364>
    labels = ['3.8', 'type-bug', 'library', '3.9']
    title = 'Remove AsyncMock.assert_awaited_*'
    updated_at = <Date 2019-09-30.04:21:49.191>
    user = 'https://github.com/lisroach'

    bugs.python.org fields:

    activity = <Date 2019-09-30.04:21:49.191>
    actor = 'lisroach'
    assignee = 'lisroach'
    closed = True
    closed_date = <Date 2019-09-30.04:21:49.191>
    closer = 'lisroach'
    components = ['Library (Lib)']
    creation = <Date 2019-09-12.12:40:20.364>
    creator = 'lisroach'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38136
    keywords = ['patch']
    message_count = 6.0
    messages = ['352144', '352147', '352155', '352168', '353053', '353424']
    nosy_count = 8.0
    nosy_names = ['cjw296', 'ezio.melotti', 'michael.foord', 'asvetlov', 'yselivanov', 'lisroach', 'mariocj89', 'xtreak']
    pr_nums = ['16192', '16431']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue38136'
    versions = ['Python 3.8', 'Python 3.9']

    @lisroach
    Copy link
    Contributor Author

    After some discussion about call_count vs await_count, I believe call_count should be counting when things are *awaited* (await foo()), not when they are *called* (foo()).

    I think people expect "calling" to execute the code and give them a return_value, which for asyncio is what happens when you await, not when you call with (). If people disagree about this I am open to discussion, we can change the current functionality and leave in the assert_awaited_* calls.

    Currently the code does count asyncio calls when they are awaited, but this makes the assert_awaited_* calls redundant.

    We should remove these in favor of the call_count_* functions.

    @lisroach lisroach added 3.8 only security fixes 3.9 only security fixes labels Sep 12, 2019
    @lisroach lisroach self-assigned this Sep 12, 2019
    @lisroach lisroach added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 12, 2019
    @tirkarthi
    Copy link
    Member

    IMO it gives a good segregation between other mocks and AsyncMock that using assert_awaited_* makes it explicit and to cause less confusion over whether this is an awaitable or a synchronous mock. I would favor in keeping the API. I also found it to better in conceptualizing while writing tests for other PRs over having assert_call_* and assert_await_*.

    @voidspace
    Copy link
    Contributor

    I'm particularly concerned that we have call_count "sane" for AsyncMock and avoid adding "await_count" if possible. I think we've decided on that.

    I'm more agnostic on the assert_await* methods. I agree they're a nice API. I don't mind those staying but I also don't mind them going.

    @lisroach
    Copy link
    Contributor Author

    Going to try to recap an in-person conversation:

    There are some cases where calls are made separate from when they are awaited, for example:
       >>> call = foo()
       >>> await call

    This would be 1 call and 1 await:

    Call List Await List
    --------- ----------
    [foo] [call]

    Calls like:
       >>> await foo()

    Should also be counted as 1 call and 1 await, there is no difference between this call and the call above (expect for slight differences in when the lists are updated):

    Call List Await List
    --------- ----------
    [foo] [call]

    If someone were to do this:

    >>>call_1 = foo()
    >>>call_2 = foo()

    >>> await call_1
    >>> await foo(x)

    We should see 2 calls added to the call list, then 1 await added to the await list, then 1 call added to the call list and 1 await added to the await list. We would end up with 3 calls and 2 awaits.

    Call List Await List
    --------- ----------
    [foo, foo, foo] [call_1, foo]

    And a call without an await:
     
      >>> call = foo()

    Call List Await List
    --------- ----------
    [foo] []

    With a setup like this, we would keep the API the same (leaving in the assert_await*). There is some risk that users will incorrectly be using assert_call* when they really want to test awaits, but we can try to make to docs as clear as possible around this.

    @lisroach
    Copy link
    Contributor Author

    New changeset ef04851 by Lisa Roach in branch 'master':
    bpo-38136: Updates await_count and call_count to be different things (GH-16192)
    ef04851

    @lisroach
    Copy link
    Contributor Author

    New changeset 52bdd41 by Lisa Roach in branch '3.8':
    [3.8] bpo-38136: Updates await_count and call_count to be different things (GH-16192) (GH-16431)
    52bdd41

    @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 stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants