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

Add HTTPX support #91

Merged
merged 1 commit into from
Dec 28, 2023
Merged

Add HTTPX support #91

merged 1 commit into from
Dec 28, 2023

Conversation

sarayourfriend
Copy link
Collaborator

Closes #71.

HTTPX documentation is here: https://www.python-httpx.org/

The current implementation supports everything, except HTTP2.

At first, I tried to implement this on the httpcore level, figuring that it'd be nice to go ahead and support anything that depended on httpcore. I still think that would be a good way to approach this, perhaps even supplanting the httpx-specific approach. However, httpcore's connection interfaces are significantly more complex to mock, if only because whereas httpx has only two entrypoints, httpcore technically has 8, an HTTP1, HTTP1.1 and HTTP2, and a connection pool connection for both sync and async requests (luckily all the supported proxy request types are implemented over top the other four and wouldn't require their own interceptors). Additionally, it wasn't entirely clear to me whether merely mocking the "handle request" method was sufficient to prevent outbound network connections for all 8 of these connection types.

For the sake of having support for HTTPX at least, and not holding it back by trying to support a broader, I chose to stick to just supporting HTTPX for the time being. The implementation luckily turned out to be pretty simple, and I took a helpful queue from the excellent https://github.com/Colin-b/pytest_httpx, specifically the idea to mock the _transport_for_url method, which prevents any of the httpcore complexity from leaking into this feature.

@tekumara
Copy link

Thanks @sarayourfriend! I've tried #91 and run into an error when trying to use @pook.on with @pytest.mark.asyncio, eg:

@pook.on
@pytest.mark.asyncio
async def test_pook():
    pass

errors with

    @functools.wraps(func)
    def inner(**kwargs):
        coro = func(**kwargs)
        if coro is not None:
>           task = asyncio.ensure_future(coro, loop=_loop)

.venv/lib/python3.11/site-packages/pytest_asyncio/plugin.py:193: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/homebrew/Cellar/python@3.11/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/tasks.py:659: in ensure_future
    return _ensure_future(coro_or_future, loop=loop)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

coro_or_future = <async_generator object test_pook at 0x1075b2e40>

    def _ensure_future(coro_or_future, *, loop=None):
        if futures.isfuture(coro_or_future):
            if loop is not None and loop is not futures._get_loop(coro_or_future):
                raise ValueError('The future belongs to a different loop than '
                                'the one specified as the loop argument')
            return coro_or_future
        called_wrap_awaitable = False
        if not coroutines.iscoroutine(coro_or_future):
            if inspect.isawaitable(coro_or_future):
                coro_or_future = _wrap_awaitable(coro_or_future)
                called_wrap_awaitable = True
            else:
>               raise TypeError('An asyncio.Future, a coroutine or an awaitable '
                                'is required')
E               TypeError: An asyncio.Future, a coroutine or an awaitable is required

This doesn't happen with pook 1.1.1

@sarayourfriend
Copy link
Collaborator Author

sarayourfriend commented Dec 28, 2023

Thanks @tekumara! I need to rebase this branch, I believe it would fix the issue with the coroutine issue you're seeing. The problem was that the wrapped async function in async_activate was an async generator rather than a coroutine before, and this caused issues with the pook.on decorator on async tests.

Here is the code that was fixed:

async def wrapper(*args, **kw):
_engine.activate()
try:
if iscoroutinefunction(fn):
return await fn(*args, **kw)
else:
fn(*args, **kw)
finally:
_engine.disable()

Previously the wrapped function used the pre-async def notation of yielding in an async generator:

async def wrapper(*args, **kw):
_engine.activate()
try:
if iscoroutinefunction(fn):
async for v in fn(*args, **kw):
yield v
else:
fn(*args, **kw)
finally:
_engine.disable()

@sarayourfriend
Copy link
Collaborator Author

Update: I've rebased the branch since the changes in master and that should fix the issue you mentioned with pook.on on async test functions.

Copy link

@tekumara tekumara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sarayourfriend I can confirm it now works well on my code base!

@sarayourfriend
Copy link
Collaborator Author

Awesome, thanks 🙏 I'm very excited to add httpx support in the next release 🎉

@sarayourfriend sarayourfriend merged commit b061a82 into master Dec 28, 2023
7 checks passed
@sarayourfriend sarayourfriend deleted the add/httpx branch December 28, 2023 22:19
@tekumara
Copy link

Thanks for adding this feature!

PS: I like pook over alternatives because the no match error message shows any unmatched mocks which is great for debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support httpx client
2 participants