envoy
envoy copied to clipboard
envoyproxy
Deduplicate and fix header validator unit tests
Commit Message: Deduplicate and fix header validator unit tests Additional Description: N/A Risk Level: Low Testing: Unit tests Docs Changes: Release Notes: Platform Specific Features:
These are not the same tests. The HttpConnectionManagerImplTest test validates integration of UHV with HCM, while the Http2HeaderValidatorTest validates logic in the default HTTP/2 header validator.
/wait-any
These are not the same tests. The HttpConnectionManagerImplTest test validates integration of UHV with HCM, while the Http2HeaderValidatorTest validates logic in the default HTTP/2 header validator.
/wait-any
Yeah, I agree that HttpConnectionManagerImplTest and Http2HeaderValidatorTest are different test suites. However, there are duplicated tests within each test suite.
These are not the same tests. The HttpConnectionManagerImplTest test validates integration of UHV with HCM, while the Http2HeaderValidatorTest validates logic in the default HTTP/2 header validator. /wait-any
Yeah, I agree that
HttpConnectionManagerImplTestandHttp2HeaderValidatorTestare different test suites. However, there are duplicated tests within each test suite.
They test very different things. One tests interaction between HCM and UHV using mock UHV, the other tests real (default) UHV validation of trailers.
These are not the same tests. The HttpConnectionManagerImplTest test validates integration of UHV with HCM, while the Http2HeaderValidatorTest validates logic in the default HTTP/2 header validator. /wait-any
Yeah, I agree that
HttpConnectionManagerImplTestandHttp2HeaderValidatorTestare different test suites. However, there are duplicated tests within each test suite.They test very different things. One tests interaction between HCM and UHV using mock UHV, the other tests real (default) UHV validation of trailers.
Sorry, trying to make sure we are on the same page. Do you think below two tests are the same?
- https://github.com/envoyproxy/envoy/blob/40dddc9ee82e63ef06903efac822a08e031473b6/test/common/http/conn_manager_impl_test_2.cc#L3827
- https://github.com/envoyproxy/envoy/blob/40dddc9ee82e63ef06903efac822a08e031473b6/test/common/http/conn_manager_impl_test_2.cc#L3881
- HeaderValidatorFailTrailersTransformationBeforeResponse
These do look like the same test.
- HeaderValidatorFailTrailersTransformationBeforeResponse
These do look like the same test.
Thank you for the confirmation! And I removed the 2nd test case in this PR.
Also, https://github.com/envoyproxy/envoy/blob/c61838567f214812b991f0c76db935a54300d94a/test/extensions/http/header_validators/envoy_default/http2_header_validator_test.cc#L441 and https://github.com/envoyproxy/envoy/blob/c61838567f214812b991f0c76db935a54300d94a/test/extensions/http/header_validators/envoy_default/http2_header_validator_test.cc#L729 look the same. So I removed the 1st test case.
