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

timing test #2035

Merged
merged 1 commit into from
Feb 3, 2022
Merged

timing test #2035

merged 1 commit into from
Feb 3, 2022

Conversation

alyssawilk
Copy link
Contributor

Signed-off-by: Alyssa Wilk alyssar@chromium.org

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk merged commit cd2a10c into envoyproxy:main Feb 3, 2022
jpsim added a commit to jpsim/envoy-mobile that referenced this pull request Feb 3, 2022
* main:
  timing test (envoyproxy#2035)
  [CI] Update Xcode to 13.2.1 (envoyproxy#1997)
  owners: adding charles (envoyproxy#2038)
  [OWNERS] Update Envoy Mobile owners (envoyproxy#2031)
  [deps] Update rules_android to a stable release URL (envoyproxy#2032)

Signed-off-by: JP Simard <jp@jpsim.com>
@@ -82,6 +106,7 @@ class ClientIntegrationTest : public BaseIntegrationTest,
};
bridge_callbacks_.on_complete = [](envoy_stream_intel, envoy_final_stream_intel final_intel,
void* context) -> void* {
validateStreamIntel(final_intel);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is doing asserts inside a callback a usual strategy for C++ tests in Envoy?

https://g3doc.corp.google.com/eng/doc/devguide/testing/unit/best-practices.md?cl=head&polyglot=cpp#testing-callbacks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

arguably not optimal but I guess I found it cleaner than a ton of copy-paste code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't worry - I checked other EM tests, and assert in callbacks seems rather popular.

jpsim added a commit to jpsim/envoy-mobile that referenced this pull request Feb 3, 2022
* main:
  envoy: 71248e512 (envoyproxy#2027)
  Migrate EngFlow configurations from rbe_autoconfig to rbe_configs_gen (envoyproxy#2037)
  timing test (envoyproxy#2035)
  [CI] Update Xcode to 13.2.1 (envoyproxy#1997)
  owners: adding charles (envoyproxy#2038)

Signed-off-by: JP Simard <jp@jpsim.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