-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed bug in serving static directory #803
Conversation
import asyncio | ||
|
||
SERVER_HOST = '127.0.0.1' | ||
SERVER_PORT = 8080 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use unused_port
fixture here.
See https://github.com/KeepSafe/aiohttp/blob/90f16168fd8510fe930994c4b4170a18cd8f2568/tests/test_run_app.py#L35-L36 for inspiration.
Thanks! I'll merge after you'll fix two mentioned tiny comments. |
I added the required modifications. Thank you for teaching me about flake8 :) |
pass | ||
|
||
|
||
def run_timeout(cor, loop, timeout=ASYNC_TEST_TIMEOUT): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need the function? What the purpose of timeout parameter -- looks like you don't use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is used for making sure the test doesn't run too long. Do you have other mechanism to do this in the code base? If not, maybe we can add it in a more global location, to be used with other tests. The ASYNC_TEST_TIMEOUT is the default timeout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
Right now aiohttp test suite has no such feature. I didn't feel the need for it.
Let's add the feature later to hooks that provide @pytest.mark.run_loop
functionality.
I fixed the test case. It now uses mark.run_loop and create_app_and_client, so it is much shorter. |
Fixed bug in serving static directory
Thanks! |
When using app.add_static(...) to serve a directory, if the root of the directory is requested, the connection between the server and client is crashed.
This happens because the server tries to read a directory using open with 'rb' and fails.
The fix first makes sure that the requested resource is a file. If it is not a file, it returns a 404 error. I think this is the correct behaviour, feel free to change it if you think it should be something else.
I also include a relevant test case, to avoid regressions with this problem.