Colin-b / pytest_httpx

pytest fixture to mock HTTPX
https://colin-b.github.io/pytest_httpx/
MIT License
344 stars 32 forks source link

pytest_httpx breaks httpx timeout logic #131

Closed andmis closed 6 months ago

andmis commented 8 months ago

The following test should pass but doesn't:

import asyncio
import httpx
import pytest
from pytest_httpx import HTTPXMock

async def test_timeouts(httpx_mock: HTTPXMock):
    async def hanging_backend(request: httpx.Request):
        await asyncio.sleep(99999)

    httpx_mock.add_callback(hanging_backend)

    async with httpx.AsyncClient() as client:
        with pytest.raises(httpx.ReadTimeout):
            await client.get("http://foobar", timeout=1.0)

I haven't gone digging but it looks like pytest_httpx injects itself into httpx in a way that disables httpx's timeout logic. I know I can raise the exception from my CB, but I think that pytest_httpx should in principle leave the normal httpx timeout functionality working and just mock the part of the stack sending an HTTP request and receiving an HTTP response (which conceptually is lower in the stack than timeout logic) (and which is what it looks like add_callback is supposed to be doing).

If that isn't an option then at least the callback should get passed all of the args to the original request (specifically the timeout parameter to the get call in this case) so that it in principle can figure out how to respond correctly based on just the arguments to the callback. But IMO the solution in the previous paragraph is much better.

Colin-b commented 7 months ago

Hello @andmis and apologies on the late reply,

Can you confirm my understanding:

Depending on your answer I can already say that it is likely to break expected behavior for some users but I could envision implementation of 1 as the workaround would be to increase the timeout on user side, should they want to do operations taking more than timeout time.

2 is of course not acceptable unfortunately as a test framework should fail fast, considering a standard application with around 500+ test cases, even the addition of 1 extra second to 500 test cases would result in a huge amount of extra duration to receive the failures to analyse and fix. Also some users already have test cases relying on the fail fast timeout scenario.

Please tell me if I am correct in my understanding to figure out what we could work towards that would fit your needs.

Colin-b commented 6 months ago

Considering as already supported, httpx.Request contains every timeout information in the extensions property. You can act upon it in your callback.

Please note that a test case aims at reproducing behavior in a deterministic manner. Having your test case failing or not based on your machine performances from one run to another is not acceptable IMO and this is not what this package is aiming at. For this reason, the mock will always do what you instruct him to,

Moreover, timeout does not mean much as you have connect, read, and so on types of timeout.

You already can reproduce timeouts in a variety of ways:

1) Do not mock anything, a Timeout exception will instantly be raised. 2) Call add_exception with a timeout exception based on your matching criteria (if any). 3) Add a callback and reproduce or not a real timeout (with/without waiting, up to you and your test duration/scenarios preferences).