Issue38136
Created on 2019-09-12 12:40 by lisroach, last changed 2019-09-30 04:21 by lisroach. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 16192 | merged | lisroach, 2019-09-16 16:52 | |
| PR 16431 | merged | lisroach, 2019-09-26 23:12 | |
| Messages (6) | |||
|---|---|---|---|
| msg352144 - (view) | Author: Lisa Roach (lisroach) * | Date: 2019-09-12 12:40 | |
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. |
|||
| msg352147 - (view) | Author: Karthikeyan Singaravelan (xtreak) * | Date: 2019-09-12 12:51 | |
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_*. |
|||
| msg352155 - (view) | Author: Michael Foord (michael.foord) * | Date: 2019-09-12 13:14 | |
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. |
|||
| msg352168 - (view) | Author: Lisa Roach (lisroach) * | Date: 2019-09-12 14:04 | |
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. |
|||
| msg353053 - (view) | Author: Lisa Roach (lisroach) * | Date: 2019-09-24 03:49 | |
New changeset ef048517755db1f0d211fb6dfc655a8b412cc96f by Lisa Roach in branch 'master': bpo-38136: Updates await_count and call_count to be different things (GH-16192) https://github.com/python/cpython/commit/ef048517755db1f0d211fb6dfc655a8b412cc96f |
|||
| msg353424 - (view) | Author: Lisa Roach (lisroach) * | Date: 2019-09-27 22:44 | |
New changeset 52bdd414ed9da7c62c312c542803753986a0040a 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) https://github.com/python/cpython/commit/52bdd414ed9da7c62c312c542803753986a0040a |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2019-09-30 04:21:49 | lisroach | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2019-09-27 22:44:37 | lisroach | set | messages: + msg353424 |
| 2019-09-26 23:12:27 | lisroach | set | pull_requests: + pull_request16010 |
| 2019-09-24 03:49:43 | lisroach | set | messages: + msg353053 |
| 2019-09-16 16:52:07 | lisroach | set | keywords:
+ patch stage: patch review pull_requests: + pull_request15798 |
| 2019-09-12 14:04:34 | lisroach | set | messages: + msg352168 |
| 2019-09-12 13:14:43 | michael.foord | set | messages: + msg352155 |
| 2019-09-12 12:51:13 | xtreak | set | nosy:
+ asvetlov, yselivanov, mariocj89 messages: + msg352147 |
| 2019-09-12 12:40:20 | lisroach | create | |