-
Notifications
You must be signed in to change notification settings - Fork 90
Enhance end-to-end test for POST requests with an entity body. #532
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
Enhance end-to-end test for POST requests with an entity body. #532
Conversation
Make the end-to-end test for POST handling cover all extensions. Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
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.
Two really small comments.
'filter_configs', | ||
["{}", "{static_delay: \"0.01s\"}", "{emit_previous_request_delta_in_response_header: \"aa\"}"]) | ||
def test_request_body_gets_transmitted(http_test_server_fixture, filter_configs): | ||
"""Test request body transmission with. |
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.
I think this comment is incomplete?
@@ -434,13 +437,17 @@ def check_upload_expectations(fixture, parsed_json, expected_transmitted_bytes, | |||
expected_received_bytes) | |||
|
|||
upload_bytes = 1024 * 1024 * 3 | |||
# TODO(#531): The dynamic-delay extension hangs unless we lower the request entity body size. | |||
if "static_delay" in filter_configs: |
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.
How do you feel about combining this with the above line and making it a ternary, rather than an if condition?
My 2 cents is that as is, it's possible to scan this and not notice that upload_bytes has changed from its original value.
…g-integration-test Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@dubious90 thanks for the review; I applied corresponding changes in 467dd6b |
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.
LGTM
Make the end-to-end test for POST handling cover all extensions.
Split out from #512
Signed-off-by: Otto van der Schaaf oschaaf@we-amp.com