mindflayer / python-mocket

a socket mock framework - for all kinds of socket animals, web-clients included
BSD 3-Clause "New" or "Revised" License
280 stars 42 forks source link

Support for `fastapi`.`TestClient` #187

Closed gregbrowndev closed 2 years ago

gregbrowndev commented 2 years ago

Hi,

I've discovered an issue using mocket and end-to-end testing. In the code below, Mocket breaks the TestClient, which is a FastAPI utility that is subclass of Request's Session, as it prevents it from sending the real requests to the app's host, causing the test to hang indefinitely. I only want to mock the requests made by the app, e.g. to "https://example.org/".

Is it possible to exclude hosts from mocket's grasp? The TestClient internal sets the base_url to "http://testserver".

import httpx
import pytest
from fastapi import FastAPI
from fastapi.testclient import TestClient
from mocket import mocketize
from mocket.mockhttp import Entry

app = FastAPI()

@app.get("/")
async def read_main() -> dict:
    async with httpx.AsyncClient() as client:
        r = await client.get("https://example.org/")
        return r.json()

# End-to-end test

@pytest.fixture
def client() -> TestClient:
    return TestClient(app)

@mocketize
def test_read_main(client: TestClient) -> None:
    Entry.single_register(Entry.GET, "https://example.org/", body='{"id": 1}')

    response = client.get("/")

    assert response.status_code == 200
    assert response.json() == {"id": 1}
mindflayer commented 2 years ago

It's not your TestClient that is calling https://example.org/, hence the decorator is not covering that specific request. It's definitely something outside the scope of mocket.

gregbrowndev commented 2 years ago

If you debug the test the program never makes it into the FastAPI read_main function, it doesn't even reach the await client.get("https://example.org/") line (of course, if you remove the mocketize decorator it does). So it seems that mocket is affecting TestClient, which is to be expected since it is based on Requests.

Does mocket mock all HTTP requests or only the ones you set up with Entry.single_register?

mindflayer commented 2 years ago

Mocket works monkey-patching the socket module (w/ ssl for supporting HTTPS). What is not registered should be able to reach the real destination, through a real socket. If it does not, there might be something wrong. What I was trying to say is that in your scenario the call you want to mock happens outside mocket jurisdiction and it should not work as you expect. The fact that it gets stuck is definitely something to investigate, but what you want to do is not done properly because app lives outside the monkey-patching.

gregbrowndev commented 2 years ago

Thanks again @mindflayer for all the help with this and previous issues. Unfortunately, I'm at a push to get these httpx changes in before next week, and our app is pretty small, so I'm going to pick up RESPX for this specific app. mocket is great though, I've used it on a bunch of other projects and will continue it being my go-to.

Here is a test that passes using RESPX without the issues around jurisdiction. Maybe it benefits from mocking the internals of HTTPX directly so that it isn't affected by this problem (of course, it doesn't affect Requests at all so has no issues around hanging). Hopefully, this is helpful for mocket nonetheless.

def test_respx(
    respx_mock: MockRouter,
    client: TestClient
) -> None:
    respx_mock.get("https://example.org/").mock(
        return_value=httpx.Response(200, json={"id": 1})
    )

    response = client.get("/")

    assert response.status_code == 200
    assert response.json() == {"id": 1}
mindflayer commented 2 years ago

Did you try to create the app from inside your test case? In case you did not, I'll try that and let you know, just out of curiosity.

gregbrowndev commented 2 years ago

Just gave it a try now. It still has the hanging issue so not able to see whether it made a difference to the mocked request. Also, this pattern is at odds with pytest's fixture-based approach (maybe you could put with Mocketizer in the fixture like we did in a previous issue, but that is also not so obvious):

import httpx
from fastapi import FastAPI
from fastapi.testclient import TestClient
from mocket import mocketize
from mocket.mockhttp import Entry

def create_app() -> FastAPI:
    app = FastAPI()

    @app.get("/")
    async def read_main() -> dict:
        async with httpx.AsyncClient() as client:
            r = await client.get("https://example.org/")
            return r.json()

    return app

@mocketize
def test_mocket() -> None:
    app = create_app()
    client = TestClient(app)

    Entry.single_register(Entry.GET, "https://example.org/", body='{"id": 1}')

    response = client.get("/")

    assert response.status_code == 200
    assert response.json() == {"id": 1}
mindflayer commented 2 years ago

Here is a test that passes using RESPX without the issues around jurisdiction

Of course using a mock for a specific client is a totally different approach, and I am glad you found something that works for you. I'll have a look at the issue you mentioned here and I'll let you know what I discover.

mindflayer commented 2 years ago

I am still trying to put all the pieces together, but I have a suspect. Fastapi TestClient is based on starlette.testclient which makes use of BlockingPortal from anyio. Copying from its documentation:

If you need to run async code from a thread that is not a worker thread spawned by the event loop, you need a blocking portal. This needs to be obtained from within the event loop thread.

