Skip to content

test: add process_object_ to BaseIntegrationTest#7183

Merged
alyssawilk merged 2 commits intoenvoyproxy:masterfrom
ahedberg:process_context_in_tests
Jun 18, 2019
Merged

test: add process_object_ to BaseIntegrationTest#7183
alyssawilk merged 2 commits intoenvoyproxy:masterfrom
ahedberg:process_context_in_tests

Conversation

@ahedberg
Copy link
Copy Markdown
Contributor

@ahedberg ahedberg commented Jun 5, 2019

Signed-off-by: Ashley Hedberg ahedberg@google.com

Description: Adds a process_object_ member to BaseIntegrationTest. If an integration test subclassing from BaseIntegrationTest sets this member, the envoy server will be started with a ProcessContext referencing that object.
Risk Level: low (test-only, defaults to nullopt)
Testing: bazel test //test/... - not sure if stats_integration_test failures were related. Also not sure where to add a test for this, but happy to do so (I have an internal test that proves this does what I wanted it to).
Docs Changes: none
Release Notes: n/a
Fixes #6969

Signed-off-by: Ashley Hedberg <ahedberg@google.com>
@lizan lizan requested a review from junr03 June 5, 2019 21:52
@ahedberg
Copy link
Copy Markdown
Contributor Author

ahedberg commented Jun 7, 2019

Friendly ping @junr03?

@junr03
Copy link
Copy Markdown
Member

junr03 commented Jun 7, 2019

Sorry, I have been flying yesterday and today. I took a look at the code and it looks reasonable. I feel the same way about this as mine and Alyssa's comments here. @jmarantz might have a good suggestion for where to drop a test that exercises this.

Signed-off-by: Ashley Hedberg <ahedberg@google.com>
@ahedberg
Copy link
Copy Markdown
Contributor Author

Test added, PTAL. Thanks!

@ahedberg
Copy link
Copy Markdown
Contributor Author

Ping @junr03 ? Or @htuch who reviewed the PR adding this feature?

@ahedberg
Copy link
Copy Markdown
Contributor Author

Ping @junr03

Copy link
Copy Markdown
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

awesome, lgtm! Thanks @ahedberg. @htuch can you take a last pass?

@junr03 junr03 requested a review from htuch June 17, 2019 20:39
@ahedberg
Copy link
Copy Markdown
Contributor Author

Thanks for the review! Harvey is out of office until Thursday--Alyssa, mind doing the final pass?

/assign @alyssawilk

@alyssawilk alyssawilk merged commit d2068d3 into envoyproxy:master Jun 18, 2019
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.

Proposal: ProcessContext

4 participants