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

Support for async functions with mock_callable() #19

Closed
fornellas opened this issue Apr 23, 2019 · 6 comments · Fixed by #83
Closed

Support for async functions with mock_callable() #19

fornellas opened this issue Apr 23, 2019 · 6 comments · Fixed by #83
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@fornellas
Copy link
Contributor

fornellas commented Apr 23, 2019

Support for async functions with mock_callable().

Here's a half working patch for that, as a starting point.

To prove that any solution works, all relevant tests at tests/mock_callable_testslide.py must be updated to to cover async cases (eg: async function at module, async instance method, async class method etc).

PS: previous PR #8

@fornellas fornellas added enhancement New feature or request help wanted Extra attention is needed labels Apr 23, 2019
@david-caro
Copy link
Contributor

Hi @fornellas, I have a question about this, in order to support asyncio, the whole module would have to drop python2 support right?

@fornellas
Copy link
Contributor Author

I suppose you're correct. Python 2 is dying anyway, it's support can be dropped at this point.

@fornellas
Copy link
Contributor Author

PS: I created a milestone for async stuff https://github.com/facebookincubator/TestSlide/milestone/1, as there's more stuff to implement.

@fornellas
Copy link
Contributor Author

@fried, what do you think about the approach of using inspect.iscoroutinefunction to detect if the callable (module function, instance/class/static method) is async, and keep the self.mock_callable( interface? The other option would be to have self.mock_async_callable(. I understand that inspect has its limitations, I'm not experience with async to judge if this might lead to users shooting themselves in the foot in those cases.

@jivid
Copy link

jivid commented Oct 27, 2019

I've put up #60 to provide mock_async_callable. I do believe having an explicit mock_async_callable is better than having the mock_callable interface detect whether it's received a coroutine or a regular function. The latter just feels like it would hide too much from the users.

@fried
Copy link
Member

fried commented Nov 4, 2019

unittest.IsolatedAsyncioTestCase in 3.8 uses that. So to have async test methods they have to explicitly be coroutines. No sync methods that return awaitables, since that's pretty hairy.
Also the AsyncMock in 3.8 is also a really good interface for async mocks, in that it looks at the spec and if its a coroutine the mocks that return when that is called is another AsyncMock, otherwise you get a normal MagicMock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants