[balsa] Parametrize IntegrationTest on HTTP/1 implementation.#24254
[balsa] Parametrize IntegrationTest on HTTP/1 implementation.#24254KBaichoo merged 6 commits intoenvoyproxy:mainfrom bencebeky:integrationtest
Conversation
Run IntegrationTest with BalsaParser in addition to http-parser. Disable failing tests for now. Tracking issue: #21245 Signed-off-by: Bence Béky <bnc@google.com>
test/integration/integration_test.cc
Outdated
| } | ||
|
|
||
| std::string testParamToString( | ||
| const ::testing::TestParamInfo<std::tuple<Network::Address::IpVersion, Http1ParserImpl>>& |
There was a problem hiding this comment.
nit: I think there's a preference here to avoid fully qualifying names with the leading "::" if possible, unless there's a using declaration or namespace alias. I like using the decision tree at internal TOTW 151 as a reference (unfortunately, there is no analogue at https://abseil.io/tips/ - the closest thing there seems to be https://abseil.io/tips/130).
There was a problem hiding this comment.
Good catch. I was not aware of that guidance, thank you for digging it up.
There was a problem hiding this comment.
On that note, also adding using directives for Combine, Values, and ValuesIn.
test/integration/integration_test.cc
Outdated
| testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), | ||
| TestUtility::ipTestParamsToString); | ||
| INSTANTIATE_TEST_SUITE_P( | ||
| IpVersions, IntegrationTest, |
There was a problem hiding this comment.
optional: s/IpVersions/IpVersionsAndHttp1ParserVersions/ or similar.
There was a problem hiding this comment.
How about IpVersionsAndHttp1Parser as a compromise for brevity?
Signed-off-by: Bence Béky <bnc@google.com>
Signed-off-by: Bence Béky <bnc@google.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
/assign @KBaichoo Kevin: PTAL, thanks! |
KBaichoo
left a comment
KBaichoo
left a comment
There was a problem hiding this comment.
Thank you for your contribution.
|
can you try merging? I think #24383 might have renamed some jobs so CI is waiting on them to occur but they won't be unless it's ran again |
Signed-off-by: Bence Béky <bnc@google.com>
Thanks for the advice. |
Signed-off-by: Bence Béky <bnc@google.com>
|
@bencebeky im confused that this is failing format checks - seems unrelated and merged to /retest |
|
Retrying Azure Pipelines: |
Signed-off-by: Bence Béky <bnc@google.com>
|
/retest |
|
Retrying Azure Pipelines: |
6 participants
Run IntegrationTest with BalsaParser in addition to http-parser. Disable failing tests for now.
Tracking issue: #21245
Signed-off-by: Bence Béky bnc@google.com
Commit Message: [balsa] Parametrize IntegrationTest on HTTP/1 implementation.
Additional Description: Run IntegrationTest with BalsaParser in addition to http-parser. Disable failing tests for now.
Risk Level: low, test-only change
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a