Use TCLAP for router_check_tool

docs/root/install/tools/route_table_check_tool.rst

@@ -7,37 +7,55 @@ The route table check tool checks whether the route parameters returned by a rou
 The tool can also be used to check whether a path redirect, path rewrite, or host rewrite
 match what is expected.
 
-Input
-  The tool expects two input files:
-
-  1. A v2 router config file (YAML or JSON). The router config file schema is found in
-     :ref:`config <envoy_api_file_envoy/api/v2/route/route.proto>` and the config file extension
-     must reflect its file type (for instance, .json for JSON and .yaml for YAML).
-
-  2. A tool config JSON file. The tool config JSON file schema is found in
-     :ref:`config <config_tools_router_check_tool>`.
-     The tool config input file specifies urls (composed of authorities and paths)
-     and expected route parameter values. Additional parameters such as additional headers are optional.
-     Schema: All internal schemas in the tool are based on :repo:`proto3 <test/tools/router_check/validation.proto>`.
-     This is enabled by an extra optional parameter ``--useproto``. This parameter will become the default in the future releases and enables more validation features in the tool.
-     Any new feature addition in validations will be added behind this parameter.
-     Migration: If you are currently using the tool and plan to migrate to use ``--useproto``, change the yaml/json test's schema based on the :repo:`proto <test/tools/router_check/validation.proto>`.
-     Few known changes necessary are:
-     ``:authority`` input is now ``authority``.
-     ``:path`` input is now ``path``.
-     ``:method`` input is now ``method``. This is a required property.
-     ``additional_headers`` in the input along with ``header_fields`` and ``custom_header_fields`` contain ``key`` instead of ``field``.
-     ``tests`` is a root level field in the yaml/json.
+Usage
+  router_check_tool [-t <string>] [-c <string>] [-d] [-p] [--] [--version] [-h] <unlabelledConfigStrings>
+    -t <string>,  --test-path <string>
+      Path to a tool config JSON file. The tool config JSON file schema is found in
+      :ref:`config <config_tools_router_check_tool>`.
+      The tool config input file specifies urls (composed of authorities and paths)
+      and expected route parameter values. Additional parameters such as additional headers are optional.
+
+      Schema: All internal schemas in the tool are based on :repo:`proto3 <test/tools/router_check/validation.proto>`.
+      This is enabled by an extra optional parameter ``--useproto``.
+      This parameter will become the default in the future releases and enables more validation features in the tool.
+      Any new feature addition in validations will be added behind this parameter.
+
+      Migration: If you are currently using the tool and plan to migrate to use ``--useproto``,
+      change the yaml/json test's schema based on the :repo:`proto <test/tools/router_check/validation.proto>`.
+      Few known changes necessary are:
+      ``:authority`` input is now ``authority``.
+      ``:path`` input is now ``path``.
+      ``:method`` input is now ``method``. This is a required property.
+      ``additional_headers``, ``header_fields`` and ``custom_header_fields`` contain ``key`` instead of ``field``.
+      ``tests`` is a root level field in the yaml/json.
+
+    -c <string>,  --config-path <string>
+      Path to a v2 router config file (YAML or JSON). The router config file schema is found in
+      :ref:`config <envoy_api_file_envoy/api/v2/route/route.proto>` and the config file extension
+      must reflect its file type (for instance, .json for JSON and .yaml for YAML).
+
+    -d,  --details
+      Show detailed test execution results. The first line indicates the test name.
+
+    -p,  --useproto
+      Use Proto test file schema
+
+    -h,  --help
+      Displays usage information and exits.
+
+    <unlabelledConfigStrings>  (accepted multiple times)
+      If you haven't migrated to the proto schema, the binary accepts the paths to the 
+      router config file and the tool config JSON file in that particular order.
+      The ``--details`` parameter can be appended at the end to show the detailed test results.
 
 Output
   The program exits with status EXIT_FAILURE if any test case does not match the expected route parameter
   value.
 
-  The ``--details`` option prints out details for each test. The first line indicates the test name.
-
-  If a test fails, details of the failed test cases are printed. 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
+  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
   passed. In the failed test cases, conflict details are printed. ::
 
     Test_1
