Skip to content

integration test: Add DrainCloseIntegrationTest#11499

Merged
alyssawilk merged 9 commits into
envoyproxy:masterfrom
aunu53:drain
Jun 11, 2020
Merged

integration test: Add DrainCloseIntegrationTest#11499
alyssawilk merged 9 commits into
envoyproxy:masterfrom
aunu53:drain

Conversation

@aunu53
Copy link
Copy Markdown

@aunu53 aunu53 commented Jun 8, 2020

  • Add a DrainCloseIntegration test
  • Move ProtocolIntegrationTest.DrainClose to the new test
  • Move IntegrationTest.AdminDrainDrainsListeners to the new test

Description: I wanted to group all of the tests related to drain close behaviour in one place, in preparation for adding additional test coverage and complexity (e.g. simulated time tests, new admin endpoint options).

Risk Level: Test only

Auni Ahsan added 4 commits June 8, 2020 16:17
Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: Auni Ahsan <auni@google.com>
@aunu53
Copy link
Copy Markdown
Author

aunu53 commented Jun 8, 2020

A few notes:

  1. I left the TestAdminDrain function in http_integration.h so it can be used by extensions like quiche.
  2. Please see the note on the BUILD target, not sure if this should be blocked on windows.
  3. Could this instead be called ServerShutdownTest, and be the place for server shutdown sequence tests beyond specifically drain closing?

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

I'd say if you intend to add shutdown tests that aren't drain close you can rename, otherwise I don't think you have to try to fugure proof

Comment thread test/integration/BUILD Outdated
Comment thread test/integration/drain_close_integration_test.cc Outdated
Comment thread test/integration/drain_close_integration_test.cc Outdated
Signed-off-by: Auni Ahsan <auni@google.com>
@aunu53
Copy link
Copy Markdown
Author

aunu53 commented Jun 9, 2020

/azp run envoy-presubmit

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 11499 in repo envoyproxy/envoy

Auni Ahsan added 4 commits June 9, 2020 18:34
Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: Auni Ahsan <auni@google.com>
@alyssawilk alyssawilk merged commit be75dae into envoyproxy:master Jun 11, 2020
@aunu53 aunu53 deleted the drain branch June 11, 2020 19:17
arthuryan-k pushed a commit to arthuryan-k/envoy that referenced this pull request Jun 15, 2020
Description: groups drain close tests together
Risk Level: n/a (test only)

Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: Arthur Yan <arthuryan@google.com>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jun 24, 2020
Description: groups drain close tests together
Risk Level: n/a (test only)

Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
songhu pushed a commit to songhu/envoy that referenced this pull request Jun 25, 2020
Description: groups drain close tests together
Risk Level: n/a (test only)

Signed-off-by: Auni Ahsan <auni@google.com>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jul 24, 2020
Description: groups drain close tests together
Risk Level: n/a (test only)

Signed-off-by: Auni Ahsan <auni@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.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