route check tool: add support for outputting missing tests (#8240)

Signed-off-by: Lisa Lu <lisalu@lyft.com>

docs/root/install/tools/route_table_check_tool.rst

@@ -48,7 +48,7 @@ Usage
 
     --covall
       Enables comprehensive code coverage percent calculation taking into account all the possible
-      asserts.
+      asserts. Displays missing tests.
 
     --disable-deprecation-check
       Disables the deprecation check for RouteConfiguration proto.
@@ -65,7 +65,7 @@ Output
   The program exits with status EXIT_FAILURE if any test case does not match the expected route parameter
   value.
 
-  If a test fails, details of the failed test cases are printed if ``-details`` flag is provided. 
+  If a test fails, details of the failed test cases are printed if ``--details`` flag is provided. 
   The first field is the expected route parameter value. The second field is the actual route parameter value. 
   The third field indicates the parameter that is compared.
   In the following example, Test_2 and Test_5 failed while the other tests

docs/root/intro/version_history.rst

@@ -54,6 +54,7 @@ Version history
 * router check tool: add comprehensive coverage reporting.
 * router check tool: add deprecated field check.
 * router check tool: add flag for only printing results of failed tests.
+* router check tool: add support for outputting missing tests in the detailed coverage report.
 * server: added a post initialization lifecycle event, in addition to the existing startup and shutdown events.
 * 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

test/tools/router_check/coverage.cc

@@ -53,7 +53,25 @@ double Coverage::report() {
   return 100 * static_cast<double>(covered_routes_.size()) / num_routes;
 }
 
+void Coverage::printMissingTests(const std::set<std::string>& all_route_names,
+                                 const std::set<std::string>& covered_route_names) {
+  std::set<std::string> missing_route_names;
+  std::set_difference(all_route_names.begin(), all_route_names.end(), covered_route_names.begin(),
+                      covered_route_names.end(),
+                      std::inserter(missing_route_names, missing_route_names.end()));
+  for (const auto& host : route_config_.virtual_hosts()) {
+    for (const auto& route : host.routes()) {
+      if (missing_route_names.find(route.name()) != missing_route_names.end()) {
+        std::cout << "Missing test for host: " << host.name()
+                  << ", route: " << route.match().DebugString() << std::endl;
+      }
+    }
+  }
+}
+
 double Coverage::detailedReport() {
+  std::set<std::string> all_route_names;
+  std::set<std::string> covered_route_names;
   uint64_t num_routes = 0;
   for (const auto& host : route_config_.virtual_hosts()) {
     for (const auto& route : host.routes()) {
@@ -62,12 +80,15 @@ double Coverage::detailedReport() {
       } else {
         num_routes += 1;
       }
+      all_route_names.emplace(route.name());
     }
   }
   double cumulative_coverage = 0;
   for (auto& covered_route : covered_routes_) {
     cumulative_coverage += covered_route->report();
+    covered_route_names.emplace(covered_route->route().routeName());
   }
+  printMissingTests(all_route_names, covered_route_names);
   return 100 * cumulative_coverage / num_routes;
 }
 

test/tools/router_check/coverage.h

@@ -17,6 +17,7 @@ class RouteCoverage : Logger::Loggable<Logger::Id::testing> {
   void setHostRewriteCovered() { host_rewrite_covered_ = true; }
   void setRedirectPathCovered() { redirect_path_covered_ = true; }
   bool covers(const Envoy::Router::RouteEntry* route) { return &route_ == route; }
+  const Envoy::Router::RouteEntry& route() { return route_; }
 
 private:
   const Envoy::Router::RouteEntry& route_;
@@ -45,6 +46,8 @@ class Coverage : Logger::Loggable<Logger::Id::testing> {
   void markRedirectPathCovered(const Envoy::Router::RouteEntry& route);
   double report();
   double detailedReport();
+  void printMissingTests(const std::set<std::string>& all_route_names,
+                         const std::set<std::string>& covered_route_names);
 
 private:
   RouteCoverage& coveredRoute(const Envoy::Router::RouteEntry& route);

test/tools/router_check/router.cc

@@ -8,6 +8,7 @@
 #include "common/network/utility.h"
 #include "common/protobuf/message_validator_impl.h"
 #include "common/protobuf/utility.h"
+#include "common/runtime/runtime_impl.h"
 #include "common/stream_info/stream_info_impl.h"
 
 #include "test/test_common/printers.h"
@@ -71,6 +72,7 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file,
   auto stats = std::make_unique<Stats::IsolatedStoreImpl>();
   auto api = Api::createApiForTest(*stats);
   TestUtility::loadFromFile(router_config_file, route_config, *api);
+  assignUniqueRouteNames(route_config);
 
   auto factory_context = std::make_unique<NiceMock<Server::Configuration::MockFactoryContext>>();
   auto config = std::make_unique<Router::ConfigImpl>(route_config, *factory_context, false);
@@ -84,6 +86,15 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_file,
                          std::move(api), Coverage(route_config));
 }
 
+void RouterCheckTool::assignUniqueRouteNames(envoy::api::v2::RouteConfiguration& route_config) {
+  Runtime::RandomGeneratorImpl random;
+  for (auto& host : *route_config.mutable_virtual_hosts()) {
+    for (auto& route : *host.mutable_routes()) {
+      route.set_name(random.uuid());
+    }
+  }
+}
+
 RouterCheckTool::RouterCheckTool(
     std::unique_ptr<NiceMock<Server::Configuration::MockFactoryContext>> factory_context,
     std::unique_ptr<Router::ConfigImpl> config, std::unique_ptr<Stats::IsolatedStoreImpl> stats,

test/tools/router_check/router.h

@@ -106,6 +106,11 @@ class RouterCheckTool : Logger::Loggable<Logger::Id::testing> {
       std::unique_ptr<Router::ConfigImpl> config, std::unique_ptr<Stats::IsolatedStoreImpl> stats,
       Api::ApiPtr api, Coverage coverage);
 
+  /**
+   * Set UUID as the name for each route for detecting missing tests during the coverage check.
+   */
+  static void assignUniqueRouteNames(envoy::api::v2::RouteConfiguration& route_config);
+
   bool compareCluster(ToolConfig& tool_config, const std::string& expected);
   bool compareCluster(ToolConfig& tool_config,
                       const envoy::RouterCheckToolSchema::ValidationAssert& expected);

test/tools/router_check/test/route_tests.sh

@@ -86,3 +86,11 @@ FAILURE_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PAT
 if [[ "${FAILURE_OUTPUT}" != *"Test_2"*"expected: [cluster1], actual: [instant-server], test type: cluster_name"* ]] || [[ "${FAILURE_OUTPUT}" == *"Test_1"* ]]; then
   exit 1
 fi
+
+# Missing test results
+echo "testing missing tests output test cases"
+MISSING_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/TestRoutes.yaml" "-t" "${PATH_CONFIG}/TestRoutes.golden.proto.json" "--details" "--useproto" "--covall" 2>&1) ||
+  echo "${MISSING_OUTPUT:-no-output}"
+if [[ "${MISSING_OUTPUT}" != *"Missing test for host: www2_staging, route: prefix: \"/\""*"Missing test for host: default, route: prefix: \"/api/application_data\""* ]]; then
+  exit 1
+fi