Skip to content

Conversation

oschaaf
Copy link
Member

@oschaaf oschaaf commented Sep 14, 2020

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

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>
Copy link
Contributor

@dubious90 dubious90 left a 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.
Copy link
Contributor

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:
Copy link
Contributor

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.

@dubious90 dubious90 added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Sep 14, 2020
…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>
@oschaaf
Copy link
Member Author

oschaaf commented Sep 14, 2020

@dubious90 thanks for the review; I applied corresponding changes in 467dd6b

@oschaaf oschaaf added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Sep 14, 2020
Copy link
Contributor

@dubious90 dubious90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dubious90 dubious90 merged commit 44ba1ca into envoyproxy:master Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-review A PR waiting for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants