Enable manual tests in CI:

Fixes: RIPD-1575. Fix argument passing to runner. Allow multiple unit
test selectors to be passed via --unittest argument. Add optional
integer priority value to test suite list. Fix several failing manual
tests. Update CLI usage message to make it clearer.

Jenkinsfile

@@ -89,6 +89,7 @@ try {
 
     stage ('Parallel Build') {
         String[][] variants = [
+            ['gcc.release.unity', '-Dassert=true', 'MANUAL_TESTS=true'],
             ['coverage'],
             ['docs'],
             ['msvc.debug'],
@@ -101,8 +102,8 @@ try {
             ['clang.debug.nounity'],
             ['gcc.debug.unity'],
             ['gcc.debug.nounity'],
-            ['clang.release.unity'],
-            ['gcc.release.unity'],
+            ['clang.release.unity', '-Dassert=true'],
+            ['gcc.release.unity', '-Dassert=true'],
             // add a static build just to make sure it works
             ['gcc.debug.unity', '-Dstatic=true'],
             // TODO - sanitizer runs currently fail

bin/ci/ubuntu/build-and-test.sh

@@ -96,8 +96,34 @@ echo "using APP_PATH: $APP_PATH"
 # See what we've actually built
 ldd $APP_PATH
 
+function join_by { local IFS="$1"; shift; echo "$*"; }
+
+# This is a list of manual tests
+# in rippled that we want to run
+declare -a manual_tests=(
+  "beast.chrono.abstract_clock"
+  "beast.unit_test.print"
+  "ripple.NodeStore.Timing"
+  "ripple.app.Flow_manual"
+  "ripple.app.NoRippleCheckLimits"
+  "ripple.app.PayStrandAllPairs"
+  "ripple.consensus.ByzantineFailureSim"
+  "ripple.consensus.DistributedValidators"
+  "ripple.consensus.ScaleFreeSim"
+  "ripple.ripple_data.digest"
+  "ripple.tx.CrossingLimits"
+  "ripple.tx.FindOversizeCross"
+  "ripple.tx.Offer_manual"
+  "ripple.tx.OversizeMeta"
+  "ripple.tx.PlumpBook"
+)
+
 if [[ ${APP} == "rippled" ]]; then
-  APP_ARGS+="--unittest --quiet --unittest-log"
+  if [[ ${MANUAL_TESTS:-} == true ]]; then
+    APP_ARGS+="--unittest=$(join_by , "${manual_tests[@]}")"
+  else
+    APP_ARGS+="--unittest --quiet --unittest-log"
+  fi
   # Only report on src/ripple files
   export LCOV_FILES="*/src/ripple/*"
   # Nothing to explicitly exclude

src/beast/extras/beast/unit_test/global_suites.hpp

@@ -28,10 +28,10 @@ template<class Suite>
 struct insert_suite
 {
     insert_suite(char const* name, char const* module,
-        char const* library, bool manual)
+        char const* library, bool manual, int priority)
     {
         global_suites().insert<Suite>(
-            name, module, library, manual);
+            name, module, library, manual, priority);
     }
 };
 

src/beast/extras/beast/unit_test/suite.hpp

@@ -627,10 +627,10 @@ run(runner& r)
 
 // detail:
 // This inserts the suite with the given manual flag
-#define BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,manual) \
+#define BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,manual,priority) \
     static beast::unit_test::detail::insert_suite <Class##_test>   \
         Library ## Module ## Class ## _test_instance(             \
-            #Class, #Module, #Library, manual)
+            #Class, #Module, #Library, manual, priority)
 
 //------------------------------------------------------------------------------
 
@@ -675,14 +675,19 @@ run(runner& r)
 #if BEAST_NO_UNIT_TEST_INLINE
 #define BEAST_DEFINE_TESTSUITE(Class,Module,Library)
 #define BEAST_DEFINE_TESTSUITE_MANUAL(Class,Module,Library)
+#define BEAST_DEFINE_TESTSUITE_PRIO(Class,Module,Library,Priority)
+#define BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Class,Module,Library,Priority)
 
 #else
 #include <beast/unit_test/global_suites.hpp>
 #define BEAST_DEFINE_TESTSUITE(Class,Module,Library) \
-        BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,false)
+        BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,false,0)
 #define BEAST_DEFINE_TESTSUITE_MANUAL(Class,Module,Library) \
-        BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,true)
-
+        BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,true,0)
+#define BEAST_DEFINE_TESTSUITE_PRIO(Class,Module,Library,Priority) \
+        BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,false,Priority)
+#define BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Class,Module,Library,Priority) \
+        BEAST_DEFINE_TESTSUITE_INSERT(Class,Module,Library,true,Priority)
 #endif
 
 #endif

src/beast/extras/beast/unit_test/suite_info.hpp

@@ -27,6 +27,7 @@ class suite_info
     std::string module_;
     std::string library_;
     bool manual_;
