Bug: TestClient used in pytest fails with `KeyError: 'session_maker_class'`

sherbang commented 6 months ago

Description

I've developed a failing test by writing a test that tries to test the litestar example in this repo: https://github.com/jolt-org/advanced-alchemy/compare/main...sherbang:advanced-alchemy:litestar_test

URL to code causing the issue

https://github.com/sherbang/advanced-alchemy/tree/litestar_test

MCVE

# Your MCVE code here

Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

"In the format of: ![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

/home/sherbang/devel/advanced-alchemy/tests/examples/test_litestar.py::test_create failed: test_client = <litestar.testing.client.sync_client.TestClient object at 0x7582d98e9c50>

    async def test_create(test_client: TestClient[Litestar]) -> None:
        author = AuthorCreate(name="foo")

        response = test_client.post(
            "/authors",
            json=author.model_dump(mode="json"),
        )
>       assert response.status_code == 200, response.text
E       AssertionError: Traceback (most recent call last):
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/middleware/exceptions/middleware.py", line 218, in __call__
E             await self.app(scope, receive, send)
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 82, in handle
E             response = await self._get_response_for_request(
E                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 134, in _get_response_for_request
E             return await self._call_handler_function(
E                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 154, in _call_handler_function
E             response_data, cleanup_group = await self._get_response_data(
E                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/routes/http.py", line 191, in _get_response_data
E             cleanup_group = await parameter_model.resolve_dependencies(request, kwargs)
E                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/_kwargs/kwargs_model.py", line 394, in resolve_dependencies
E             await resolve_dependency(next(iter(batch)), connection, kwargs, cleanup_group)
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/_kwargs/dependencies.py", line 65, in resolve_dependency
E             value = await dependency.provide(**dependency_kwargs)
E                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/di.py", line 101, in __call__
E             value = self.dependency(**kwargs)
E                     ^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/advanced_alchemy/extensions/litestar/plugins/init/config/asyncio.py", line 193, in provide_session
E             session_maker = cast("Callable[[], AsyncSession]", state[self.session_maker_app_state_key])
E                                                                ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           File "/home/sherbang/devel/advanced-alchemy/.venv/lib/python3.12/site-packages/litestar/datastructures/state.py", line 90, in __getitem__
E             return self._state[key]
E                    ~~~~~~~~~~~^^^^^
E         KeyError: 'session_maker_class'
E         
E       assert 500 == 200
E        +  where 500 = <Response [500 Internal Server Error]>.status_code

tests/examples/test_litestar.py:28: AssertionError

Jolt Project Version

0.8.1

Platform

[X] Linux
[ ] Mac
[ ] Windows
[ ] Other (Please specify in the description above)

Funding

If you would like to see an issue prioritized, make a pledge towards it!
We receive the pledge once the issue is completed & verified

Alc-Alc commented 6 months ago

Heya, I do not think this is a bug but rather an issue with how you are using the test client fixture.

https://github.com/sherbang/advanced-alchemy/blob/10a72168b3266a648fbf05f1cc289c411b3dab77/tests/examples/test_litestar.py#L15-L18

@pytest.fixture()
def test_client() -> TestClient[Litestar]:
    app.debug = True
    return TestClient(app=app)

That is your code, you are not using it as a yield based fixture. In your case (returning the client directly instead of yielding inside a context manager) does not emit the events and the key that it claims to be missing is only set when asgi events are sent (this only happens when you use it as a yield based fixture with context manager).

The following is an example for a test client fixture in fastapi (the same applies for Litestar), if you can write your fixture this way for a Litestar app it should work. (If it doesn't then that is a bug 🙃)

https://github.com/jolt-org/advanced-alchemy/blob/c3dba026e5e3d68717a41565e8cb659ee46cd1e9/tests/unit/test_extensions/test_starlette.py#L29-L32

sherbang commented 6 months ago

So it is... Thank you.

I'll submit a PR against Litestar's documentation that shows examples of fixtures just like this.

Is my PR adding a test for the example helpful? With this error fixed, it's catching another error with the id field. If it's helpful, then I'll look into fixing that, so the test can pass.

Alc-Alc commented 6 months ago

I'll submit a PR against Litestar's documentation that shows examples of fixtures just like this.

thanks 😇

Is my PR adding a test for the example helpful?

I will let other @jolt-org/maintainers and @jolt-org/members comment on that, I do not object to this 🙂

cofin commented 6 months ago

Hi - yes, it's definitely helpful. More coverage is always good!

sherbang commented 6 months ago

Well, the Litestar docs weren't wrong, I was just not reading carefully enough. Still, I opened this PR to improve them: https://github.com/litestar-org/litestar/pull/3258

I'll update my PR on this repo as well, but close this issue since it's not a real issue. Thanks for the feedback!

litestar-org / advanced-alchemy