Skip to content

Enhance end-to-end test for POST requests with an entity body.#532

Merged
dubious90 merged 4 commits intoenvoyproxy:masterfrom
oschaaf:post-path-handling-integration-test
Sep 14, 2020
Merged

Enhance end-to-end test for POST requests with an entity body.#532
dubious90 merged 4 commits intoenvoyproxy:masterfrom
oschaaf:post-path-handling-integration-test

Conversation

@oschaaf
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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?


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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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