Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues found when creating integration tests #951

Merged
merged 4 commits into from
Dec 2, 2022

Conversation

dubious90
Copy link
Contributor

The resulting PR was too large, so splitting these fixes out from the full integration PR:

  1. FakeUserDefinedOutputPlugin is now thread safe.
  2. AggregateGlobalOutput takes in UserDefinedOutput so that it can account for errors
  • Previously there was no way for the global aggregate to know about missing outputs which could have created confusion for users
  1. FakeUserDefinedOutput - handleResponseData now ignores empty data responses
  1. fake_user_defined_output plugin can no longer be test only because of a dependency chain that leads to non-test code, such as benchmarks and dynamic_config. It is too complex to break this dependency chain at the moment.

Nathan Perry and others added 3 commits November 29, 2022 15:40
@dubious90 dubious90 changed the title Fix issues found when doing integration tests Fix issues found when creating integration tests Nov 30, 2022
@dubious90
Copy link
Contributor Author

@fei-deng please review and assign to @mum4k when ready

@dubious90 dubious90 marked this pull request as ready for review November 30, 2022 21:48
@dubious90 dubious90 added the waiting-for-review A PR waiting for a review. label Dec 1, 2022
fei-deng
fei-deng previously approved these changes Dec 1, 2022
Copy link
Contributor

@fei-deng fei-deng left a comment

Choose a reason for hiding this comment

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

LGTM. Nice change dubious90.

@mum4k mum4k added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Dec 2, 2022
Signed-off-by: Nathan Perry <nbperry@google.com>
@dubious90 dubious90 added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Dec 2, 2022
@mum4k mum4k merged commit a8cf502 into envoyproxy:main Dec 2, 2022
@dubious90 dubious90 deleted the response-plugin-statuses branch December 7, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-review A PR waiting for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants