docs: Fix testing code examples

[hugovk]

Description

• At https://docs.litestar.dev/2/usage/testing.html, the examples after "We would then be able to rewrite our test like so:" don't follow. They contain a full-working example (production code, fixture and test case: sync and async). We had just moved the fixture to tests/conftest.py, and the production code is still in my_app/main.py, so we just need the test case in tests/test_health_check.py.
• And fix some typos.

[provinzkraut]

Could you explain a bit more what exactly is being fixed here?

The purpose of the examples being in self-contained files instead of inline is that they can be automatically tested as part of our test suite and typed checked.

[hugovk]

It starts off saying we have two files, one of production code, and another test file:

All good so far.

It then suggests moving the test client into a fixture in conftest.py. We're shown the conftest.py:

OK, that's part 1 of a refactor.

I would then expect part 2 to show the test file using the fixture from conftest, instead of its own.

But instead we're shown a single file that suddenly not only has the test case (that's fine) and fixture (but why, we were just told to put in conftest), and also the production code (again why, it started off being in its own file).

So not only does the page not do what it promised to do (move a fixture into conftest -- we now have two copies of the fixture), we've also inexplicably moved prod code into the test file (so now have two copies of that!).

[provinzkraut]

Thanks for the explanation @hugovk, makes sense.

I think the idea here was that example files should be self-contained, which is why all the code was copied into one file. Of course that doesn't make sense here given the text. I see two ways to fix this: Either stick to "one self-contained file" and remove the conftest.py or adjust the tests accordingly to use the fixtures from conftest.py.

I'm actually leaning towards option 1 slightly, because the conftest.py isn't really needed for what this example is trying to demonstrate. So the example with the fixture should just look like

import pytest

from litestar.status_codes import HTTP_200_OK
from litestar.testing import TestClient
from typing import Generator

from my_app.main import app


@pytest.fixture()
def client() -> Generator[TestClient, None, None]:
    with TestClient(app=app) as client:
        yield client


def test_health_check(client: TestClient):
    response = client.get("/health-check")
    assert response.status_code == HTTP_200_OK
    assert response.text == "healthy"

and the text be adjusted accordingly.

[hugovk]

Option 1 sounds fine. Like you say, the main point is you can put the client in a fixture for re-use, and you can put that where you like (same file or conftest.py).

(Apologies for the force-push, I forgot this PR is off my main.)

[provinzkraut]

@guacs I think this still needs the changes discussed 👀

[guacs]

@guacs I think this still needs the changes discussed 👀

Oh I think I misunderstood then :P I thought the changes were fine since it showed that you can use the test client as a fixture, but now looking at it again I think you may be right.

I think you wanted to make the change so that we remove the reference to conftest.py right?

[provinzkraut]

I think you wanted to make the change so that we remove the reference to conftest.py right?

That's what I thought @kedod was agreeing to here:

Option 1 sounds fine. Like you say, the main point is you can put the client in a fixture for re-use, and you can put that where you like (same file or conftest.py).

It also still has the in-line code, which we don't test.

[guacs]

I think you wanted to make the change so that we remove the reference to conftest.py right?

That's what I thought @kedod was agreeing to here:

Option 1 sounds fine. Like you say, the main point is you can put the client in a fixture for re-use, and you can put that where you like (same file or conftest.py).

It also still has the in-line code, which we don't test.

You're right. Also, did you mean @hugovk?

There are still some changes that need to be made.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.23%. Comparing base (243733a) to head (fc4c500).

❗ Current head fc4c500 differs from pull request most recent head 877e5d5. Consider uploading reports for the commit 877e5d5 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3143   +/-   ##
=======================================
  Coverage   98.23%   98.23%           
=======================================
  Files         320      320           
  Lines       14445    14425   -20     
  Branches     2298     2322   +24     
=======================================
- Hits        14190    14171   -19     
  Misses        113      113           
+ Partials      142      141    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3143

[hugovk]

I'll close this because it needs rewriting another way, and this is from my main (because of the CI branch restriction) and I keep forgetting and update it. I'll let someone else take care of the rewrite.