Skip to content

Conversation

@wdauchy
Copy link
Contributor

@wdauchy wdauchy commented Nov 25, 2025

Commit Message:
Add integration tests verifying VHDS on-demand updates work correctly with request bodies for both HTTP/1.1 and HTTP/2. The tests use HttpProtocolIntegrationTest parameterization to automatically run for both downstream protocols (HTTP/1.1 and HTTP/2), ensuring the fix for #17891 works correctly across different HTTP protocol versions.

Additional Description:

Tests verify that:

  • Request bodies are preserved when triggering on-demand VHDS updates
  • Stream recreation works correctly after VHDS updates complete
  • Both HTTP/1.1 and HTTP/2 protocols handle bodies correctly

The test instantiation uses getProtocolTestParamsWithoutHTTP3() which returns parameter combinations for both HTTP/1.1 and HTTP/2 downstream protocols, so each test runs multiple times (once per protocol combination).

Complements the unit tests in d39afea and provides end-to-end verification for #17891.

followup of #42158
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #42248 was opened by wdauchy.

see: more, trace.

@wdauchy wdauchy force-pushed the vhds-test branch 29 times, most recently from e4672b7 to ed732e5 Compare November 26, 2025 15:57
@wdauchy wdauchy force-pushed the vhds-test branch 5 times, most recently from 0e738ed to 9ac4ca9 Compare November 27, 2025 12:17
@wdauchy wdauchy marked this pull request as ready for review November 27, 2025 12:45
Copy link
Member

@botengyao botengyao left a comment

Choose a reason for hiding this comment

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

thanks for adding the integration test!

/wait

TEST_P(OnDemandVhdsWithBodyIntegrationTest, VhdsOnDemandUpdateWithBody) {
// Skip Unified mux - it doesn't properly handle on-demand VHDS updates.
const bool is_unified_mux =
std::get<2>(std::get<1>(GetParam())) == Grpc::LegacyOrUnified::Unified;
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason that unified client doesn't handle it properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is a bad wording I used. I was unable to make it work so far, I am getting a timeout all the time (a race in the setup?). so I thought I would first propose an initial step.

Copy link
Member

@botengyao botengyao Dec 1, 2025

Choose a reason for hiding this comment

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

nit: mind adding TODO(wdauchy)? just to keep it be consistent with other places, and it is okay that others pick up as well

Add integration tests verifying VHDS on-demand updates work correctly
with request bodies for both HTTP/1.1 and HTTP/2. The tests use
HttpProtocolIntegrationTest parameterization to automatically run
for both downstream protocols (HTTP/1.1 and HTTP/2), ensuring the fix
for envoyproxy#17891 works correctly across different HTTP protocol versions.

Tests verify that:
- Request bodies are preserved when triggering on-demand VHDS updates
- Stream recreation works correctly after VHDS updates complete
- Both HTTP/1.1 and HTTP/2 protocols handle bodies correctly

The test instantiation uses getProtocolTestParamsWithoutHTTP3() which
returns parameter combinations for both HTTP/1.1 and HTTP/2 downstream
protocols, so each test runs multiple times (once per protocol combination).

Complements the unit tests in d39afea
and provides end-to-end verification for envoyproxy#17891.

Risk Level: Low
Testing: New integration tests parameterized for HTTP/1.1 and HTTP/2

Signed-off-by: William Dauchy <[email protected]>
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