@@ -58,18 +76,13 @@ Building
     bazel build //test/tools/router_check:router_check_tool
 
 Running
-  The tool takes two input files and an optional command line parameter ``--details``. The
-  expected order of command line arguments is:
-  1. The router configuration file.
-  2. The tool configuration json file.
-  3. ``--useproto`` to use any new features in the tool.
-  4. The optional details flag. ::
+  Examples ::
 
     bazel-bin/test/tools/router_check/router_check_tool router_config.(yaml|json) tool_config.json
 
     bazel-bin/test/tools/router_check/router_check_tool router_config.(yaml|json) tool_config.json --details
 
-    bazel-bin/test/tools/router_check/router_check_tool router_config.(yaml|json) tool_config.json --details --useproto
+    bazel-bin/test/tools/router_check/router_check_tool -c router_config.(yaml|json) -t tool_config.json --details --useproto
 
 Testing
   A bash shell script test can be run with bazel. The test compares routes using different router and

test/tools/router_check/BUILD

@@ -22,6 +22,8 @@ envoy_cc_test_library(
         "router.cc",
         "router.h",
     ],
+    copts = ["-DHAVE_LONG_LONG"],
+    external_deps = ["tclap"],
     deps = [
         ":validation_proto_cc",
         "//source/common/config:rds_json_lib",

test/tools/router_check/router.cc

@@ -84,6 +84,7 @@ RouterCheckTool::RouterCheckTool(
     : factory_context_(std::move(factory_context)), config_(std::move(config)),
       stats_(std::move(stats)), api_(std::move(api)) {}
 
+// TODO(jyotima): Remove this code path once the json schema code path is deprecated.
 bool RouterCheckTool::compareEntriesInJson(const std::string& expected_route_json) {
   Json::ObjectSharedPtr loader = Json::Factory::loadFromFile(expected_route_json, *api_);
   loader->validateSchema(Json::ToolSchema::routerCheckSchema());
@@ -384,4 +385,41 @@ bool RouterCheckTool::compareResults(const std::string& actual, const std::strin
   }
   return false;
 }
+
+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::ValueArg<std::string> config_path("c", "config-path", "Path to configuration file.", false,
+                                           "", "string", cmd);
+  TCLAP::ValueArg<std::string> test_path("t", "test-path", "Path to test file.", false, "",
+                                         "string", cmd);
+  TCLAP::UnlabeledMultiArg<std::string> unlabelled_configs(
+      "unlabelled-configs", "unlabelled configs", false, "unlabelledConfigStrings", cmd);
+  try {
+    cmd.parse(argc, argv);
+  } catch (TCLAP::ArgException& e) {
+    std::cerr << "error: " << e.error() << std::endl;
+    exit(EXIT_FAILURE);
+  }
+
+  is_proto_ = is_proto.getValue();
+  is_detailed_ = is_detailed.getValue();
+
+  if (is_proto_) {
+    config_path_ = config_path.getValue();
+    test_path_ = test_path.getValue();
+    if (config_path_.empty() || test_path_.empty()) {
+      std::cerr << "error: "
+                << "Both --config-path/c and --test-path/t are mandatory with --useproto"
+                << std::endl;
+      exit(EXIT_FAILURE);
+    }
+  } else {
+    if (!unlabelled_configs.getValue().empty()) {
+      unlabelled_config_path_ = unlabelled_configs.getValue()[0];
+      unlabelled_test_path_ = unlabelled_configs.getValue()[1];
+    }
+  }
+}
 } // namespace Envoy

test/tools/router_check/router.h

@@ -21,6 +21,8 @@
 #include "test/tools/router_check/validation.pb.h"
 #include "test/tools/router_check/validation.pb.validate.h"
 