+    int priority_;
     run_type run_;
 
 public:
@@ -35,11 +36,13 @@ class suite_info
             std::string module,
             std::string library,
             bool manual,
+            int priority,
             run_type run)
         : name_(std::move(name))
         , module_(std::move(module))
         , library_(std::move(library))
         , manual_(manual)
+        , priority_(priority)
         , run_(std::move(run))
     {
     }
@@ -87,9 +90,10 @@ class suite_info
     bool
     operator<(suite_info const& lhs, suite_info const& rhs)
     {
-        return
-            std::tie(lhs.library_, lhs.module_, lhs.name_) <
-            std::tie(rhs.library_, rhs.module_, rhs.name_);
+        // we want higher priority suites sorted first, thus the negation
+        // of priority value here
+        return std::forward_as_tuple(-lhs.priority_, lhs.library_, lhs.module_, lhs.name_) <
+               std::forward_as_tuple(-rhs.priority_, rhs.library_, rhs.module_, rhs.name_);
     }
 };
 
@@ -102,13 +106,15 @@ make_suite_info(
     std::string name,
     std::string module,
     std::string library,
-    bool manual)
+    bool manual,
+    int priority)
 {
     return suite_info(
         std::move(name),
         std::move(module),
         std::move(library),
         manual,
+        priority,
         [](runner& r)
         {
             Suite{}(r);

src/beast/extras/beast/unit_test/suite_list.hpp

@@ -39,7 +39,8 @@ class suite_list
         char const* name,
         char const* module,
         char const* library,
-        bool manual);
+        bool manual,
+        int priority);
 };
 
 //------------------------------------------------------------------------------
@@ -50,7 +51,8 @@ suite_list::insert(
     char const* name,
     char const* module,
     char const* library,
-    bool manual)
+    bool manual,
+    int priority)
 {
 #ifndef NDEBUG
     {
@@ -67,7 +69,7 @@ suite_list::insert(
     }
 #endif
     cont().emplace(make_suite_info<Suite>(
-        name, module, library, manual));
+        name, module, library, manual, priority));
 }
 
 } // unit_test

src/ripple/app/main/Main.cpp

@@ -182,6 +182,40 @@ void printHelp (const po::options_description& desc)
 
 //------------------------------------------------------------------------------
 
