Skip to content

integration test: remove unused overloaded constructor#5080

Merged
htuch merged 2 commits intoenvoyproxy:masterfrom
talnordan:base-integration-1
Nov 19, 2018
Merged

integration test: remove unused overloaded constructor#5080
htuch merged 2 commits intoenvoyproxy:masterfrom
talnordan:base-integration-1

Conversation

@talnordan
Copy link
Copy Markdown
Member

In PR #4512, the 2-arg variant of BaseIntegrationTest's constructor
was left temporarily, to allow echo2_integration_test.cc to compile.

This overloaded constructor can safely be removed once the following PR
is merged to the envoyproxy/envoy-filter-example repository:
envoyproxy/envoy-filter-example#69

Risk Level: Low
Testing: bazel test //test/...
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Tal Nordan github@talnordan.com

In PR envoyproxy#4512, the 2-arg variant of `BaseIntegrationTest`'s constructor
was left temporarily, to allow `echo2_integration_test.cc` to compile.

This overloaded constructor can safely be removed once the following PR
is merged to the envoyproxy/envoy-filter-example repository:
envoyproxy/envoy-filter-example#69

Risk Level: Low
Testing: `bazel test //test/...`
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Tal Nordan <github@talnordan.com>
mattklein123
mattklein123 previously approved these changes Nov 18, 2018
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@talnordan
Copy link
Copy Markdown
Member Author

Both failing checks seem to be caused by the fact that CircleCI uses an outdated revision of envoyproxy/envoy-filter-example:

Cloning into '/build/envoy-filter-example'...
⋮
Note: checking out '3e5b73305b961526ffcee7584251692a9a3ce4b3'.
⋮
HEAD is now at 3e5b733... cleanup: match NOT_IMPLEMENTED_GCOVR_EXCL_LINE change (#53)
⋮
echo2_integration_test.cc:14:28: error: no matching constructor for initialization of 'Envoy::BaseIntegrationTest'
  Echo2IntegrationTest() : BaseIntegrationTest(GetParam(), echoConfig()) {}
                           ^                   ~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Tal Nordan <github@talnordan.com>
@mattklein123
Copy link
Copy Markdown
Member

@talnordan yeah looks like you figured it out. @htuch I think is going to fix this so it's less painful.

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks!

@htuch htuch merged commit 6c2c7b5 into envoyproxy:master Nov 19, 2018
@htuch
Copy link
Copy Markdown
Member

htuch commented Nov 19, 2018

Yep, will try and do the repo convergence this week.

@talnordan talnordan deleted the base-integration-1 branch November 20, 2018 22:30
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
In PR envoyproxy#4512, the 2-arg variant of BaseIntegrationTest's constructor
was left temporarily, to allow echo2_integration_test.cc to compile.

This overloaded constructor can safely be removed once the following PR
is merged to the envoyproxy/envoy-filter-example repository:
envoyproxy/envoy-filter-example#69

Risk Level: Low
Testing: bazel test //test/...
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Tal Nordan <github@talnordan.com>
Signed-off-by: Fred Douglas <fredlas@google.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.

3 participants