+#include "tclap/CmdLine.h"
+
 namespace Envoy {
 /**
  * Struct that stores the configuration parameters of the router check tool extracted from a json
@@ -136,4 +138,50 @@ class RouterCheckTool : Logger::Loggable<Logger::Id::testing> {
   std::unique_ptr<Stats::IsolatedStoreImpl> stats_;
   Api::ApiPtr api_;
 };
+
+/**
+ * Parses command line arguments for Router Check Tool.
+ */
+class Options {
+public:
+  Options(int argc, char** argv);
+
+  /**
+   * @return the path to configuration file.
+   */
+  const std::string& configPath() const { return config_path_; }
+
+  /**
+   * @return the path to test file.
+   */
+  const std::string& testPath() const { return test_path_; }
+
+  /**
+   * @return the path to json schema configuration file.
+   */
+  const std::string& unlabelledConfigPath() const { return unlabelled_config_path_; }
+
+  /**
+   * @return the path to json schema test file.
+   */
+  const std::string& unlabelledTestPath() const { return unlabelled_test_path_; }
+
+  /**
+   * @return true if proto schema test is used.
+   */
+  bool isProto() const { return is_proto_; }
+
+  /**
+   * @return true is detailed test execution results are displayed.
+   */
+  bool isDetailed() const { return is_detailed_; }
+
+private:
+  std::string test_path_;
+  std::string config_path_;
+  std::string unlabelled_test_path_;
+  std::string unlabelled_config_path_;
+  bool is_proto_;
+  bool is_detailed_;
+};
 } // namespace Envoy

test/tools/router_check/router_check.cc

@@ -4,38 +4,21 @@
 
 #include "test/tools/router_check/router.h"
 
-// TODO(jyotima): In the future, change this to use TCLAP
-bool isArgument(int argc, char* argv[], const std::string& argument) {
-  if (argc == 5 && (std::string(argv[3]) == argument || std::string(argv[4]) == argument)) {
-    return true;
-  }
-
-  if (argc == 4 && std::string(argv[3]) == argument) {
-    return true;
-  }
-
-  return false;
-}
-
 int main(int argc, char* argv[]) {
-  if (argc < 3 || argc > 5) {
-    return EXIT_FAILURE;
-  }
+  Envoy::Options options(argc, argv);
 
   try {
-    Envoy::RouterCheckTool checktool = Envoy::RouterCheckTool::create(argv[1]);
+    Envoy::RouterCheckTool checktool =
+        options.isProto() ? Envoy::RouterCheckTool::create(options.configPath())
+                          : Envoy::RouterCheckTool::create(options.unlabelledConfigPath());
 
-    if (isArgument(argc, argv, "--details")) {
+    if (options.isDetailed()) {
       checktool.setShowDetails();
     }
 
-    bool is_equal = true;
-    if (isArgument(argc, argv, "--useproto")) {
-      is_equal = checktool.compareEntries(argv[2]);
-    } else {
-      // TODO(jyotima): Remove this code path once the json schema code path is deprecated.
-      is_equal = checktool.compareEntriesInJson(argv[2]);
-    }
+    bool is_equal = options.isProto()
+                        ? checktool.compareEntries(options.testPath())
+                        : checktool.compareEntriesInJson(options.unlabelledTestPath());
     // Test fails if routes do not match what is expected
     if (!is_equal) {
       return EXIT_FAILURE;

test/tools/router_check/test/route_tests.sh

@@ -20,14 +20,14 @@ done
 # --useproto needs the test schema as a validation.proto message.
 for t in "${TESTS[@]}"
 do
-  TEST_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/${t}.yaml" "${PATH_CONFIG}/${t}.golden.proto.json" "--details" "--useproto")
+  TEST_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/${t}.yaml" "-t" "${PATH_CONFIG}/${t}.golden.proto.json" "--details" "--useproto")
 done
 
 # Test the yaml test file support
-TEST_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/Weighted.yaml" "${PATH_CONFIG}/Weighted.golden.proto.yaml" "--details" "--useproto")
+TEST_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/Weighted.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.yaml" "--details" "--useproto")
 
 # Test the proto text test file support
-TEST_OUTPUT=$("${PATH_BIN}" "${PATH_CONFIG}/Weighted.yaml" "${PATH_CONFIG}/Weighted.golden.proto.pb_text" "--details" "--useproto")
+TEST_OUTPUT=$("${PATH_BIN}" "-c" "${PATH_CONFIG}/Weighted.yaml" "-t" "${PATH_CONFIG}/Weighted.golden.proto.pb_text" "--details" "--useproto")
 
 # Bad config file
 echo testing bad config output