This smells like something a bit outside the scope of Mocket.

mindflayer commented 2 years ago

In case you'd want to test something like that with Mocket, I'd suggest you to use it for testing the functions/code behind your Fastapi service. In that case, you could then test your API endpoints while monkey-patching the external calls with unittest.mock.patch.

gregbrowndev commented 2 years ago

Interesting, thanks for looking into this @mindflayer

KentShikama commented 2 years ago

I'm also interested in using the FastAPI/httpx stack along with Python Mocket. Using the MVCE from https://github.com/mindflayer/python-mocket/issues/187#issuecomment-1220831109, it looks like https://github.com/encode/starlette/blob/bd219edc4571806edf80fd6a48c8ac3fbbadcf22/starlette/testclient.py#L264 is where it ends up hanging versus if you take out the mocketize, that call goes through to hit the FastAPI application. The error is type object 'Mocket' has no attribute '_address'.

KentShikama commented 2 years ago

Ok after some more digging, I realized that the error is coming from the previous line. No wonder I was confused. Here is the stacktrace:

get_entry, mocket.py:437
get_entry, mocket.py:258
send, mocket.py:391
_write_to_self, selector_events.py:140
call_soon_threadsafe, base_events.py:797
run_sync_from_thread, _asyncio.py:959
_spawn_task_from_thread, _asyncio.py:989
start_task_soon, from_thread.py:369
call, from_thread.py:283
send, testclient.py:271
send, sessions.py:701
request, sessions.py:587
request, testclient.py:481
get, sessions.py:600
test_mocket, test_mocket.py:27
KentShikama commented 2 years ago

I'm not sure why but asyncio tries to write a null byte to the socket (see https://github.com/python/cpython/blob/129998bd7b133defa37c7529bfad9052c0022b5c/Lib/asyncio/selector_events.py#L139) but mocket has already "hijacked" the socket so the call fails.

KentShikama commented 2 years ago

Ah ok, so mocket expects a call to socket.connect in order to set the address but the TestClient does not.

KentShikama commented 2 years ago

Specifically it looks like mocket doesn't really mock a "self-socket": see https://github.com/python/cpython/blob/main/Lib/asyncio/selector_events.py#L106-L107.

KentShikama commented 2 years ago

I think mocket needs to support the idea of a "self-socket" being mocked in order for it to support FastAPI's TestClient.

mindflayer commented 2 years ago

I'm not sure why but asyncio tries to write a null byte to the socket

The other day I left here. Thanks for making some more progress.

I think mocket needs to support the idea of a "self-socket"

Let's see what I can do. :)

mindflayer commented 2 years ago

So, the two instances of socket created by socket.socketpair() are MocketSocket ones. If I make this send(b'\0') pass the code is still stuck, waiting forever:

