Skip to content

timing test#2035

Merged
alyssawilk merged 1 commit intoenvoyproxy:mainfrom
alyssawilk:timing
Feb 3, 2022
Merged

timing test#2035
alyssawilk merged 1 commit intoenvoyproxy:mainfrom
alyssawilk:timing

Conversation

@alyssawilk
Copy link
Copy Markdown
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>
};
bridge_callbacks_.on_complete = [](envoy_stream_intel, envoy_final_stream_intel final_intel,
void* context) -> void* {
validateStreamIntel(final_intel);
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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