Full test suite for HTTP requests #91
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hypothesis: "It works by letting you write tests that assert that something should be true for every case, not just the ones you happen to think of." I think it's well suited for the task of ensuring Daphne conforms to the ASGI specification.
While grepping for calls to str(), I found this bit which looks like a cast to unicode was intended under Python 2.
This commit introduces a new test case class, with it's main method assert_valid_http_request_message. The idea is that this method is a translation of the ASGI spec to code, and can be used to check channel messages for conformance with that part of the spec. I plan to add further methods for other parts of the spec.
Hypothesis, our framework for test data generation, contains only general so-called strategies for generating data. This commit adds a few which will be useful for generating the data for our tests. Alos see http://hypothesis.readthedocs.io/en/latest/data.html.
This commit introduces a few Hypothesis tests to test the HTTP request part of the specification. To keep things organized, I split the existing tests module into two: one concerned with requests, and one concerned with responses. I anticipate that we'll also add modules for chunks and server push later. daphne already had tests for the HTTP protocol. Some of them I converted to Hypothesis tests to increase what was tested. Some were also concerned with HTTP responses, so they were moved to the new response module. And three tests were concerned with proxy behaviour, which I wasn't sure about, and I just kept them as-is, but also moved them to the request tests.
Twisted seems to return a byte string for the client and server IP address. It is easily rectified by casting to the required unicode string. Also added a test to ensure this is also handled correctly in the X-Forwarded-For header parsing.
I'm in the process of updating the ASGI spec to require that the order of header values is kept. To match that work, I'm adding matching assertions to the tests. The code unfortunately is not as elegant as I'd like, but then it's only a result of the underlying HTTP spec.
The kitchen sink test expectedly can be slow. So far it wasn't slow enough for hypothesis to trigger a warning, but today Travis must've had a bad day. I think for this test is is acceptable to exceed hypothesis' warning threshold.
|
I proposed some changes to the ASGI spec in django/channels#554, and updated this pull request with matching tests. |
|
Ugh, somehow I have missed looking at this for over a week. It looks good, I'll pull it in. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main goal of this pull request is to add a couple tests for HTTP requests. All channel messages generated by those tests also get verified by a new method, which is a translation of the ASGI spec. Instead of using static test data, the tests employ hypothesis, which is rather good at generating data that breaks tests.
As part of this pull requests, some of the existing tests were removed, or moved into a new module. I also stumbled across two minor byte string issues that I could fix.