TestClient fails to close memory stream objects
#2603
-
|
EDIT: My original post below was based on my shallow understanding of the problem. See the comments below by @ppena-LiveData and @markwaddle for better test examples. An example is worth a thousand words: from starlette.testclient import TestClient
from starlette.testclient import TestClient
import contextlib
from typing import AsyncIterator, TypedDict
import httpx
from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import PlainTextResponse
from starlette.routing import Route
# From https://www.starlette.io/lifespan/#running-lifespan-in-tests
class State(TypedDict):
http_client: httpx.AsyncClient
@contextlib.asynccontextmanager
async def lifespan(app: Starlette) -> AsyncIterator[State]:
async with httpx.AsyncClient() as client:
yield {"http_client": client}
async def homepage(request: Request) -> PlainTextResponse:
client = request.state.http_client
response = await client.get("https://www.example.com")
return PlainTextResponse(response.text)
app = Starlette(lifespan=lifespan, routes=[Route("/", homepage)])
# Actual testing code
def test_app():
with TestClient(app) as client:
response = client.get("/")
assert response.status_code == 200
def test_app2():
with TestClient(app) as client:
response = client.get("/")
assert response.status_code == 200
def test_app3():
with TestClient(app) as client:
response = client.get("/")
assert response.status_code == 200When running this with Note that this warning appears with anyio 4.4.0. Also, the warning does not appear when running a single test (which is why multiple My guess is that the |
Beta Was this translation helpful? Give feedback.
Replies: 8 comments 13 replies
-
|
This doesn't require multiple from fastapi import APIRouter, FastAPI
from fastapi.testclient import TestClient
app = FastAPI()
router = APIRouter()
@router.post('/api/test')
def route() -> str:
return 'OK'
app.include_router(router)
def test_route():
with TestClient(app) as client:
client.get('/api/test')For some reason, |
Beta Was this translation helpful? Give feedback.
-
|
@loichuder, do you mind if we change the subject of this discussion to something simpler, like " |
Beta Was this translation helpful? Give feedback.
-
|
a somewhat simpler test that often, but not always, reproduces this warning from starlette.applications import Starlette
from starlette.testclient import TestClient
def test_close_warning():
for _ in range(10):
with TestClient(Starlette()) as client:
client.get("/") |
Beta Was this translation helpful? Give feedback.
-
|
i'm fairly confident this is an issue. i've tracked it down to the async def wait_shutdown(self) -> None:
async def receive() -> typing.Any:
message = await self.stream_send.receive()
if message is None:
self.task.result()
return message
async with self.stream_send:
await self.stream_receive.send({"type": "lifespan.shutdown"})
message = await receive()
assert message["type"] in (
"lifespan.shutdown.complete",
"lifespan.shutdown.failed",
)
if message["type"] == "lifespan.shutdown.failed":
await receive()i recommend this function be changed to the following, to close both streams: async def wait_shutdown(self) -> None:
async def receive() -> typing.Any:
message = await self.stream_send.receive()
if message is None:
self.task.result()
return message
async with self.stream_send, self.stream_receive:
await self.stream_receive.send({"type": "lifespan.shutdown"})
message = await receive()
assert message["type"] in (
"lifespan.shutdown.complete",
"lifespan.shutdown.failed",
)
if message["type"] == "lifespan.shutdown.failed":
await receive()i've verified that this change fixes the issue. |
Beta Was this translation helpful? Give feedback.
-
|
Now that we have a fix for this, what is the procedure; can we submit a PR with that fix, or do we need an admin to approve something? The Reporting Bugs or Other Issues section fo |
Beta Was this translation helpful? Give feedback.
-
|
I've encountered the same issue just today but it appears no PR was created so I've created one here PR-2693. Credit to @markwaddle for proposing the solution. @Kludex @jhominal can you please take a look. Thanks. |
Beta Was this translation helpful? Give feedback.
-
|
I'm still seeing this error when testing fastapi using the following package versions, even with PR-2693 applied:
So there still seem to be cases where this PR is insufficient to close the streams, unfortunately. But I have no idea how to track down the cause. (I also posted this to the PR, but realised that this is a better location.) |
Beta Was this translation helpful? Give feedback.
-
|
This should be solved now. |
Beta Was this translation helpful? Give feedback.
This should be solved now.