Traceback (most recent call last):
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 495, in start_blocking_portal
    yield portal
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/starlette/testclient.py", line 454, in _portal_factory
    yield portal
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/starlette/testclient.py", line 266, in send
    response_complete = portal.call(anyio.Event)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 283, in call
    return cast(T_Retval, self.start_task_soon(func, *args).result())
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 369, in start_task_soon
    self._spawn_task_from_thread(func, args, {}, name, f)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 989, in _spawn_task_from_thread
    run_sync_from_thread(
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 961, in run_sync_from_thread
    return f.result()
  File "/usr/lib/python3.10/concurrent/futures/_base.py", line 441, in result
    self._condition.wait(timeout)
  File "/usr/lib/python3.10/threading.py", line 320, in wait
    waiter.acquire()
KeyboardInterrupt

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 497, in start_blocking_portal
    portal.call(portal.stop, True)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 283, in call
    return cast(T_Retval, self.start_task_soon(func, *args).result())
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 369, in start_task_soon
    self._spawn_task_from_thread(func, args, {}, name, f)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 989, in _spawn_task_from_thread
    run_sync_from_thread(
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 961, in run_sync_from_thread
    return f.result()
  File "/usr/lib/python3.10/concurrent/futures/_base.py", line 441, in result
    self._condition.wait(timeout)
  File "/usr/lib/python3.10/threading.py", line 320, in wait
    waiter.acquire()
KeyboardInterrupt

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/drizzt/repos/python-mocket/prova.py", line 26, in <module>
    response = client.get("/")
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/requests/sessions.py", line 600, in get
    return self.request("GET", url, **kwargs)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/starlette/testclient.py", line 476, in request
    return super().request(
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/requests/sessions.py", line 587, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/requests/sessions.py", line 701, in send
    r = adapter.send(request, **kwargs)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/starlette/testclient.py", line 270, in send
    raise exc
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/starlette/testclient.py", line 265, in send
    with self.portal_factory() as portal:
  File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/starlette/testclient.py", line 453, in _portal_factory
    with anyio.start_blocking_portal(**self.async_backend) as portal:
  File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/home/drizzt/repos/python-mocket/.venv/lib/python3.10/site-packages/anyio/from_thread.py", line 475, in start_blocking_portal
    with ThreadPoolExecutor(1) as executor:
  File "/usr/lib/python3.10/concurrent/futures/_base.py", line 637, in __exit__
    self.shutdown(wait=True)
  File "/usr/lib/python3.10/concurrent/futures/thread.py", line 235, in shutdown
    t.join()
  File "/usr/lib/python3.10/threading.py", line 1089, in join
    self._wait_for_tstate_lock()
  File "/usr/lib/python3.10/threading.py", line 1109, in _wait_for_tstate_lock
    if lock.acquire(block, timeout):
KeyboardInterrupt
KentShikama commented 2 years ago

@mindflayer I'm not sure I follow. Are you saying if you comment out the null byte send it still hangs? There are two portal.calls so you would have to comment out both of them.

KentShikama commented 2 years ago

The host and port aren't set until .connect is called in MocketSocket. Once it hits

  File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
    self.gen.throw(typ, value, traceback)

it is already failing. You can see the error message type object 'Mocket' has no attribute '_address' if you put a breakpoint there. The "hanging" after that is misleading I think.

mindflayer commented 2 years ago

Let me share my change with you, see this commit on the branch I've just pushed. With this change the error is not happening, but it still hangs.

KentShikama commented 2 years ago

Thank for sharing! That clears it up as to what you're trying to do.

I think the null byte is fine. The problem is that the host and port aren't set yet when the null byte is written out (or anything else written out after that) and so the get_entry call fails. The exception is then swallowed by the context manager but you can print it out if you breakpoint at File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__. I'm currently struggling to figure out how to set the host and port as there is no call to .connect.

mindflayer commented 2 years ago

Well, that's not a problem any more after my change. Since it's just a "ping" we don't need to worry about it. The problem is that after send(b'\0') nothing really happens.

KentShikama commented 2 years ago

Oh I see. The https://github.com/agronholm/anyio/blob/d9c2d9b3517e1137beb35a4a2cd56fb1af26a859/src/anyio/_backends/_asyncio.py#L961 hangs because it is waiting for the condition at self._condition.wait(timeout) (and the timeout by default is None).

mindflayer commented 2 years ago

Even if we change the timeout nothing really happens.

KentShikama commented 2 years ago

Well if you change the timeout, it certainly goes onto the next portal.call instead of hanging at the first one.

KentShikama commented 2 years ago

Ok so I see the event being scheduled onto the event loop (with https://github.com/python/cpython/blob/575f8880bf8498ee05a8e197fc2ed85db6880361/Lib/asyncio/base_events.py#L781) but for whatever reason I don't see the next event loop being scheduled. I wonder if that null byte is to say that the current event loop is finished.

KentShikama commented 2 years ago

The major difference between call_soon and call_soon_threadsafe, however, is that call_soon_threadsafe wakes up the event loop by calling loop._write_to_self() ." - https://stackoverflow.com/a/43275001

KentShikama commented 2 years ago

Hmm I'm starting to think that Mocket shouldn't be messing with the sockets that drive the core of the asyncio event loop. Maybe there is a way to detect "non-local" sockets and only mock those?

mindflayer commented 2 years ago

Maybe there is a way to detect "non-local" sockets and only mock those?

I am not sure I understand what you mean with that. Before a connect() happens, a socket is just a socket and you have no information about its purpose. Mocket already supports aiohttp, so it's not asyncio that does not work with it. What you'd like to do here is using Mocket for intercepting calls made by an asyncio server, which is what is not working properly.

mindflayer commented 2 years ago

Like I said at first, the fact that this BlockingPortal plays with different threads is probably the root of the issues we see here, and I am sorry but I don't see an easy way to fix this. Of course I am open to review PRs to make it happen, if the rest of the tests keep passing.

mindflayer commented 2 years ago

I want to try out something completely different, making socket.socketpair() returning real socket instances. Let's see what happens.

KentShikama commented 2 years ago

socket is just a socket and you have no information about its purpose

Yup you're right. Pardon my stream of conscious.

socket.socketpair() returning real socket instances

Oh I like this idea. I still haven't figured out how the asyncio event loop works under the hood but it seems like it uses a socket pair (I'll call them "incoming" and "outgoing") and outside threads can write to the "outgoing" socket to wake it up. And because Mocket is mocking both the sockets but it doesn't actually send the contents of "outgoing" socket to the "incoming" socket, the event loop isn't woken up.

mindflayer commented 2 years ago

It's a kind of magic. <3

KentShikama commented 2 years ago

I just looked at your PR. It looks great! Thank you for pushing on this!

On an unrelated note, I am surprised how little options there are when it comes to having a "true mock server" that can just run through pytest. Thank you for filling in this gap.

mindflayer commented 2 years ago

It drove me crazy, but here it is: https://pypi.org/project/mocket/3.10.8/

Feel free to pip install mocket==3.10.8.