test: Refactor OptionsImplTest#651
Merged
mattklein123 merged 1 commit intoenvoyproxy:masterfrom Mar 29, 2017
Merged
Conversation
OptionsImpl requires a char** argv, but let's hide that behind a helper function so that each test can just have a string containing an entire command line, instead of needing to break it up. This is preparatory to adding a few more tests as part of envoyproxy#613.
| EXPECT_EXIT(OptionsImpl(argv.size(), const_cast<char**>(&argv[0]), "1", spdlog::level::warn), | ||
| testing::ExitedWithCode(0), ""); | ||
| for (const std::string& s : words) { | ||
| argv.push_back(s.c_str()); |
Member
There was a problem hiding this comment.
This won't actually work. You are storing reference to temporary which will get destroyed when func returns.
Contributor
Author
There was a problem hiding this comment.
That's why I construct the OptionsImpl within the function -- after that we don't need it, right?
Member
There was a problem hiding this comment.
Sigh, sorry, I'm not having a good code review day. I need to spend more time looking. Yes this is fine.
htuch
reviewed
Mar 29, 2017
Member
htuch
left a comment
There was a problem hiding this comment.
LGTM, but I personally would have considered just using C++ vector literals, e.g. {"envoy", "--concurrency", "2", "-c", etc, since this is also concise and would allow hypothetical tests to be written that have non-trivial space escaping or quoting.
Member
|
I'm fine with this change, but if you want to do @htuch suggestion that's fine also. LMK. |
Contributor
Author
|
I did think about vector literals, but I just couldn't think of a case where we'd feel the need to test that -- in real life we get handed a char** to start with, so this is purely for testing convenience. Happy to go the other way if anyone feels strongly about it. |
mattklein123
approved these changes
Mar 29, 2017
PiotrSikora
added a commit
to jplevyak/envoy
that referenced
this pull request
Oct 9, 2020
Import upstream wasm
jpsim
pushed a commit
that referenced
this pull request
Nov 28, 2022
Description: update abseil to track upstream Envoy. This will also allow us to test @lizan's suggestion #9654 (comment) Risk Level: low. keeps track with Envoy. Testing: CI Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
jpsim
pushed a commit
that referenced
this pull request
Nov 29, 2022
Description: update abseil to track upstream Envoy. This will also allow us to test @lizan's suggestion #9654 (comment) Risk Level: low. keeps track with Envoy. Testing: CI Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
vehre-x41
pushed a commit
to vehre-x41/envoy
that referenced
this pull request
Jan 18, 2023
…oyproxy#651) * build(deps): bump setuptools from 59.0.1 to 59.5.0 in /tools/base (envoyproxy#606) Bumps [setuptools](https://github.com/pypa/setuptools) from 59.0.1 to 59.5.0. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/main/CHANGES.rst) - [Commits](pypa/setuptools@v59.0.1...v59.5.0) --- updated-dependencies: - dependency-name: setuptools dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump setuptools from 59.5.0 to 60.1.0 in /tools/base (envoyproxy#620) Bumps [setuptools](https://github.com/pypa/setuptools) from 59.5.0 to 60.1.0. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/main/CHANGES.rst) - [Commits](pypa/setuptools@v59.5.0...v60.1.0) --- updated-dependencies: - dependency-name: setuptools dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * add support for validating header keys and values fixes envoyproxy#627 Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * address PR feedback Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * add quic header key/value validation Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * add runtime guard: envoy.reloadable_features.validate_upstream_headers Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * honor runtime guard Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * update integration tests for invalid header key/value validation Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix http/2 integration tests for invalid header values Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * use nghttp2 for header name and value rfc compliance checks Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * bypass assertions to set invalid header key/value for protocol integration tests Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix bad merge on changelog Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix unit invalid header integration tests Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix tests Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * add entry for new envoy.reloadable_features.validate_upstream_headers runtime guard Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * clarify validate_upstream_headers guard Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * disable validate_upstream_headers guard Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix build Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix unit tests Signed-off-by: Adam Meily <adam.meily@trailofbits.com> * fix unit test Signed-off-by: Adam Meily <adam.meily@trailofbits.com> Signed-off-by: Adam Meily <adam.meily@trailofbits.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
mathetake
added a commit
that referenced
this pull request
Mar 3, 2026
**Commit Message** `fakeBackends` were utilized before #620 but after that, it's effectively not used anymore. This refactors tests/extproc around on the backend definition so that it will be clearer that the always-failing backend is configured explicitly. --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OptionsImpl requires a char** argv, but let's hide that behind a helper
function so that each test can just have a string containing an entire
command line, instead of needing to break it up.
This is preparatory to adding a few more tests as part of #613.