From acaf2b64335f9e327805f25d35ebeff13fe9e568 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 3 Sep 2019 16:35:43 -0700 Subject: [PATCH 01/19] Add line on installing XCode Signed-off-by: Lisa Lu --- ci/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/README.md b/ci/README.md index 04864d581498..857e5f9595ab 100644 --- a/ci/README.md +++ b/ci/README.md @@ -133,7 +133,8 @@ The macOS CI build is part of the [CircleCI](https://circleci.com/gh/envoyproxy/ Dependencies are installed by the `ci/mac_ci_setup.sh` script, via [Homebrew](https://brew.sh), which is pre-installed on the CircleCI macOS image. The dependencies are cached are re-installed on every build. The `ci/mac_ci_steps.sh` script executes the specific commands that -build and test Envoy. +build and test Envoy. If Envoy cannot be built (`error: /Library/Developer/CommandLineTools/usr/bin/libtool: no output file specified (specify with -o output) +`), ensure that XCode is installed. # Coverity Scan Build Flow From 0a6aec7b8d5018d877d2b175c2f40c1f361006be Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Wed, 4 Sep 2019 14:30:57 -0700 Subject: [PATCH 02/19] Try again with sign-off Signed-off-by: Lisa Lu --- bazel/README.md | 1 + ci/README.md | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bazel/README.md b/bazel/README.md index b99b9e50621e..499dd46b80cf 100644 --- a/bazel/README.md +++ b/bazel/README.md @@ -65,6 +65,7 @@ for how to update or override dependencies. ``` _notes_: `coreutils` is used for `realpath`, `gmd5sum` and `gsha256sum` + XCode is also required to build Envoy on macOS. Envoy compiles and passes tests with the version of clang installed by XCode 9.3.0: Apple LLVM version 9.1.0 (clang-902.0.30). diff --git a/ci/README.md b/ci/README.md index 857e5f9595ab..72d1600fac52 100644 --- a/ci/README.md +++ b/ci/README.md @@ -133,8 +133,8 @@ The macOS CI build is part of the [CircleCI](https://circleci.com/gh/envoyproxy/ Dependencies are installed by the `ci/mac_ci_setup.sh` script, via [Homebrew](https://brew.sh), which is pre-installed on the CircleCI macOS image. The dependencies are cached are re-installed on every build. The `ci/mac_ci_steps.sh` script executes the specific commands that -build and test Envoy. If Envoy cannot be built (`error: /Library/Developer/CommandLineTools/usr/bin/libtool: no output file specified (specify with -o output) -`), ensure that XCode is installed. +build and test Envoy. If Envoy cannot be built (`error: /Library/Developer/CommandLineTools/usr/bin/libtool: no output file specified (specify with -o output)`), +ensure that XCode is installed. # Coverity Scan Build Flow From b08e9e7251f60c26f96fde84ebe9fe4e4e987b72 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 11:20:36 -0700 Subject: [PATCH 03/19] Add flag for printing failed tests only Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 20 +++++++++++++++++--- test/tools/router_check/router.h | 15 ++++++++++++++- test/tools/router_check/router_check.cc | 4 ++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index e8e2143f4b8f..f95d21aad7b7 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -108,7 +108,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); std::string test_name = check_config->getString("test_name", ""); - if (details_) { + if (details_ && !only_show_failures_) { std::cout << test_name << std::endl; } Json::ObjectSharedPtr validate = check_config->getObject("validate"); @@ -137,6 +137,9 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso compareResults("", expected, test.first); } else { if (!test.second(tool_config, expected)) { + if (only_show_failures_) { + std::cout << "in test: " << test_name << std::endl; + } no_failures = false; } } @@ -147,6 +150,9 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("header_fields")) { if (!compareHeaderField(tool_config, header_field->getString("field"), header_field->getString("value"))) { + if (only_show_failures_) { + std::cout << "in test: " << test_name << std::endl; + } no_failures = false; } } @@ -157,6 +163,9 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso validate->getObjectArray("custom_header_fields")) { if (!compareCustomHeaderField(tool_config, header_field->getString("field"), header_field->getString("value"))) { + if (only_show_failures_) { + std::cout << "in test: " << test_name << std::endl; + } no_failures = false; } } @@ -183,7 +192,7 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); const std::string& test_name = check_config.test_name(); - if (details_) { + if (details_ && !only_show_failures_) { std::cout << test_name << std::endl; } const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); @@ -204,6 +213,9 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { // Call appropriate function for each match case. for (const auto& test : checkers) { if (!test(tool_config, validate)) { + if (only_show_failures_) { + std::cout << "in test: " << test_name << std::endl; + } no_failures = false; } } @@ -426,7 +438,7 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin } // Output failure details to stdout if details_ flag is set to true - if (details_) { + if (details_ || only_show_failures_) { std::cerr << "expected: [" << expected << "], actual: [" << actual << "], test type: " << test_type << std::endl; } @@ -446,6 +458,7 @@ Options::Options(int argc, char** argv) { TCLAP::CmdLine cmd("router_check_tool", ' ', "none", true); TCLAP::SwitchArg is_proto("p", "useproto", "Use Proto test file schema", cmd, false); TCLAP::SwitchArg is_detailed("d", "details", "Show detailed test execution results", cmd, false); + TCLAP::SwitchArg only_show_failures("o", "only-show-failures", "Only display failing tests", cmd, false); TCLAP::SwitchArg disable_deprecation_check("", "disable-deprecation-check", "Disable deprecated fields check", cmd, false); TCLAP::ValueArg fail_under("f", "fail-under", @@ -468,6 +481,7 @@ Options::Options(int argc, char** argv) { is_proto_ = is_proto.getValue(); is_detailed_ = is_detailed.getValue(); + only_show_failures_ = only_show_failures.getValue(); fail_under_ = fail_under.getValue(); comprehensive_coverage_ = comprehensive_coverage.getValue(); disable_deprecation_check_ = disable_deprecation_check.getValue(); diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index e2156afa1072..8f764ba24e1d 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -91,6 +91,11 @@ class RouterCheckTool : Logger::Loggable { */ void setShowDetails() { details_ = true; } + /** + * Set whether to only print failing match cases. + */ + void onlyShowFailures() { only_show_failures_ = true; } + float coverage(bool detailed) { return detailed ? coverage_.detailedReport() : coverage_.report(); } @@ -144,6 +149,8 @@ class RouterCheckTool : Logger::Loggable { bool details_{false}; + bool only_show_failures_{false}; + // TODO(hennna): Switch away from mocks following work done by @rlazarus in github issue #499. std::unique_ptr> factory_context_; std::unique_ptr config_; @@ -196,10 +203,15 @@ class Options { bool isProto() const { return is_proto_; } /** - * @return true is detailed test execution results are displayed. + * @return true if detailed test execution results are displayed. */ bool isDetailed() const { return is_detailed_; } + /** + * @return true if only test failures are displayed. + */ + bool onlyShowFailures() const { return only_show_failures_; } + /** * @return true if the deprecated field check for RouteConfiguration is disabled. */ @@ -214,6 +226,7 @@ class Options { bool comprehensive_coverage_; bool is_proto_; bool is_detailed_; + bool only_show_failures_; bool disable_deprecation_check_; }; } // namespace Envoy diff --git a/test/tools/router_check/router_check.cc b/test/tools/router_check/router_check.cc index f3856e285d8c..88cd0c194507 100644 --- a/test/tools/router_check/router_check.cc +++ b/test/tools/router_check/router_check.cc @@ -18,6 +18,10 @@ int main(int argc, char* argv[]) { checktool.setShowDetails(); } + if (options.onlyShowFailures()) { + checktool.onlyShowFailures(); + } + bool is_equal = options.isProto() ? checktool.compareEntries(options.testPath()) : checktool.compareEntriesInJson(options.unlabelledTestPath()); From cfc566fb6abebfa866e0d28884099d6ef92b030f Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 11:58:20 -0700 Subject: [PATCH 04/19] Add unit tests, fix formatting Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 3 ++- test/tools/router_check/test/route_tests.sh | 26 +++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index f95d21aad7b7..f45bf406ee31 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -458,7 +458,8 @@ Options::Options(int argc, char** argv) { TCLAP::CmdLine cmd("router_check_tool", ' ', "none", true); TCLAP::SwitchArg is_proto("p", "useproto", "Use Proto test file schema", cmd, false); TCLAP::SwitchArg is_detailed("d", "details", "Show detailed test execution results", cmd, false); - TCLAP::SwitchArg only_show_failures("o", "only-show-failures", "Only display failing tests", cmd, false); + TCLAP::SwitchArg only_show_failures("o", "only-show-failures", "Only display failing tests", cmd, + false); TCLAP::SwitchArg disable_deprecation_check("", "disable-deprecation-check", "Disable deprecated fields check", cmd, false); TCLAP::ValueArg fail_under("f", "fail-under", diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index 1e85bea3f598..4a0044839440 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -64,10 +64,32 @@ if [[ "${BAD_CONFIG_OUTPUT}" != *"Unable to parse"* ]]; then exit 1 fi -# Failure test case -echo "testing failure test case" +# Failure output flag test cases +echo "testing failure test cases" +# Failure test case with only details flag set FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi + +# Failure test case with details flag set and failures flag set +FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" "--only-show-failures" 2>&1) || + echo "${FAILURE_OUTPUT:-no-output}" +if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"*"in test: Test_2"* ]]; then + exit 1 +fi + +# Failure test case with details flag unset and failures flag set +FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--only-show-failures" 2>&1) || + echo "${FAILURE_OUTPUT:-no-output}" +if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"*"in test: Test_2"* ]]; then + exit 1 +fi + +# Failure test case with neither flag set +FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" 2>&1) || + echo "${FAILURE_OUTPUT:-no-output}" +if [[ "${FAILURE_OUTPUT}" != "" ]]; then + exit 1 +fi \ No newline at end of file From 7272326b872900cf8367819d716f1386f10a0dcb Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 12:02:29 -0700 Subject: [PATCH 05/19] newline Signed-off-by: Lisa Lu --- test/tools/router_check/test/route_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index 4a0044839440..6c2686ac2195 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -92,4 +92,4 @@ FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/ echo "${FAILURE_OUTPUT:-no-output}" if [[ "${FAILURE_OUTPUT}" != "" ]]; then exit 1 -fi \ No newline at end of file +fi From f58805cbbe1e8fa87a5e4a7e3199d1a33443a906 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 13:14:43 -0700 Subject: [PATCH 06/19] Remove no-output test Signed-off-by: Lisa Lu --- test/tools/router_check/test/route_tests.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index 6c2686ac2195..3c78ecb0bdef 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -86,10 +86,3 @@ FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/ if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"*"in test: Test_2"* ]]; then exit 1 fi - -# Failure test case with neither flag set -FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" 2>&1) || - echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != "" ]]; then - exit 1 -fi From 0609bdb655badf99a1b800a777585c768b3f30a9 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 17:06:20 -0700 Subject: [PATCH 07/19] Update docs and version history Signed-off-by: Lisa Lu --- docs/root/install/tools/route_table_check_tool.rst | 3 +++ docs/root/intro/version_history.rst | 1 + 2 files changed, 4 insertions(+) diff --git a/docs/root/install/tools/route_table_check_tool.rst b/docs/root/install/tools/route_table_check_tool.rst index 8acb2ac34a9d..ff8e6653a9ca 100644 --- a/docs/root/install/tools/route_table_check_tool.rst +++ b/docs/root/install/tools/route_table_check_tool.rst @@ -37,6 +37,9 @@ Usage -d, --details Show detailed test execution results. The first line indicates the test name. + -o, --only-show-failures + Displays test names for failed tests. Omits test names for passing tests if the details flag is set. + -p, --useproto Use Proto test file schema diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index b10f9ce98b29..09ee99f62d84 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -48,6 +48,7 @@ Version history * router check tool: add coverage reporting & enforcement. * router check tool: add comprehensive coverage reporting. * router check tool: add deprecated field check. +* router check tool: add flag for only printing names of failed tests. * tls: added verification of IP address SAN fields in certificates against configured SANs in the * tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP. certificate validation context. From 474eee321bdaa4032dbc94163855d71b8a361418 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 17:18:29 -0700 Subject: [PATCH 08/19] Remove shorthand Signed-off-by: Lisa Lu --- docs/root/install/tools/route_table_check_tool.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/install/tools/route_table_check_tool.rst b/docs/root/install/tools/route_table_check_tool.rst index ff8e6653a9ca..c3c142bdef05 100644 --- a/docs/root/install/tools/route_table_check_tool.rst +++ b/docs/root/install/tools/route_table_check_tool.rst @@ -37,7 +37,7 @@ Usage -d, --details Show detailed test execution results. The first line indicates the test name. - -o, --only-show-failures + --only-show-failures Displays test names for failed tests. Omits test names for passing tests if the details flag is set. -p, --useproto From a642ba5f3ddabaa58008a4dc8c81645af02cf861 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Thu, 5 Sep 2019 17:34:27 -0700 Subject: [PATCH 09/19] Change method name and remove call from deprecated method Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 2 +- test/tools/router_check/router.h | 2 +- test/tools/router_check/router_check.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index f45bf406ee31..a41e6f9af369 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -108,7 +108,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); std::string test_name = check_config->getString("test_name", ""); - if (details_ && !only_show_failures_) { + if (details_) { std::cout << test_name << std::endl; } Json::ObjectSharedPtr validate = check_config->getObject("validate"); diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index 8f764ba24e1d..8cbdfa5ddd76 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -94,7 +94,7 @@ class RouterCheckTool : Logger::Loggable { /** * Set whether to only print failing match cases. */ - void onlyShowFailures() { only_show_failures_ = true; } + void setOnlyShowFailures() { only_show_failures_ = true; } float coverage(bool detailed) { return detailed ? coverage_.detailedReport() : coverage_.report(); diff --git a/test/tools/router_check/router_check.cc b/test/tools/router_check/router_check.cc index 88cd0c194507..1792af5c5b35 100644 --- a/test/tools/router_check/router_check.cc +++ b/test/tools/router_check/router_check.cc @@ -19,7 +19,7 @@ int main(int argc, char* argv[]) { } if (options.onlyShowFailures()) { - checktool.onlyShowFailures(); + checktool.setOnlyShowFailures(); } bool is_equal = options.isProto() From 0f0fc9e1130c9cf6ac9af3157e7a006eac64e1d2 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Mon, 9 Sep 2019 11:51:41 -0700 Subject: [PATCH 10/19] Restructure test result output Signed-off-by: Lisa Lu --- .../install/tools/route_table_check_tool.rst | 2 +- docs/root/intro/version_history.rst | 2 +- test/tools/router_check/router.cc | 126 ++++++++---------- test/tools/router_check/router.h | 44 +++--- test/tools/router_check/test/route_tests.sh | 13 +- 5 files changed, 94 insertions(+), 93 deletions(-) diff --git a/docs/root/install/tools/route_table_check_tool.rst b/docs/root/install/tools/route_table_check_tool.rst index c3c142bdef05..186df20b130e 100644 --- a/docs/root/install/tools/route_table_check_tool.rst +++ b/docs/root/install/tools/route_table_check_tool.rst @@ -38,7 +38,7 @@ Usage Show detailed test execution results. The first line indicates the test name. --only-show-failures - Displays test names for failed tests. Omits test names for passing tests if the details flag is set. + Displays test results for failed tests. Omits test names for passing tests if the details flag is set. -p, --useproto Use Proto test file schema diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 29cf547bdbea..e503c6172c7a 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -48,7 +48,7 @@ Version history * router check tool: add coverage reporting & enforcement. * router check tool: add comprehensive coverage reporting. * router check tool: add deprecated field check. -* router check tool: add flag for only printing names of failed tests. +* router check tool: add flag for only printing results of failed tests. * thrift_proxy: fix crashing bug on invalid transport/protocol framing * tls: added verification of IP address SAN fields in certificates against configured SANs in the * tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP. diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index a41e6f9af369..6ba347e1ad4b 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -113,7 +113,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso } Json::ObjectSharedPtr validate = check_config->getObject("validate"); - using checkerFunc = std::function; + using checkerFunc = std::function; const std::unordered_map checkers = { {"cluster_name", [this](auto&... params) -> bool { return this->compareCluster(params...); }}, @@ -134,12 +134,9 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso if (validate->hasObject(test.first)) { const std::string& expected = validate->getString(test.first); if (tool_config.route_ == nullptr) { - compareResults("", expected, test.first); + compareResults("", expected, test.first, test_name); } else { - if (!test.second(tool_config, expected)) { - if (only_show_failures_) { - std::cout << "in test: " << test_name << std::endl; - } + if (!test.second(tool_config, expected, test_name)) { no_failures = false; } } @@ -149,10 +146,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso if (validate->hasObject("header_fields")) { for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("header_fields")) { if (!compareHeaderField(tool_config, header_field->getString("field"), - header_field->getString("value"))) { - if (only_show_failures_) { - std::cout << "in test: " << test_name << std::endl; - } + header_field->getString("value"), test_name)) { no_failures = false; } } @@ -162,10 +156,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("custom_header_fields")) { if (!compareCustomHeaderField(tool_config, header_field->getString("field"), - header_field->getString("value"))) { - if (only_show_failures_) { - std::cout << "in test: " << test_name << std::endl; - } + header_field->getString("value"), test_name)) { no_failures = false; } } @@ -192,13 +183,11 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); const std::string& test_name = check_config.test_name(); - if (details_ && !only_show_failures_) { - std::cout << test_name << std::endl; - } + tests_.insert({test_name, {}}); const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); using checkerFunc = - std::function; + std::function; checkerFunc checkers[] = { [this](auto&... params) -> bool { return this->compareCluster(params...); }, [this](auto&... params) -> bool { return this->compareVirtualCluster(params...); }, @@ -212,25 +201,32 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { // Call appropriate function for each match case. for (const auto& test : checkers) { - if (!test(tool_config, validate)) { - if (only_show_failures_) { - std::cout << "in test: " << test_name << std::endl; - } + if (!test(tool_config, validate, test_name)) { no_failures = false; } } } + // Output failure details to stdout if details_ flag is set to true + for (const auto& test_result : tests_) { + // All test names are printed if the details_ flag is true unless only_show_failures_ is also true. + if ((details_ && !only_show_failures_) || (only_show_failures_ && !test_result.second.empty())) { + std::cout << test_result.first << std::endl; + for (const auto& failure : test_result.second) { + std::cerr << failure << std::endl; + } + } + } return no_failures; } -bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& expected) { +bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr) { actual = tool_config.route_->routeEntry()->clusterName(); } - const bool matches = compareResults(actual, expected, "cluster_name"); + const bool matches = compareResults(actual, expected, "cluster_name", test_name); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markClusterCovered(*tool_config.route_->routeEntry()); } @@ -238,17 +234,17 @@ bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& } bool RouterCheckTool::compareCluster( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { if (!expected.has_cluster_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.cluster_name().value(), "cluster_name"); + return compareResults("", expected.cluster_name().value(), "cluster_name", test_name); } - return compareCluster(tool_config, expected.cluster_name().value()); + return compareCluster(tool_config, expected.cluster_name().value(), test_name); } -bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std::string& expected) { +bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr && @@ -257,7 +253,7 @@ bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std:: tool_config.route_->routeEntry()->virtualCluster(*tool_config.headers_)->statName(); actual = tool_config.symbolTable().toString(stat_name); } - const bool matches = compareResults(actual, expected, "virtual_cluster_name"); + const bool matches = compareResults(actual, expected, "virtual_cluster_name", test_name); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markVirtualClusterCovered(*tool_config.route_->routeEntry()); } @@ -265,23 +261,23 @@ bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std:: } bool RouterCheckTool::compareVirtualCluster( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { if (!expected.has_virtual_cluster_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.virtual_cluster_name().value(), "virtual_cluster_name"); + return compareResults("", expected.virtual_cluster_name().value(), "virtual_cluster_name", test_name); } - return compareVirtualCluster(tool_config, expected.virtual_cluster_name().value()); + return compareVirtualCluster(tool_config, expected.virtual_cluster_name().value(), test_name); } -bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::string& expected) { +bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr) { Stats::StatName stat_name = tool_config.route_->routeEntry()->virtualHost().statName(); actual = tool_config.symbolTable().toString(stat_name); } - const bool matches = compareResults(actual, expected, "virtual_host_name"); + const bool matches = compareResults(actual, expected, "virtual_host_name", test_name); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markVirtualHostCovered(*tool_config.route_->routeEntry()); } @@ -289,17 +285,17 @@ bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareVirtualHost( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { if (!expected.has_virtual_host_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.virtual_host_name().value(), "virtual_host_name"); + return compareResults("", expected.virtual_host_name().value(), "virtual_host_name", test_name); } - return compareVirtualHost(tool_config, expected.virtual_host_name().value()); + return compareVirtualHost(tool_config, expected.virtual_host_name().value(), test_name); } -bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected) { +bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -312,7 +308,7 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str actual = tool_config.headers_->get_(Http::Headers::get().Path); } - const bool matches = compareResults(actual, expected, "path_rewrite"); + const bool matches = compareResults(actual, expected, "path_rewrite", test_name); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markPathRewriteCovered(*tool_config.route_->routeEntry()); } @@ -320,17 +316,17 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareRewritePath( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { if (!expected.has_path_rewrite()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.path_rewrite().value(), "path_rewrite"); + return compareResults("", expected.path_rewrite().value(), "path_rewrite", test_name); } - return compareRewritePath(tool_config, expected.path_rewrite().value()); + return compareRewritePath(tool_config, expected.path_rewrite().value(), test_name); } -bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected) { +bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -343,7 +339,7 @@ bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::str actual = tool_config.headers_->get_(Http::Headers::get().Host); } - const bool matches = compareResults(actual, expected, "host_rewrite"); + const bool matches = compareResults(actual, expected, "host_rewrite", test_name); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markHostRewriteCovered(*tool_config.route_->routeEntry()); } @@ -351,23 +347,23 @@ bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareRewriteHost( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { if (!expected.has_host_rewrite()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.host_rewrite().value(), "host_rewrite"); + return compareResults("", expected.host_rewrite().value(), "host_rewrite", test_name); } - return compareRewriteHost(tool_config, expected.host_rewrite().value()); + return compareRewriteHost(tool_config, expected.host_rewrite().value(), test_name); } -bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected) { +bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { std::string actual = ""; if (tool_config.route_->directResponseEntry() != nullptr) { actual = tool_config.route_->directResponseEntry()->newPath(*tool_config.headers_); } - const bool matches = compareResults(actual, expected, "path_redirect"); + const bool matches = compareResults(actual, expected, "path_redirect", test_name); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markRedirectPathCovered(*tool_config.route_->routeEntry()); } @@ -375,22 +371,22 @@ bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::st } bool RouterCheckTool::compareRedirectPath( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { if (!expected.has_path_redirect()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.path_redirect().value(), "path_redirect"); + return compareResults("", expected.path_redirect().value(), "path_redirect", test_name); } - return compareRedirectPath(tool_config, expected.path_redirect().value()); + return compareRedirectPath(tool_config, expected.path_redirect().value(), test_name); } bool RouterCheckTool::compareHeaderField( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { bool no_failures = true; if (expected.header_fields().data()) { for (const envoy::api::v2::core::HeaderValue& header : expected.header_fields()) { - if (!compareHeaderField(tool_config, header.key(), header.value())) { + if (!compareHeaderField(tool_config, header.key(), header.value(), test_name)) { no_failures = false; } } @@ -399,13 +395,13 @@ bool RouterCheckTool::compareHeaderField( } bool RouterCheckTool::compareHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected) { + const std::string& expected, const std::string& test_name) { std::string actual = tool_config.headers_->get_(field); - return compareResults(actual, expected, "check_header"); + return compareResults(actual, expected, "check_header", test_name); } bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected) { + const std::string& expected, const std::string& test_name) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -415,15 +411,15 @@ bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const st true); actual = tool_config.headers_->get_(field); } - return compareResults(actual, expected, "custom_header"); + return compareResults(actual, expected, "custom_header", test_name); } bool RouterCheckTool::compareCustomHeaderField( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { bool no_failures = true; if (expected.custom_header_fields().data()) { for (const envoy::api::v2::core::HeaderValue& header : expected.custom_header_fields()) { - if (!compareCustomHeaderField(tool_config, header.key(), header.value())) { + if (!compareCustomHeaderField(tool_config, header.key(), header.value(), test_name)) { no_failures = false; } } @@ -432,16 +428,12 @@ bool RouterCheckTool::compareCustomHeaderField( } bool RouterCheckTool::compareResults(const std::string& actual, const std::string& expected, - const std::string& test_type) { + const std::string& test_type, const std::string& test_name) { if (expected == actual) { return true; } - // Output failure details to stdout if details_ flag is set to true - if (details_ || only_show_failures_) { - std::cerr << "expected: [" << expected << "], actual: [" << actual - << "], test type: " << test_type << std::endl; - } + tests_[test_name].push_back("expected: [" + expected + "], actual: [" + actual + "], test type: " + test_type); return false; } @@ -458,7 +450,7 @@ Options::Options(int argc, char** argv) { TCLAP::CmdLine cmd("router_check_tool", ' ', "none", true); TCLAP::SwitchArg is_proto("p", "useproto", "Use Proto test file schema", cmd, false); TCLAP::SwitchArg is_detailed("d", "details", "Show detailed test execution results", cmd, false); - TCLAP::SwitchArg only_show_failures("o", "only-show-failures", "Only display failing tests", cmd, + TCLAP::SwitchArg only_show_failures("", "only-show-failures", "Only display failing tests", cmd, false); TCLAP::SwitchArg disable_deprecation_check("", "disable-deprecation-check", "Disable deprecated fields check", cmd, false); diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index 8cbdfa5ddd76..992dcd11e822 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -106,32 +106,40 @@ class RouterCheckTool : Logger::Loggable { std::unique_ptr config, std::unique_ptr stats, Api::ApiPtr api, Coverage coverage); - bool compareCluster(ToolConfig& tool_config, const std::string& expected); + bool compareCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); bool compareCluster(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); - bool compareVirtualCluster(ToolConfig& tool_config, const std::string& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); + bool compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); bool compareVirtualCluster(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); - bool compareVirtualHost(ToolConfig& tool_config, const std::string& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); + bool compareVirtualHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); bool compareVirtualHost(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); - bool compareRewriteHost(ToolConfig& tool_config, const std::string& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); + bool compareRewriteHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); bool compareRewriteHost(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); - bool compareRewritePath(ToolConfig& tool_config, const std::string& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); + bool compareRewritePath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); bool compareRewritePath(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); - bool compareRedirectPath(ToolConfig& tool_config, const std::string& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); + bool compareRedirectPath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); bool compareRedirectPath(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); bool compareHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected); + const std::string& expected, const std::string& test_name); bool compareHeaderField(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); bool compareCustomHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected); + const std::string& expected, const std::string& test_name); bool compareCustomHeaderField(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected); + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name); /** * Compare the expected and actual route parameter values. Print out match details if details_ * flag is set. @@ -140,7 +148,7 @@ class RouterCheckTool : Logger::Loggable { * @return bool if actual and expected match. */ bool compareResults(const std::string& actual, const std::string& expected, - const std::string& test_type); + const std::string& test_type, const std::string& test_name); bool runtimeMock(const std::string& key, const envoy::type::FractionalPercent& default_value, uint64_t random_value); @@ -151,6 +159,8 @@ class RouterCheckTool : Logger::Loggable { bool only_show_failures_{false}; + std::map> tests_; + // TODO(hennna): Switch away from mocks following work done by @rlazarus in github issue #499. std::unique_ptr> factory_context_; std::unique_ptr config_; diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index 3c78ecb0bdef..cdfb8cae1a8c 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -67,22 +67,21 @@ fi # Failure output flag test cases echo "testing failure test cases" # Failure test case with only details flag set -FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" 2>&1) || - echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then +FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--details" "--useproto" 2>&1) || +if [[ "${FAILURE_OUTPUT}" != *"Test_1"*"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi # Failure test case with details flag set and failures flag set -FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" "--only-show-failures" 2>&1) || +FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--details" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"*"in test: Test_2"* ]]; then +if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi # Failure test case with details flag unset and failures flag set -FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--only-show-failures" 2>&1) || +FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"*"in test: Test_2"* ]]; then +if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi From 15d58bdd30e9c791f085cefad1dfbf8e5c791970 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Mon, 9 Sep 2019 11:54:06 -0700 Subject: [PATCH 11/19] Lint Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 66 ++++++++++++++++++++----------- test/tools/router_check/router.h | 20 ++++++---- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 6ba347e1ad4b..3977a4246514 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -186,8 +186,8 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tests_.insert({test_name, {}}); const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); - using checkerFunc = - std::function; + using checkerFunc = std::function; checkerFunc checkers[] = { [this](auto&... params) -> bool { return this->compareCluster(params...); }, [this](auto&... params) -> bool { return this->compareVirtualCluster(params...); }, @@ -208,8 +208,10 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { } // Output failure details to stdout if details_ flag is set to true for (const auto& test_result : tests_) { - // All test names are printed if the details_ flag is true unless only_show_failures_ is also true. - if ((details_ && !only_show_failures_) || (only_show_failures_ && !test_result.second.empty())) { + // All test names are printed if the details_ flag is true unless only_show_failures_ is also + // true. + if ((details_ && !only_show_failures_) || + (only_show_failures_ && !test_result.second.empty())) { std::cout << test_result.first << std::endl; for (const auto& failure : test_result.second) { std::cerr << failure << std::endl; @@ -220,7 +222,8 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { return no_failures; } -bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { +bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr) { @@ -233,8 +236,9 @@ bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& return matches; } -bool RouterCheckTool::compareCluster( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { +bool RouterCheckTool::compareCluster(ToolConfig& tool_config, + const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { if (!expected.has_cluster_name()) { return true; } @@ -244,7 +248,8 @@ bool RouterCheckTool::compareCluster( return compareCluster(tool_config, expected.cluster_name().value(), test_name); } -bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { +bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr && @@ -261,17 +266,20 @@ bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std:: } bool RouterCheckTool::compareVirtualCluster( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { if (!expected.has_virtual_cluster_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.virtual_cluster_name().value(), "virtual_cluster_name", test_name); + return compareResults("", expected.virtual_cluster_name().value(), "virtual_cluster_name", + test_name); } return compareVirtualCluster(tool_config, expected.virtual_cluster_name().value(), test_name); } -bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { +bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr) { Stats::StatName stat_name = tool_config.route_->routeEntry()->virtualHost().statName(); @@ -285,7 +293,8 @@ bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareVirtualHost( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { if (!expected.has_virtual_host_name()) { return true; } @@ -295,7 +304,8 @@ bool RouterCheckTool::compareVirtualHost( return compareVirtualHost(tool_config, expected.virtual_host_name().value(), test_name); } -bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { +bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -316,7 +326,8 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareRewritePath( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { if (!expected.has_path_rewrite()) { return true; } @@ -326,7 +337,8 @@ bool RouterCheckTool::compareRewritePath( return compareRewritePath(tool_config, expected.path_rewrite().value(), test_name); } -bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { +bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -347,7 +359,8 @@ bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareRewriteHost( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { if (!expected.has_host_rewrite()) { return true; } @@ -357,7 +370,8 @@ bool RouterCheckTool::compareRewriteHost( return compareRewriteHost(tool_config, expected.host_rewrite().value(), test_name); } -bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name) { +bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name) { std::string actual = ""; if (tool_config.route_->directResponseEntry() != nullptr) { actual = tool_config.route_->directResponseEntry()->newPath(*tool_config.headers_); @@ -371,7 +385,8 @@ bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::st } bool RouterCheckTool::compareRedirectPath( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { if (!expected.has_path_redirect()) { return true; } @@ -382,7 +397,8 @@ bool RouterCheckTool::compareRedirectPath( } bool RouterCheckTool::compareHeaderField( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { bool no_failures = true; if (expected.header_fields().data()) { for (const envoy::api::v2::core::HeaderValue& header : expected.header_fields()) { @@ -395,13 +411,15 @@ bool RouterCheckTool::compareHeaderField( } bool RouterCheckTool::compareHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected, const std::string& test_name) { + const std::string& expected, + const std::string& test_name) { std::string actual = tool_config.headers_->get_(field); return compareResults(actual, expected, "check_header", test_name); } bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected, const std::string& test_name) { + const std::string& expected, + const std::string& test_name) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -415,7 +433,8 @@ bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const st } bool RouterCheckTool::compareCustomHeaderField( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, + const std::string& test_name) { bool no_failures = true; if (expected.custom_header_fields().data()) { for (const envoy::api::v2::core::HeaderValue& header : expected.custom_header_fields()) { @@ -433,7 +452,8 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin return true; } - tests_[test_name].push_back("expected: [" + expected + "], actual: [" + actual + "], test type: " + test_type); + tests_[test_name].push_back("expected: [" + expected + "], actual: [" + actual + + "], test type: " + test_type); return false; } diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index 992dcd11e822..5a0aab940c50 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -106,27 +106,33 @@ class RouterCheckTool : Logger::Loggable { std::unique_ptr config, std::unique_ptr stats, Api::ApiPtr api, Coverage coverage); - bool compareCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); + bool compareCluster(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name); bool compareCluster(ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name); - bool compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); + bool compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name); bool compareVirtualCluster(ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name); - bool compareVirtualHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); + bool compareVirtualHost(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name); bool compareVirtualHost(ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name); - bool compareRewriteHost(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); + bool compareRewriteHost(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name); bool compareRewriteHost(ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name); - bool compareRewritePath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); + bool compareRewritePath(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name); bool compareRewritePath(ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name); - bool compareRedirectPath(ToolConfig& tool_config, const std::string& expected, const std::string& test_name); + bool compareRedirectPath(ToolConfig& tool_config, const std::string& expected, + const std::string& test_name); bool compareRedirectPath(ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, const std::string& test_name); @@ -148,7 +154,7 @@ class RouterCheckTool : Logger::Loggable { * @return bool if actual and expected match. */ bool compareResults(const std::string& actual, const std::string& expected, - const std::string& test_type, const std::string& test_name); + const std::string& test_type, const std::string& test_name); bool runtimeMock(const std::string& key, const envoy::type::FractionalPercent& default_value, uint64_t random_value); From 9e58cfcbd6ea06e56215edda67e5f3dd39665a91 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Mon, 9 Sep 2019 13:53:55 -0700 Subject: [PATCH 12/19] Use vector instead of map for non-unique test names Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 113 +++++++++++++----------------- test/tools/router_check/router.h | 50 +++++-------- 2 files changed, 66 insertions(+), 97 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 3977a4246514..987b8b2d3053 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -113,7 +113,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso } Json::ObjectSharedPtr validate = check_config->getObject("validate"); - using checkerFunc = std::function; + using checkerFunc = std::function; const std::unordered_map checkers = { {"cluster_name", [this](auto&... params) -> bool { return this->compareCluster(params...); }}, @@ -134,9 +134,9 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso if (validate->hasObject(test.first)) { const std::string& expected = validate->getString(test.first); if (tool_config.route_ == nullptr) { - compareResults("", expected, test.first, test_name); + compareResults("", expected, test.first); } else { - if (!test.second(tool_config, expected, test_name)) { + if (!test.second(tool_config, expected)) { no_failures = false; } } @@ -146,7 +146,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso if (validate->hasObject("header_fields")) { for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("header_fields")) { if (!compareHeaderField(tool_config, header_field->getString("field"), - header_field->getString("value"), test_name)) { + header_field->getString("value"))) { no_failures = false; } } @@ -156,7 +156,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("custom_header_fields")) { if (!compareCustomHeaderField(tool_config, header_field->getString("field"), - header_field->getString("value"), test_name)) { + header_field->getString("value"))) { no_failures = false; } } @@ -183,11 +183,11 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); const std::string& test_name = check_config.test_name(); - tests_.insert({test_name, {}}); + tests_.push_back(std::pair>(test_name, {})); const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); using checkerFunc = std::function; + ToolConfig&, const envoy::RouterCheckToolSchema::ValidationAssert&)>; checkerFunc checkers[] = { [this](auto&... params) -> bool { return this->compareCluster(params...); }, [this](auto&... params) -> bool { return this->compareVirtualCluster(params...); }, @@ -201,7 +201,7 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { // Call appropriate function for each match case. for (const auto& test : checkers) { - if (!test(tool_config, validate, test_name)) { + if (!test(tool_config, validate)) { no_failures = false; } } @@ -222,14 +222,13 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { return no_failures; } -bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name) { +bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr) { actual = tool_config.route_->routeEntry()->clusterName(); } - const bool matches = compareResults(actual, expected, "cluster_name", test_name); + const bool matches = compareResults(actual, expected, "cluster_name"); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markClusterCovered(*tool_config.route_->routeEntry()); } @@ -237,19 +236,17 @@ bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& } bool RouterCheckTool::compareCluster(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_cluster_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.cluster_name().value(), "cluster_name", test_name); + return compareResults("", expected.cluster_name().value(), "cluster_name"); } - return compareCluster(tool_config, expected.cluster_name().value(), test_name); + return compareCluster(tool_config, expected.cluster_name().value()); } -bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name) { +bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr && @@ -258,7 +255,7 @@ bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std:: tool_config.route_->routeEntry()->virtualCluster(*tool_config.headers_)->statName(); actual = tool_config.symbolTable().toString(stat_name); } - const bool matches = compareResults(actual, expected, "virtual_cluster_name", test_name); + const bool matches = compareResults(actual, expected, "virtual_cluster_name"); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markVirtualClusterCovered(*tool_config.route_->routeEntry()); } @@ -266,26 +263,23 @@ bool RouterCheckTool::compareVirtualCluster(ToolConfig& tool_config, const std:: } bool RouterCheckTool::compareVirtualCluster( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_virtual_cluster_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.virtual_cluster_name().value(), "virtual_cluster_name", - test_name); + return compareResults("", expected.virtual_cluster_name().value(), "virtual_cluster_name"); } - return compareVirtualCluster(tool_config, expected.virtual_cluster_name().value(), test_name); + return compareVirtualCluster(tool_config, expected.virtual_cluster_name().value()); } -bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name) { +bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; if (tool_config.route_->routeEntry() != nullptr) { Stats::StatName stat_name = tool_config.route_->routeEntry()->virtualHost().statName(); actual = tool_config.symbolTable().toString(stat_name); } - const bool matches = compareResults(actual, expected, "virtual_host_name", test_name); + const bool matches = compareResults(actual, expected, "virtual_host_name"); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markVirtualHostCovered(*tool_config.route_->routeEntry()); } @@ -293,19 +287,17 @@ bool RouterCheckTool::compareVirtualHost(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareVirtualHost( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_virtual_host_name()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.virtual_host_name().value(), "virtual_host_name", test_name); + return compareResults("", expected.virtual_host_name().value(), "virtual_host_name"); } - return compareVirtualHost(tool_config, expected.virtual_host_name().value(), test_name); + return compareVirtualHost(tool_config, expected.virtual_host_name().value()); } -bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name) { +bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -318,7 +310,7 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str actual = tool_config.headers_->get_(Http::Headers::get().Path); } - const bool matches = compareResults(actual, expected, "path_rewrite", test_name); + const bool matches = compareResults(actual, expected, "path_rewrite"); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markPathRewriteCovered(*tool_config.route_->routeEntry()); } @@ -326,19 +318,17 @@ bool RouterCheckTool::compareRewritePath(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareRewritePath( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_path_rewrite()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.path_rewrite().value(), "path_rewrite", test_name); + return compareResults("", expected.path_rewrite().value(), "path_rewrite"); } - return compareRewritePath(tool_config, expected.path_rewrite().value(), test_name); + return compareRewritePath(tool_config, expected.path_rewrite().value()); } -bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name) { +bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -351,7 +341,7 @@ bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::str actual = tool_config.headers_->get_(Http::Headers::get().Host); } - const bool matches = compareResults(actual, expected, "host_rewrite", test_name); + const bool matches = compareResults(actual, expected, "host_rewrite"); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markHostRewriteCovered(*tool_config.route_->routeEntry()); } @@ -359,25 +349,23 @@ bool RouterCheckTool::compareRewriteHost(ToolConfig& tool_config, const std::str } bool RouterCheckTool::compareRewriteHost( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_host_rewrite()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.host_rewrite().value(), "host_rewrite", test_name); + return compareResults("", expected.host_rewrite().value(), "host_rewrite"); } - return compareRewriteHost(tool_config, expected.host_rewrite().value(), test_name); + return compareRewriteHost(tool_config, expected.host_rewrite().value()); } -bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name) { +bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::string& expected) { std::string actual = ""; if (tool_config.route_->directResponseEntry() != nullptr) { actual = tool_config.route_->directResponseEntry()->newPath(*tool_config.headers_); } - const bool matches = compareResults(actual, expected, "path_redirect", test_name); + const bool matches = compareResults(actual, expected, "path_redirect"); if (matches && tool_config.route_->routeEntry() != nullptr) { coverage_.markRedirectPathCovered(*tool_config.route_->routeEntry()); } @@ -385,24 +373,22 @@ bool RouterCheckTool::compareRedirectPath(ToolConfig& tool_config, const std::st } bool RouterCheckTool::compareRedirectPath( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_path_redirect()) { return true; } if (tool_config.route_ == nullptr) { - return compareResults("", expected.path_redirect().value(), "path_redirect", test_name); + return compareResults("", expected.path_redirect().value(), "path_redirect"); } - return compareRedirectPath(tool_config, expected.path_redirect().value(), test_name); + return compareRedirectPath(tool_config, expected.path_redirect().value()); } bool RouterCheckTool::compareHeaderField( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { bool no_failures = true; if (expected.header_fields().data()) { for (const envoy::api::v2::core::HeaderValue& header : expected.header_fields()) { - if (!compareHeaderField(tool_config, header.key(), header.value(), test_name)) { + if (!compareHeaderField(tool_config, header.key(), header.value())) { no_failures = false; } } @@ -411,15 +397,13 @@ bool RouterCheckTool::compareHeaderField( } bool RouterCheckTool::compareHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected, - const std::string& test_name) { + const std::string& expected) { std::string actual = tool_config.headers_->get_(field); - return compareResults(actual, expected, "check_header", test_name); + return compareResults(actual, expected, "check_header"); } bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected, - const std::string& test_name) { + const std::string& expected) { std::string actual = ""; Envoy::StreamInfo::StreamInfoImpl stream_info(Envoy::Http::Protocol::Http11, factory_context_->dispatcher().timeSource()); @@ -429,16 +413,15 @@ bool RouterCheckTool::compareCustomHeaderField(ToolConfig& tool_config, const st true); actual = tool_config.headers_->get_(field); } - return compareResults(actual, expected, "custom_header", test_name); + return compareResults(actual, expected, "custom_header"); } bool RouterCheckTool::compareCustomHeaderField( - ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name) { + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { bool no_failures = true; if (expected.custom_header_fields().data()) { for (const envoy::api::v2::core::HeaderValue& header : expected.custom_header_fields()) { - if (!compareCustomHeaderField(tool_config, header.key(), header.value(), test_name)) { + if (!compareCustomHeaderField(tool_config, header.key(), header.value())) { no_failures = false; } } @@ -447,12 +430,12 @@ bool RouterCheckTool::compareCustomHeaderField( } bool RouterCheckTool::compareResults(const std::string& actual, const std::string& expected, - const std::string& test_type, const std::string& test_name) { + const std::string& test_type) { if (expected == actual) { return true; } - tests_[test_name].push_back("expected: [" + expected + "], actual: [" + actual + + tests_.back().second.push_back("expected: [" + expected + "], actual: [" + actual + "], test type: " + test_type); return false; } diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index 5a0aab940c50..41e0b57a0665 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -106,46 +106,32 @@ class RouterCheckTool : Logger::Loggable { std::unique_ptr config, std::unique_ptr stats, Api::ApiPtr api, Coverage coverage); - bool compareCluster(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name); + bool compareCluster(ToolConfig& tool_config, const std::string& expected); bool compareCluster(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); - bool compareVirtualCluster(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); + bool compareVirtualCluster(ToolConfig& tool_config, const std::string& expected); bool compareVirtualCluster(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); - bool compareVirtualHost(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); + bool compareVirtualHost(ToolConfig& tool_config, const std::string& expected); bool compareVirtualHost(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); - bool compareRewriteHost(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); + bool compareRewriteHost(ToolConfig& tool_config, const std::string& expected); bool compareRewriteHost(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); - bool compareRewritePath(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); + bool compareRewritePath(ToolConfig& tool_config, const std::string& expected); bool compareRewritePath(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); - bool compareRedirectPath(ToolConfig& tool_config, const std::string& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); + bool compareRedirectPath(ToolConfig& tool_config, const std::string& expected); bool compareRedirectPath(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); bool compareHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected, const std::string& test_name); + const std::string& expected); bool compareHeaderField(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); bool compareCustomHeaderField(ToolConfig& tool_config, const std::string& field, - const std::string& expected, const std::string& test_name); + const std::string& expected); bool compareCustomHeaderField(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected, - const std::string& test_name); + const envoy::RouterCheckToolSchema::ValidationAssert& expected); /** * Compare the expected and actual route parameter values. Print out match details if details_ * flag is set. @@ -154,7 +140,7 @@ class RouterCheckTool : Logger::Loggable { * @return bool if actual and expected match. */ bool compareResults(const std::string& actual, const std::string& expected, - const std::string& test_type, const std::string& test_name); + const std::string& test_type); bool runtimeMock(const std::string& key, const envoy::type::FractionalPercent& default_value, uint64_t random_value); @@ -165,7 +151,7 @@ class RouterCheckTool : Logger::Loggable { bool only_show_failures_{false}; - std::map> tests_; + std::vector>> tests_; // TODO(hennna): Switch away from mocks following work done by @rlazarus in github issue #499. std::unique_ptr> factory_context_; From b74ea99adc229ddbfd225d6391604f2fb13f2471 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Mon, 9 Sep 2019 13:57:12 -0700 Subject: [PATCH 13/19] Format Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 987b8b2d3053..4badce6213d2 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -186,8 +186,8 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tests_.push_back(std::pair>(test_name, {})); const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); - using checkerFunc = std::function; + using checkerFunc = + std::function; checkerFunc checkers[] = { [this](auto&... params) -> bool { return this->compareCluster(params...); }, [this](auto&... params) -> bool { return this->compareVirtualCluster(params...); }, @@ -235,8 +235,8 @@ bool RouterCheckTool::compareCluster(ToolConfig& tool_config, const std::string& return matches; } -bool RouterCheckTool::compareCluster(ToolConfig& tool_config, - const envoy::RouterCheckToolSchema::ValidationAssert& expected) { +bool RouterCheckTool::compareCluster( + ToolConfig& tool_config, const envoy::RouterCheckToolSchema::ValidationAssert& expected) { if (!expected.has_cluster_name()) { return true; } @@ -436,7 +436,7 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin } tests_.back().second.push_back("expected: [" + expected + "], actual: [" + actual + - "], test type: " + test_type); + "], test type: " + test_type); return false; } From e5d1b36b044ba6b56b408be7d62ac525320e9689 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 10 Sep 2019 10:26:01 -0700 Subject: [PATCH 14/19] Restore proto details test Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 28 +++++++++++++-------- test/tools/router_check/test/route_tests.sh | 7 +++--- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 4badce6213d2..12a2e76d4ee8 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -100,19 +100,14 @@ RouterCheckTool::RouterCheckTool( bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_json) { Json::ObjectSharedPtr loader = Json::Factory::loadFromFile(expected_route_json, *api_); loader->validateSchema(Json::ToolSchema::routerCheckSchema()); - bool no_failures = true; for (const Json::ObjectSharedPtr& check_config : loader->asObjectArray()) { headers_finalized_ = false; ToolConfig tool_config = ToolConfig::create(check_config); tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); - std::string test_name = check_config->getString("test_name", ""); - if (details_) { - std::cout << test_name << std::endl; - } + tests_.push_back(std::pair>(test_name, {})); Json::ObjectSharedPtr validate = check_config->getObject("validate"); - using checkerFunc = std::function; const std::unordered_map checkers = { {"cluster_name", @@ -128,7 +123,6 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso {"path_redirect", [this](auto&... params) -> bool { return this->compareRedirectPath(params...); }}, }; - // Call appropriate function for each match case. for (const auto& test : checkers) { if (validate->hasObject(test.first)) { @@ -142,7 +136,6 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso } } } - if (validate->hasObject("header_fields")) { for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("header_fields")) { if (!compareHeaderField(tool_config, header_field->getString("field"), @@ -151,7 +144,6 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso } } } - if (validate->hasObject("custom_header_fields")) { for (const Json::ObjectSharedPtr& header_field : validate->getObjectArray("custom_header_fields")) { @@ -162,7 +154,18 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso } } } - + // Output failure details to stdout if details_ flag is set to true + for (const auto& test_result : tests_) { + // All test names are printed if the details_ flag is true unless only_show_failures_ is also + // true. + if ((details_ && !only_show_failures_) || + (only_show_failures_ && !test_result.second.empty())) { + std::cout << test_result.first << std::endl; + for (const auto& failure : test_result.second) { + std::cerr << failure << std::endl; + } + } + } return no_failures; } @@ -434,7 +437,10 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin if (expected == actual) { return true; } - + // if (details_) { + // std::cerr << "expected: [" << expected << "], actual: [" << actual + // << "], test type: " << test_type << std::endl; + // } tests_.back().second.push_back("expected: [" + expected + "], actual: [" + actual + "], test type: " + test_type); return false; diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index cdfb8cae1a8c..44a121e549c7 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -67,7 +67,8 @@ fi # Failure output flag test cases echo "testing failure test cases" # Failure test case with only details flag set -FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--details" "--useproto" 2>&1) || +FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" 2>&1) || + echo "${FAILURE_OUTPUT:-no-output}" if [[ "${FAILURE_OUTPUT}" != *"Test_1"*"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi @@ -75,13 +76,13 @@ fi # Failure test case with details flag set and failures flag set FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--details" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then +if [[ "${FAILURE_OUTPUT}" != "Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi # Failure test case with details flag unset and failures flag set FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then +if [[ "${FAILURE_OUTPUT}" != "Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then exit 1 fi From b6d8f3d33285aae44dcd99b694c4c6b49ca11ed4 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 10 Sep 2019 10:26:57 -0700 Subject: [PATCH 15/19] Remove comments Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 12a2e76d4ee8..038937a30ab5 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -437,10 +437,6 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin if (expected == actual) { return true; } - // if (details_) { - // std::cerr << "expected: [" << expected << "], actual: [" << actual - // << "], test type: " << test_type << std::endl; - // } tests_.back().second.push_back("expected: [" + expected + "], actual: [" + actual + "], test type: " + test_type); return false; From ed0b7f8569aaf4757c74d14c61470b493ede82e4 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 10 Sep 2019 10:52:40 -0700 Subject: [PATCH 16/19] Change expected test output to account for CI tests Signed-off-by: Lisa Lu --- test/tools/router_check/test/route_tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index 44a121e549c7..b0bdb4a0488d 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -76,13 +76,13 @@ fi # Failure test case with details flag set and failures flag set FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--details" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != "Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then +if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [["${FAILURE_OUTPUT}" == *"Test_1"*]]; then exit 1 fi # Failure test case with details flag unset and failures flag set FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != "Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then +if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [["${FAILURE_OUTPUT}" == *"Test_1"*]]; then exit 1 fi From a474b4c14cd8b06ab57b58ea66722f89bd175b2a Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 10 Sep 2019 14:55:08 -0700 Subject: [PATCH 17/19] Refactor output printing, add comment Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 48 +++++++++++++------------------ test/tools/router_check/router.h | 4 +++ 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 038937a30ab5..47b0e85c9046 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -106,7 +106,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso ToolConfig tool_config = ToolConfig::create(check_config); tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); std::string test_name = check_config->getString("test_name", ""); - tests_.push_back(std::pair>(test_name, {})); + tests_.emplace_back(std::pair>(test_name, {})); Json::ObjectSharedPtr validate = check_config->getObject("validate"); using checkerFunc = std::function; const std::unordered_map checkers = { @@ -154,18 +154,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso } } } - // Output failure details to stdout if details_ flag is set to true - for (const auto& test_result : tests_) { - // All test names are printed if the details_ flag is true unless only_show_failures_ is also - // true. - if ((details_ && !only_show_failures_) || - (only_show_failures_ && !test_result.second.empty())) { - std::cout << test_result.first << std::endl; - for (const auto& failure : test_result.second) { - std::cerr << failure << std::endl; - } - } - } + printResults(); return no_failures; } @@ -186,7 +175,7 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); const std::string& test_name = check_config.test_name(); - tests_.push_back(std::pair>(test_name, {})); + tests_.emplace_back(std::pair>(test_name, {})); const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); using checkerFunc = @@ -209,19 +198,7 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { } } } - // Output failure details to stdout if details_ flag is set to true - for (const auto& test_result : tests_) { - // All test names are printed if the details_ flag is true unless only_show_failures_ is also - // true. - if ((details_ && !only_show_failures_) || - (only_show_failures_ && !test_result.second.empty())) { - std::cout << test_result.first << std::endl; - for (const auto& failure : test_result.second) { - std::cerr << failure << std::endl; - } - } - } - + printResults(); return no_failures; } @@ -437,11 +414,26 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin if (expected == actual) { return true; } - tests_.back().second.push_back("expected: [" + expected + "], actual: [" + actual + + tests_.back().second.emplace_back("expected: [" + expected + "], actual: [" + actual + "], test type: " + test_type); return false; } +void RouterCheckTool::printResults() { + // Output failure details to stdout if details_ flag is set to true + for (const auto& test_result : tests_) { + // All test names are printed if the details_ flag is true unless only_show_failures_ is also + // true. + if ((details_ && !only_show_failures_) || + (only_show_failures_ && !test_result.second.empty())) { + std::cout << test_result.first << std::endl; + for (const auto& failure : test_result.second) { + std::cerr << failure << std::endl; + } + } + } +} + // The Mock for runtime value checks. // This is a simple implementation to mimic the actual runtime checks in Snapshot.featureEnabled bool RouterCheckTool::runtimeMock(const std::string& key, diff --git a/test/tools/router_check/router.h b/test/tools/router_check/router.h index 41e0b57a0665..78352e86e5d5 100644 --- a/test/tools/router_check/router.h +++ b/test/tools/router_check/router.h @@ -142,6 +142,8 @@ class RouterCheckTool : Logger::Loggable { bool compareResults(const std::string& actual, const std::string& expected, const std::string& test_type); + void printResults(); + bool runtimeMock(const std::string& key, const envoy::type::FractionalPercent& default_value, uint64_t random_value); @@ -151,6 +153,8 @@ class RouterCheckTool : Logger::Loggable { bool only_show_failures_{false}; + // The first member of each pair is the name of the test. + // The second member is a list of any failing results for that test as strings. std::vector>> tests_; // TODO(hennna): Switch away from mocks following work done by @rlazarus in github issue #499. From 8c9012f72f07212167e28d7a7dcd1134ef7ff0a7 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 10 Sep 2019 14:57:38 -0700 Subject: [PATCH 18/19] Formatting Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 47b0e85c9046..6dd84e8f1dfe 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -415,7 +415,7 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin return true; } tests_.back().second.emplace_back("expected: [" + expected + "], actual: [" + actual + - "], test type: " + test_type); + "], test type: " + test_type); return false; } From cc362e3b79064a66b04cd01a820bb28aed3b4a24 Mon Sep 17 00:00:00 2001 From: Lisa Lu Date: Tue, 10 Sep 2019 17:31:01 -0700 Subject: [PATCH 19/19] Fix error in test script and add test for multiple errors in test Signed-off-by: Lisa Lu --- test/tools/router_check/router.cc | 4 ++-- test/tools/router_check/test/config/Weighted.golden.json | 7 ++++--- .../router_check/test/config/Weighted.golden.proto.json | 4 ++-- .../test/config/Weighted.golden.proto.pb_text | 5 +++-- .../router_check/test/config/Weighted.golden.proto.yaml | 3 ++- test/tools/router_check/test/config/Weighted.yaml | 9 +++++++++ test/tools/router_check/test/route_tests.sh | 6 +++--- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 6dd84e8f1dfe..0547c15e03be 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -106,7 +106,7 @@ bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_jso ToolConfig tool_config = ToolConfig::create(check_config); tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); std::string test_name = check_config->getString("test_name", ""); - tests_.emplace_back(std::pair>(test_name, {})); + tests_.emplace_back(test_name, std::vector{}); Json::ObjectSharedPtr validate = check_config->getObject("validate"); using checkerFunc = std::function; const std::unordered_map checkers = { @@ -175,7 +175,7 @@ bool RouterCheckTool::compareEntries(const std::string& expected_routes) { tool_config.route_ = config_->route(*tool_config.headers_, tool_config.random_value_); const std::string& test_name = check_config.test_name(); - tests_.emplace_back(std::pair>(test_name, {})); + tests_.emplace_back(test_name, std::vector{}); const envoy::RouterCheckToolSchema::ValidationAssert& validate = check_config.validate(); using checkerFunc = diff --git a/test/tools/router_check/test/config/Weighted.golden.json b/test/tools/router_check/test/config/Weighted.golden.json index e12cb45bad4d..32e7adc56670 100644 --- a/test/tools/router_check/test/config/Weighted.golden.json +++ b/test/tools/router_check/test/config/Weighted.golden.json @@ -13,10 +13,11 @@ "test_name": "Test_2", "input": { ":authority": "www1.lyft.com", - ":path": "/foo", - "random_value": 115 + ":path": "/test/123", + "random_value": 115, + ":method": "GET" }, - "validate": {"cluster_name": "cluster1"} + "validate": {"cluster_name": "cluster1", "virtual_cluster_name": "test_virtual_cluster"} }, { "test_name": "Test_3", diff --git a/test/tools/router_check/test/config/Weighted.golden.proto.json b/test/tools/router_check/test/config/Weighted.golden.proto.json index 63622d1b4f8d..e48a0446928e 100644 --- a/test/tools/router_check/test/config/Weighted.golden.proto.json +++ b/test/tools/router_check/test/config/Weighted.golden.proto.json @@ -15,11 +15,11 @@ "test_name": "Test_2", "input": { "authority": "www1.lyft.com", - "path": "/foo", + "path": "/test/123", "method": "GET", "random_value": 115 }, - "validate": {"cluster_name": "cluster1"} + "validate": {"cluster_name": "cluster1", "virtual_cluster_name": "test_virtual_cluster"} }, { "test_name": "Test_3", diff --git a/test/tools/router_check/test/config/Weighted.golden.proto.pb_text b/test/tools/router_check/test/config/Weighted.golden.proto.pb_text index fa7dedcb27c5..9a9d0d8f7a8f 100644 --- a/test/tools/router_check/test/config/Weighted.golden.proto.pb_text +++ b/test/tools/router_check/test/config/Weighted.golden.proto.pb_text @@ -16,11 +16,12 @@ tests { test_name: "Test_2" input: { authority: "www1.lyft.com" - path: "/foo" + path: "/test/123" method: "GET" random_value: 115 } validate: { - cluster_name: { value: "cluster1" } + cluster_name: { value: "cluster1"} + virtual_cluster_name: { value: "test_virtual_cluster" } } } \ No newline at end of file diff --git a/test/tools/router_check/test/config/Weighted.golden.proto.yaml b/test/tools/router_check/test/config/Weighted.golden.proto.yaml index 2508111d5272..db59c022dc12 100644 --- a/test/tools/router_check/test/config/Weighted.golden.proto.yaml +++ b/test/tools/router_check/test/config/Weighted.golden.proto.yaml @@ -11,11 +11,12 @@ tests: - test_name: Test_2 input: authority: www1.lyft.com - path: "/foo" + path: "/test/123" method: GET random_value: 115 validate: cluster_name: cluster1 + virtual_cluster_name: test_virtual_cluster - test_name: Test_3 input: authority: www1.lyft.com diff --git a/test/tools/router_check/test/config/Weighted.yaml b/test/tools/router_check/test/config/Weighted.yaml index dd31345021a2..074a24b75b62 100644 --- a/test/tools/router_check/test/config/Weighted.yaml +++ b/test/tools/router_check/test/config/Weighted.yaml @@ -14,6 +14,15 @@ virtual_hosts: weight: 30 - name: cluster3 weight: 40 + virtual_clusters: + - headers: + - name: :path + safe_regex_match: + google_re2: {} + regex: ^/test/\d+$ + - name: :method + exact_match: GET + name: test_virtual_cluster - name: www2 domains: - www2.lyft.com diff --git a/test/tools/router_check/test/route_tests.sh b/test/tools/router_check/test/route_tests.sh index b0bdb4a0488d..74a4eeccc0df 100755 --- a/test/tools/router_check/test/route_tests.sh +++ b/test/tools/router_check/test/route_tests.sh @@ -69,20 +69,20 @@ echo "testing failure test cases" # Failure test case with only details flag set FAILURE_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/TestRoutes.yaml" "${PATH_CONFIG}/Weighted.golden.json" "--details" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"Test_1"*"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]]; then +if [[ "${FAILURE_OUTPUT}" != *"Test_1"*"Test_2"*"expected: [test_virtual_cluster], actual: [other], test type: virtual_cluster_name"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"*"Test_3"* ]]; then exit 1 fi # Failure test case with details flag set and failures flag set FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--details" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [["${FAILURE_OUTPUT}" == *"Test_1"*]]; then +if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [[ "${FAILURE_OUTPUT}" == *"Test_1"* ]]; then exit 1 fi # Failure test case with details flag unset and failures flag set FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.json" "--only-show-failures" "--useproto" 2>&1) || echo "${FAILURE_OUTPUT:-no-output}" -if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [["${FAILURE_OUTPUT}" == *"Test_1"*]]; then +if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [[ "${FAILURE_OUTPUT}" == *"Test_1"* ]]; then exit 1 fi