+/* simple unit test selector that allows a comma separated list
+ * of selectors
+ */
+class multi_selector
+{
+private:
+    std::vector<beast::unit_test::selector> selectors_;
+public:
+    explicit
+    multi_selector(std::string const& patterns = "")
+    {
+        std::vector<std::string> v;
+        boost::split (v, patterns, boost::algorithm::is_any_of (","));
+        selectors_.reserve(v.size());
+        std::for_each(v.begin(), v.end(),
+            [this](std::string s)
+            {
+                boost::trim (s);
+                if (selectors_.empty() || !s.empty())
+                    selectors_.emplace_back(
+                        beast::unit_test::selector::automatch, s);
+            });
+    }
+
+    bool
+    operator()(beast::unit_test::suite_info const& s)
+    {
+        for (auto& sel : selectors_)
+            if (sel(s))
+                return true;
+        return false;
+    }
+};
+
 static int runUnitTests(
     std::string const& pattern,
     std::string const& argument,
@@ -202,7 +236,8 @@ static int runUnitTests(
         multi_runner_parent parent_runner;
 
         multi_runner_child child_runner{num_jobs, quiet, log};
-        auto const any_failed = child_runner.run_multi(match_auto(pattern));
+        child_runner.arg(argument);
+        auto const any_failed = child_runner.run_multi(multi_selector(pattern));
 
         if (any_failed)
             return EXIT_FAILURE;
@@ -251,7 +286,8 @@ static int runUnitTests(
     {
         // child
         multi_runner_child runner{num_jobs, quiet, log};
-        auto const anyFailed = runner.run_multi(match_auto(pattern));
+        runner.arg(argument);
+        auto const anyFailed = runner.run_multi(multi_selector(pattern));
 
         if (anyFailed)
             return EXIT_FAILURE;
@@ -291,50 +327,99 @@ int run (int argc, char** argv)
 
     // Set up option parsing.
     //
-    po::options_description desc ("General Options");
-    desc.add_options ()
-    ("help,h", "Display this message.")
+    po::options_description gen ("General Options");
+    gen.add_options ()
     ("conf", po::value<std::string> (), "Specify the configuration file.")
-    ("rpc", "Perform rpc command (default).")
-    ("rpc_ip", po::value <std::string> (), "Specify the IP address for RPC command. Format: <ip-address>[':'<port-number>]")
-    ("rpc_port", po::value <std::uint16_t> (), "Specify the port number for RPC command.")
-    ("standalone,a", "Run with no peers.")
-    ("unittest,u", po::value <std::string> ()->implicit_value (""), "Perform unit tests.")
-    ("unittest-arg", po::value <std::string> ()->implicit_value (""), "Supplies argument to unit tests.")
-    ("unittest-log", po::value <std::string> ()->implicit_value (""), "Force unit test log output, even in quiet mode.")
-#if HAS_BOOST_PROCESS
-    ("unittest-jobs", po::value <std::size_t> (), "Number of unittest jobs to run.")
-    ("unittest-child", "For internal use only. Run the process as a unit test child process.")
-#endif
-    ("parameters", po::value< vector<string> > (), "Specify comma separated parameters.")
-    ("quiet,q", "Reduce diagnotics.")
-    ("quorum", po::value <std::size_t> (), "Override the minimum validation quorum.")
+    ("debug", "Enable normally suppressed debug logging")
+    ("fg", "Run in the foreground.")
+    ("help,h", "Display this message.")
+    ("quorum", po::value <std::size_t> (),
+        "Override the minimum validation quorum.")
     ("silent", "No output to the console after startup.")
+    ("standalone,a", "Run with no peers.")
     ("verbose,v", "Verbose logging.")
-    ("load", "Load the current ledger from the local DB.")
-    ("valid", "Consider the initial ledger a valid network ledger.")
-    ("replay","Replay a ledger close.")
-    ("ledger", po::value<std::string> (), "Load the specified ledger and start from .")
+    ("version", "Display the build version.")
+    ;
+
+    po::options_description data ("Ledger/Data Options");
+    data.add_options ()
+    ("import", importText.c_str ())
+    ("ledger", po::value<std::string> (),
+        "Load the specified ledger and start from the value given.")
     ("ledgerfile", po::value<std::string> (), "Load the specified ledger file.")
-    ("start", "Start from a fresh Ledger.")
+    ("load", "Load the current ledger from the local DB.")
     ("net", "Get the initial ledger from the network.")
-    ("debug", "Enable normally suppressed debug logging")
-    ("fg", "Run in the foreground.")
-    ("import", importText.c_str ())
     ("nodetoshard", "Import node store into shards")
+    ("replay","Replay a ledger close.")
+    ("start", "Start from a fresh Ledger.")
+    ("valid", "Consider the initial ledger a valid network ledger.")
     ("validateShards", shardsText.c_str ())
-    ("version", "Display the build version.")
+    ;
+
+    po::options_description rpc ("RPC Client Options");
+    rpc.add_options()
+    ("rpc",
+        "Perform rpc command - see below for available commands. "
+        "This is assumed if any positional parameters are provided.")
+    ("rpc_ip", po::value <std::string> (),
+        "Specify the IP address for RPC command. "
+        "Format: <ip-address>[':'<port-number>]")
+    ("rpc_port", po::value <std::uint16_t> (),
+        "Specify the port number for RPC command.")
+    ;
+
+    po::options_description test ("Unit Test Options");
+    test.add_options()
+    ("quiet,q",
+        "Suppress test suite messages, "
+        "including suite/case name (at start) and test log messages.")
+    ("unittest,u", po::value <std::string> ()->implicit_value (""),
+        "Perform unit tests. The optional argument specifies one or "
+        "more comma-separated selectors. Each selector specifies a suite name, "
+        "full-name (lib.module.suite), module, or library "
+        "(checked in that ""order).")
+    ("unittest-arg", po::value <std::string> ()->implicit_value (""),
+        "Supplies an argument string to unit tests. If provided, this argument "
+        "is made available to each suite that runs. Interpretation of the "
+        "argument is handled individually by any suite that accesses it -- "
+        "as such, it typically only make sense to provide this when running "
+        "a single suite.")
+    ("unittest-log",
+        "Force unit test log message output. Only useful in combination with "
+        "--quiet, in which case log messages will print but suite/case names "
+        "will not.")
+#if HAS_BOOST_PROCESS
+    ("unittest-jobs", po::value <std::size_t> (),
+        "Number of unittest jobs to run in parallel (child processes).")
+#endif
+    ;
+
+    // These are hidden options, not intended to be shown in the usage/help message
+    po::options_description hidden ("Hidden Options");
+    hidden.add_options()
+    ("parameters", po::value< vector<string> > (),
+        "Specify rpc command and parameters. This option must be repeated "
+        "for each command/param. Positional parameters also serve this purpose, "
+        "so this option is not needed for users")
+    ("unittest-child",
+        "For internal use only when spawning child unit test processes.")
     ;
 
     // Interpret positional arguments as --parameters.
     po::positional_options_description p;
     p.add ("parameters", -1);
 
+    po::options_description all;
+    all.add(gen).add(rpc).add(data).add(test).add(hidden);
+
+    po::options_description desc;
+    desc.add(gen).add(rpc).add(data).add(test);
+
     // Parse options, if no error.
     try
     {
         po::store (po::command_line_parser (argc, argv)
-            .options (desc)               // Parse options.
+            .options (all)                // Parse options.
             .positional (p)               // Remainder as --parameters.
             .run (),
             vm);