Add tests to test_responses
#2656
Conversation
tests/test_responses.py
Outdated
| def test_set_cookie_without_optional_keys( | ||
| test_client_factory: TestClientFactory, | ||
| ) -> None: | ||
| async def app(scope: Scope, receive: Receive, send: Send) -> None: | ||
| response = Response("Hello, world!", media_type="text/plain") | ||
| response.set_cookie( | ||
| "mycookie", | ||
| "myvalue", | ||
| max_age=None, | ||
| expires=None, | ||
| path=None, | ||
| domain=None, | ||
| secure=False, | ||
| httponly=False, | ||
| samesite=None, | ||
| ) | ||
| await response(scope, receive, send) | ||
|
|
||
| client = test_client_factory(app) | ||
| response = client.get("/") | ||
| assert response.headers["set-cookie"] == "mycookie=myvalue" | ||
|
|
||
|
|
There was a problem hiding this comment.
https://github.com/encode/starlette/blob/master/starlette/responses.py#L105-L120
This is testing the circumstance where all those conditions are False.
tests/test_responses.py
Outdated
| def test_file_response_with_manually_specified_media_type( | ||
| tmpdir: Path, test_client_factory: TestClientFactory | ||
| ) -> None: | ||
| path = os.path.join(tmpdir, "xyz") | ||
| content = b"<file content>" | ||
| with open(path, "wb") as file: | ||
| file.write(content) | ||
| # By default, FileResponse will determine the `media_type` based on | ||
| # the filename or path, unless a specific `media_type` is provided. | ||
| app = FileResponse(path=path, filename="example.png", media_type="image/jpeg") | ||
| client: TestClient = test_client_factory(app) | ||
| response = client.get("/") | ||
| assert response.headers["content-type"] == "image/jpeg" |
There was a problem hiding this comment.
https://github.com/encode/starlette/blob/master/starlette/responses.py#L294-L295
Testing the uncovered branch of this if statement.
|
@Kludex Thanks for the approval. I noticed that you updated some logic and variable names. Do we have relevant coding standards? If so, I can follow them while working on the rest of tests in the future. |
We do have a page in our docs, but for the changes I did in this PR, no. I just updated the code to use
The way you did here is perfect! A couple of tests per PR is good for me. |
* test: add test cases for uncovered branches in starlette.responses * chore: fix format issue * Update test * Remove unused import * Update tmpdir to tmp_path --------- Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Summary
Add tests for some of the uncovered branches in
starlette.responses, related to this issueChecklist