Skip to content
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

test interaction between drain upon completion & onDrainTimeout #35568

Merged
merged 5 commits into from
Aug 12, 2024

Conversation

antoniovleonti
Copy link
Contributor

Commit Message: test interaction between drain upon completion & onDrainTimeout
Additional Description:

adds unit & integration tests that demonstrate it is safe to use max_requests_per_connection with max_connection_duration, idle_timeout, and max_stream_duration.

(idle_timeout is not explicitly tested but triggers the same exact drain behavior as max_connection_duration.)

Risk Level: none
Testing: test-only PR

@antoniovleonti antoniovleonti force-pushed the cx-close-test branch 3 times, most recently from 5108d1b to 28d4ab1 Compare August 2, 2024 20:46
Signed-off-by: antoniovleonti <leonti@google.com>
@antoniovleonti
Copy link
Contributor Author

/retest

Signed-off-by: antoniovleonti <leonti@google.com>
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

nice testing
/wait

test/integration/protocol_integration_test.cc Show resolved Hide resolved
test/integration/protocol_integration_test.cc Outdated Show resolved Hide resolved
test/integration/protocol_integration_test.cc Outdated Show resolved Hide resolved
test/integration/protocol_integration_test.cc Outdated Show resolved Hide resolved
test/integration/protocol_integration_test.cc Show resolved Hide resolved
test/integration/protocol_integration_test.cc Outdated Show resolved Hide resolved
Signed-off-by: antoniovleonti <leonti@google.com>
@antoniovleonti
Copy link
Contributor Author

Much better, thanks.

Signed-off-by: antoniovleonti <leonti@google.com>
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

/wait-any
LGTM modulo that one comment nit

test/common/http/conn_manager_impl_test_2.cc Outdated Show resolved Hide resolved
Signed-off-by: antoniovleonti <leonti@google.com>
@antoniovleonti
Copy link
Contributor Author

/retest

@alyssawilk alyssawilk merged commit a8293b7 into envoyproxy:main Aug 12, 2024
50 checks passed
RyanTheOptimist pushed a commit that referenced this pull request Aug 12, 2024
Commit Message: test: update a broken test due to PRs race
Additional Description:
PR #35568 used an old `setup()` method call which was modified by PR
#35209 that was merged just before it, causing a build failure.
This PR update the test and fixes the build failure.

Risk Level: low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@antoniovleonti antoniovleonti deleted the cx-close-test branch August 16, 2024 14:20
asingh-g pushed a commit to asingh-g/envoy that referenced this pull request Aug 20, 2024
…yproxy#35568)

Commit Message: test interaction between drain upon completion &
onDrainTimeout
Additional Description:

adds unit & integration tests that demonstrate it is safe to use
max_requests_per_connection with max_connection_duration, idle_timeout,
and max_stream_duration.

(idle_timeout is not explicitly tested but triggers the same exact drain
behavior as max_connection_duration.)

Risk Level: none
Testing: test-only PR

---------

Signed-off-by: antoniovleonti <leonti@google.com>
Signed-off-by: asingh-g <abhisinghx@google.com>
asingh-g pushed a commit to asingh-g/envoy that referenced this pull request Aug 20, 2024
Commit Message: test: update a broken test due to PRs race
Additional Description:
PR envoyproxy#35568 used an old `setup()` method call which was modified by PR
envoyproxy#35209 that was merged just before it, causing a build failure.
This PR update the test and fixes the build failure.

Risk Level: low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
Signed-off-by: asingh-g <abhisinghx@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants