Skip to content

Add TestUtility::ipVersionToString().#24108

Merged
KBaichoo merged 7 commits intoenvoyproxy:mainfrom
bencebeky:ipversion
Nov 29, 2022
Merged

Add TestUtility::ipVersionToString().#24108
KBaichoo merged 7 commits intoenvoyproxy:mainfrom
bencebeky:ipversion

Conversation

@bencebeky
Copy link
Copy Markdown
Contributor

Signed-off-by: Bence Béky bnc@google.com

Commit Message: Add TestUtility::ipVersionToString().
Additional Description: n/a
Risk Level: low, test-only
Testing: test-only change
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Bence Béky <bnc@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

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: #24108 was opened by bencebeky.

see: more, trace.

@bencebeky
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Check envoy-presubmit didn't fail.

🐱

Caused by: a #24108 (comment) was created by @bencebeky.

see: more, trace.

Signed-off-by: Bence Béky <bnc@google.com>
@bencebeky
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24108 (comment) was created by @bencebeky.

see: more, trace.

@bencebeky
Copy link
Copy Markdown
Contributor Author

/assign @KBaichoo

@bencebeky bencebeky marked this pull request as ready for review November 22, 2022 19:59
@bencebeky
Copy link
Copy Markdown
Contributor Author

bencebeky commented Nov 22, 2022

Kevin: PTAL. This is a test-only refactor with no behavior change. Thank you.

Signed-off-by: Bence Béky <bnc@google.com>
@bencebeky
Copy link
Copy Markdown
Contributor Author

Note that the comment #24146 (comment) also applies here. However, this issue is pre-existing, also adding a new IP version is even less likely than adding a new HTTP/1 parser.

Copy link
Copy Markdown
Contributor

@KBaichoo KBaichoo left a comment

Choose a reason for hiding this comment

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

I think there are some additional opportunities to use this in some of the following files:

test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h
test/extensions/transport_sockets/tls/cert_validator/default_validator_integration_test.h
test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc
test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc
test/extensions/http/header_formatters/preserve_case/preserve_case_formatter_reason_phrase_integration_test.cc
test/extensions/http/header_formatters/preserve_case/preserve_case_formatter_integration_test.cc
test/common/quic/test_utils.h
test/common/grpc/grpc_client_integration.h
test/integration/multiplexed_integration_test.cc
test/integration/http2_flood_integration_test.cc
test/integration/sds_dynamic_integration_test.cc
test/integration/ads_integration.h

Thank you for the refactor, looks great otherwise

@KBaichoo
Copy link
Copy Markdown
Contributor

/wait

Signed-off-by: Bence Béky <bnc@google.com>
@bencebeky
Copy link
Copy Markdown
Contributor Author

I think there are some additional opportunities to use this in some of the following files:

test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h
test/extensions/transport_sockets/tls/cert_validator/default_validator_integration_test.h
test/extensions/transport_sockets/tls/integration/ssl_integration_test.cc
test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc
test/extensions/http/header_formatters/preserve_case/preserve_case_formatter_reason_phrase_integration_test.cc
test/extensions/http/header_formatters/preserve_case/preserve_case_formatter_integration_test.cc
test/common/quic/test_utils.h
test/common/grpc/grpc_client_integration.h
test/integration/multiplexed_integration_test.cc
test/integration/http2_flood_integration_test.cc
test/integration/sds_dynamic_integration_test.cc
test/integration/ads_integration.h

Thank you for the refactor, looks great otherwise

Done. It turns out there are three files on this list that this PR has not touched yet until the last commit: the ones in test/extensions/transport_sockets/tls/.

Signed-off-by: Bence Béky <bnc@google.com>
Signed-off-by: Bence Béky <bnc@google.com>
Signed-off-by: Bence Béky <bnc@google.com>
Copy link
Copy Markdown
Contributor

@KBaichoo KBaichoo left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@KBaichoo KBaichoo enabled auto-merge (squash) November 29, 2022 16:43
@KBaichoo
Copy link
Copy Markdown
Contributor

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24108 (comment) was created by @KBaichoo.

see: more, trace.

@bencebeky
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #24108 (comment) was created by @bencebeky.

see: more, trace.

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.

3 participants