From f931113d6e264bba7eaf5182a2c78d7863774a69 Mon Sep 17 00:00:00 2001
From: Michael Davidsaver <mdavidsaver@gmail.com>
Date: Mon, 22 Apr 2024 12:23:33 -0700
Subject: [PATCH] re-write CLI argument parsing for testability

---
 tools/Makefile        |  14 ++++
 tools/cliutil.cpp     |  83 ++++++++++++++++++
 tools/cliutil.h       |  58 +++++++++++++
 tools/put.cpp         |  78 ++++++++---------
 tools/testcliutil.cpp | 110 ++++++++++++++++++++++++
 tools/testxput.cpp    | 190 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 495 insertions(+), 38 deletions(-)
 create mode 100644 tools/cliutil.cpp
 create mode 100644 tools/cliutil.h
 create mode 100644 tools/testcliutil.cpp
 create mode 100644 tools/testxput.cpp

diff --git a/tools/Makefile b/tools/Makefile
index 4e3eef48c..e05698b45 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -14,6 +14,8 @@ USR_CPPFLAGS += -I$(TOP)/src
 
 PROD_LIBS += pvxs Com
 
+PROD_SRCS += cliutil.cpp
+
 PROD += pvxvct
 pvxvct_SRCS += pvxvct.cpp
 
@@ -38,6 +40,18 @@ pvxlist_SRCS += list.cpp
 PROD += pvxmshim
 pvxmshim_SRCS += mshim.cpp
 
+# tests of CLI tools
+
+TESTPROD_HOST += testcliutil
+testcliutil_SRCS += testcliutil.cpp
+TESTS += testcliutil
+
+TESTPROD_HOST += testxput
+testxput_SRCS += testxput.cpp
+TESTS += testxput
+
+TESTSCRIPTS_HOST += $(TESTS:%=%.t)
+
 #===========================
 
 include $(TOP)/configure/RULES
diff --git a/tools/cliutil.cpp b/tools/cliutil.cpp
new file mode 100644
index 000000000..8a5a15ea0
--- /dev/null
+++ b/tools/cliutil.cpp
@@ -0,0 +1,83 @@
+/**
+ * Copyright - See the COPYRIGHT that is included with this distribution.
+ * pvxs is distributed subject to a Software License Agreement found
+ * in file LICENSE that is included with this distribution.
+ */
+
+#include "cliutil.h"
+
+namespace pvxs {
+
+bool operator==(const ArgVal& rhs, const ArgVal& lhs) {
+    return rhs.defined==lhs.defined && rhs.value==lhs.value;
+}
+
+GetOpt::GetOpt(int argc, char *argv[], const char *spec)
+    :argv0("<program name>")
+{
+    if(argc>=1)
+        argv0 = argv[0];
+
+    bool allpos = false; // after "--", treat all remaining as positional
+    for(int i=1; i<argc; i++) {
+        const char * arg = argv[i];
+        if(!allpos && arg[0]=='-') {
+            arg++;
+
+            if(arg[1]=='-') {
+                if(arg[2]=='\0') { // "--"
+                    allpos = true;
+                    continue;
+                }
+                // "--..." not supported
+                arguments.emplace_back(-1, argv[i]);
+                return;
+            }
+            // process as short args
+
+            for(; *arg; arg++) {
+                for(auto s=spec; *s; s++) {
+                    if(*s==*arg) { // match
+                        if(s[1]==':') { // need arg value
+                            if(arg[1]=='\0') { // "-a", "value"
+                                if(i+1==argc) {
+                                    // oops. no value
+                                    arguments.emplace_back('?', nullptr);
+                                    return;
+                                }
+                                arguments.emplace_back(*arg, argv[i+1]);
+                                i++;
+
+                            } else {
+                                // "-avalue"
+                                arguments.emplace_back(*arg, &arg[1]);
+                            }
+                            goto nextarg;
+
+                        } else { // flag
+                            arguments.emplace_back(*s, nullptr);
+                            // continue scanning for more flags.  eg. "-vv"
+                            goto nextchar;
+                        }
+                    } else {
+                        if(s[1]==':')
+                            s++;
+                    }
+                }
+                // unrecognized
+                arguments.emplace_back(-1, nullptr);
+                return;
+nextchar:
+                (void)0;  // need a statement after label...
+            }
+nextarg:
+            (void)0;
+
+        } else {
+            positional.push_back(arg);
+        }
+        success = true;
+    }
+}
+
+} // namespace pvxs
diff --git a/tools/cliutil.h b/tools/cliutil.h
new file mode 100644
index 000000000..d28ea3fe6
--- /dev/null
+++ b/tools/cliutil.h
@@ -0,0 +1,58 @@
+/**
+ * Copyright - See the COPYRIGHT that is included with this distribution.
+ * pvxs is distributed subject to a Software License Agreement found
+ * in file LICENSE that is included with this distribution.
+ */
+
+#ifndef CLIUTIL_H
+#define CLIUTIL_H
+
+#include <vector>
+#include <string>
+#include <stdexcept>
+#include <map> // for std::pair
+
+#include <utilpvt.h>
+
+namespace pvxs {
+
+struct ArgVal {
+    std::string value;
+    bool defined = false;
+
+    ArgVal() = default;
+    ArgVal(std::nullptr_t) {}
+    ArgVal(const std::string& value) :value(value), defined(true) {}
+    ArgVal(const char* value) :value(value), defined(true) {}
+
+    inline explicit
+    operator bool() const { return defined; }
+
+    inline
+    const std::string& operator*() const {
+        if(defined)
+            return value;
+        throw std::logic_error("Undefined argument value");
+    }
+
+    template<typename V>
+    inline
+    V as() const {
+        return parseTo<V>(**this);
+    }
+};
+
+bool operator==(const ArgVal& rhs, const ArgVal& lhs);
+
+struct GetOpt {
+    GetOpt(int argc, char *argv[], const char *spec);
+
+    const char *argv0;
+    std::vector<std::string> positional;
+    std::vector<std::pair<char, ArgVal>> arguments;
+    bool success = false;
+};
+
+} // namespace pvxs
+
+#endif // CLIUTIL_H
diff --git a/tools/put.cpp b/tools/put.cpp
index cf21c17f5..9f9b568b0 100644
--- a/tools/put.cpp
+++ b/tools/put.cpp
@@ -9,7 +9,6 @@
 #include <atomic>
 
 #include <epicsVersion.h>
-#include <epicsGetopt.h>
 #include <epicsThread.h>
 
 #include <pvxs/client.h>
@@ -17,6 +16,11 @@
 #include <pvxs/json.h>
 #include "utilpvt.h"
 #include "evhelper.h"
+#include "cliutil.h"
+
+#ifndef REALMAIN
+#  define REALMAIN main
+#endif
 
 using namespace pvxs;
 
@@ -35,9 +39,9 @@ void usage(const char* argv0)
                ;
 }
 
-}
+} // namespace
 
-int main(int argc, char *argv[])
+int REALMAIN(int argc, char *argv[])
 {
     try {
         logger_config_env(); // from $PVXS_LOG
@@ -45,55 +49,53 @@ int main(int argc, char *argv[])
         bool verbose = false;
         std::string request;
 
-        {
-            int opt;
-            while ((opt = getopt(argc, argv, "hvVdw:r:")) != -1) {
-                switch(opt) {
-                case 'h':
-                    usage(argv[0]);
-                    return 0;
-                case 'V':
-                    std::cout<<pvxs::version_information;
-                    return 0;
-                case 'v':
-                    verbose = true;
-                    break;
-                case 'd':
-                    logger_level_set("pvxs.*", Level::Debug);
-                    break;
-                case 'w':
-                    timeout = parseTo<double>(optarg);
-                    break;
-                case 'r':
-                    request = optarg;
-                    break;
-                default:
-                    usage(argv[0]);
-                    std::cerr<<"\nUnknown argument: "<<char(opt)<<std::endl;
-                    return 1;
-                }
+        GetOpt opts(argc, argv, "hvVdw:r:");
+        for(auto& pair : opts.arguments) {
+            switch(pair.first) {
+            case 'h':
+                usage(opts.argv0);
+                return 0;
+            case 'V':
+                std::cout<<pvxs::version_information;
+                return 0;
+            case 'v':
+                verbose = true;
+                break;
+            case 'd':
+                logger_level_set("pvxs.*", Level::Debug);
+                break;
+            case 'w':
+                timeout = pair.second.as<double>();
+                break;
+            case 'r':
+                request = *pair.second;
+                break;
+            default:
+                usage(opts.argv0);
+                std::cerr<<"\nUnknown argument: "<<pair.first<<std::endl;
+                return 1;
             }
         }
 
-        if(optind==argc) {
-            usage(argv[0]);
-            std::cerr<<"\nExpected PV name\n";
+        if(opts.positional.size()<2) {
+            usage(opts.argv0);
+            std::cerr<<"\nExpected PV name and at least one value\n";
             return 1;
         }
 
-        std::string pvname(argv[optind++]);
+        const auto& pvname = opts.positional.front();
         std::map<std::string, std::string> values;
 
-        if(argc-optind==1 && std::string(argv[optind]).find_first_of('=')==std::string::npos) {
+        if(opts.positional.size()==2 && std::string(opts.positional[1]).find_first_of('=')==std::string::npos) {
             // only one field assignment, and field name omitted.
             // if JSON map, treat as entire struct.  Others imply .value
 
-            auto sval(argv[optind]);
+            const auto& sval = opts.positional[1];
             values[sval[0]=='{' ? "" : "value"] = sval;
 
         } else {
-            for(auto n : range(optind, argc)) {
-                std::string fv(argv[n]);
+            for(auto n : range(size_t(1), opts.positional.size())) {
+                std::string fv(opts.positional[n]);
                 auto sep = fv.find_first_of('=');
 
                 if(sep==std::string::npos) {
diff --git a/tools/testcliutil.cpp b/tools/testcliutil.cpp
new file mode 100644
index 000000000..cb9e718ae
--- /dev/null
+++ b/tools/testcliutil.cpp
@@ -0,0 +1,110 @@
+/**
+ * Copyright - See the COPYRIGHT that is included with this distribution.
+ * pvxs is distributed subject to a Software License Agreement found
+ * in file LICENSE that is included with this distribution.
+ */
+
+#include <vector>
+#include <initializer_list>
+
+#include <string.h>
+
+#include <pvxs/unittest.h>
+#include <pvxs/util.h>
+
+#include <testMain.h>
+#include <epicsUnitTest.h>
+#include <dbDefs.h>
+
+#include "cliutil.h"
+
+namespace pvxs { namespace detail {
+
+template<typename E>
+struct test_print<std::vector<E>> {
+    template<class C>
+    static inline void op(C& strm, const std::vector<E>& v) {
+        bool first = true;
+        for(auto& e : v) {
+            if(first) {
+                first = false;
+            } else {
+                strm<<' ';
+            }
+            test_print<E>::op(strm, e);
+        }
+    }
+};
+
+template<>
+struct test_print<std::pair<char, ArgVal>> {
+    template<class C>
+    static inline void op(C& strm, const std::pair<char, ArgVal>& v) {
+        strm<<'-'<<v.first;
+        if(v.second.defined)
+            strm<<" "<<pvxs::escape(v.second.value);
+    }
+};
+
+}} // namespace::detail
+
+MAIN(testcliutil) {
+    testPlan(15);
+
+    {
+        testDiag("case @%d", __LINE__);
+        const char* argv[] = {"exe", "-v", "-a", "Aa", "hello"};
+        pvxs::GetOpt opts(NELEMENTS(argv), const_cast<char**>(argv), "a:v");
+        testOk(strcmp(opts.argv0, "exe")==0, "%s", opts.argv0);
+        decltype (opts.arguments) arguments({{'v', nullptr}, {'a',"Aa"}});
+        testArrEq(opts.arguments, arguments);
+        decltype (opts.positional) positional({"hello"});
+        testArrEq(opts.positional, positional);
+    }
+
+    {
+        testDiag("case @%d", __LINE__);
+        const char* argv[] = {"exe", "-v", "-a", "Aa", "hello"};
+        pvxs::GetOpt opts(NELEMENTS(argv), const_cast<char**>(argv), "va:");
+        testOk(strcmp(opts.argv0, "exe")==0, "%s", opts.argv0);
+        decltype (opts.arguments) arguments({{'v', nullptr}, {'a',"Aa"}});
+        testArrEq(opts.arguments, arguments);
+        decltype (opts.positional) positional({"hello"});
+        testArrEq(opts.positional, positional);
+    }
+
+    {
+        testDiag("case @%d", __LINE__);
+        const char* argv[] = {"exe", "-v", "hello", "-aAa"};
+        pvxs::GetOpt opts(NELEMENTS(argv), const_cast<char**>(argv), "va:");
+        testOk(strcmp(opts.argv0, "exe")==0, "%s", opts.argv0);
+        decltype (opts.arguments) arguments({{'v', nullptr}, {'a',"Aa"}});
+        testArrEq(opts.arguments, arguments);
+        decltype (opts.positional) positional({"hello"});
+        testArrEq(opts.positional, positional);
+    }
+
+    {
+        testDiag("case @%d", __LINE__);
+        const char* argv[] = {"exe", "-v", "hello", "-a"}; // missing value
+        pvxs::GetOpt opts(NELEMENTS(argv), const_cast<char**>(argv), "va:");
+        testOk(strcmp(opts.argv0, "exe")==0, "%s", opts.argv0);
+        decltype (opts.arguments) arguments({{'v', nullptr}, {'?',nullptr}});
+        testArrEq(opts.arguments, arguments);
+        decltype (opts.positional) positional({"hello"});
+        testArrEq(opts.positional, positional);
+    }
+
+    {
+        testDiag("case @%d", __LINE__);
+        const char* argv[] = {"exe", "-vvaTest", "hello"};
+        pvxs::GetOpt opts(NELEMENTS(argv), const_cast<char**>(argv), "va:");
+        testOk(strcmp(opts.argv0, "exe")==0, "%s", opts.argv0);
+        decltype (opts.arguments) arguments({{'v', nullptr}, {'v', nullptr}, {'a', "Test"}});
+        testArrEq(opts.arguments, arguments);
+        decltype (opts.positional) positional({"hello"});
+        testArrEq(opts.positional, positional);
+    }
+
+    return testDone();
+}
diff --git a/tools/testxput.cpp b/tools/testxput.cpp
new file mode 100644
index 000000000..28a4d1df3
--- /dev/null
+++ b/tools/testxput.cpp
@@ -0,0 +1,190 @@
+/**
+ * Copyright - See the COPYRIGHT that is included with this distribution.
+ * pvxs is distributed subject to a Software License Agreement found
+ * in file LICENSE that is included with this distribution.
+ */
+
+#include <sstream>
+#include <functional>
+
+#include <testMain.h>
+
+#include <envDefs.h>
+#include <dbDefs.h>
+#include <epicsUnitTest.h>
+
+#include <pvxs/server.h>
+#include <pvxs/sharedpv.h>
+#include <pvxs/nt.h>
+#include <pvxs/unittest.h>
+
+#define REALMAIN pvxput
+#include "put.cpp"
+
+using namespace pvxs;
+
+namespace {
+struct Redirect {
+    std::ostream& ref;
+    std::streambuf *prev;
+    Redirect(std::ostream& ref, std::ostream& target)
+        :ref(ref)
+        ,prev(ref.rdbuf(target.rdbuf()))
+    {}
+    ~Redirect() {
+        ref.rdbuf(prev);
+    }
+};
+
+struct Run {
+    std::string out, err;
+    int code;
+
+    Run(std::initializer_list<std::string> args) {
+        std::vector<std::string> argsxx(args.begin(), args.end());
+        std::vector<const char*> argv({"pvxput"});
+        for(auto& arg : argsxx) {
+            argv.push_back(arg.c_str());
+        }
+        std::ostringstream out, err;
+        {
+            Redirect rd_cout(std::cout, out);
+            Redirect rd_cerr(std::cerr, err);
+            try{
+                code = pvxput(argv.size(), (char**)argv.data());
+            }catch(std::exception& e){
+                testFail("Uncaught c++ exception: %s", e.what());
+                code = -1;
+            }
+        }
+        this->out = out.str();
+        this->err = err.str();
+    }
+
+    Run& exitWith(int expect) {
+        testOk(expect==code, "%d == %d", expect, code);
+        return *this;
+    }
+    Run& success() {
+        return exitWith(0);
+    }
+
+    testCase _lineMatching(const std::string& inp, const std::string& expr) {
+        std::istringstream strm(inp);
+        std::string line;
+        testCase c;
+        while(std::getline(strm, line)) {
+            if(c.setPassMatch(expr, line))
+                break;
+        }
+        return c;
+    }
+
+    inline
+    testCase outMatch(const std::string& expr) {
+        return _lineMatching(out, expr);
+    }
+    inline
+    testCase errMatch(const std::string& expr) {
+        return _lineMatching(err, expr);
+    }
+};
+
+} // namespace
+
+MAIN(testxput)
+{
+    testPlan(26);
+
+    auto pvI32(server::SharedPV::buildMailbox());
+    pvI32.open(nt::NTScalar{TypeCode::Int32}.create()
+               .update("value", 5));
+
+    auto pvS(server::SharedPV::buildMailbox());
+    pvS.open(nt::NTScalar{TypeCode::String}.create()
+               .update("value", "foo"));
+
+    auto pvE(server::SharedPV::buildMailbox());
+    pvE.open(nt::NTEnum{}.create()
+             .update("value.index", 0)
+             .update("value.choices", shared_array<const std::string>({"one", "two"})));
+
+    // setup isolated server
+    auto srv(server::Config::isolated()
+             .build()
+             .addPV("testI32", pvI32)
+             .addPV("testS", pvS)
+             .addPV("testE", pvE)
+             .start());
+
+    // setup environment for pvxput() to find only our isolated server
+    {
+        client::Config::defs_t envs;
+        srv.clientConfig().updateDefs(envs);
+        for(const auto& pair : envs) {
+            testShow()<<" "<<pair.first<<"="<<pair.second;
+            epicsEnvSet(pair.first.c_str(), pair.second.c_str());
+        }
+    }
+
+    {
+        Run run({"-v", "-d", "-w1", "-w", "2", "-r", "foo", "-h"});
+        run.success();
+        run.outMatch(".*Show this message.*");
+    }
+
+    {
+        Run run({"-V"});
+        run.success();
+        run.outMatch(".*PVXS.*");
+        run.outMatch(".*libevent.*");
+    }
+
+    {
+        Run run({});
+        run.exitWith(1);
+        testStrEq(run.out, "");
+        run.errMatch(".*Expected PV name.*");
+    }
+
+    {
+        Run run({"-w", "foo"});
+        run.exitWith(1);
+        testStrEq(run.out, "");
+        run.errMatch(".*Invalid.*foo.*");
+    }
+
+    Run({"testI32", "6"}).success();
+    testEq(pvI32.fetch()["value"].as<int32_t>(), 6);
+
+    Run({"testI32", "value=7"}).success();
+    testEq(pvI32.fetch()["value"].as<int32_t>(), 7);
+
+    Run({"testI32", R"({"value":8})"}).success();
+    testEq(pvI32.fetch()["value"].as<int32_t>(), 8);
+
+
+    Run({"testS", "hello"}).success();
+    testEq(pvS.fetch()["value"].as<std::string>(), "hello");
+
+    Run({"testS", "value=world"}).success();
+    testEq(pvS.fetch()["value"].as<std::string>(), "world");
+
+    Run({"testS", R"({"value":"baz"})"}).success();
+    testEq(pvS.fetch()["value"].as<std::string>(), "baz");
+
+
+    Run({"testE", "hello"}).exitWith(1); // invalid choice
+    testEq(pvE.fetch()["value.index"].as<int32_t>(), 0);
+
+    Run({"testE", "two"}).success();
+    testEq(pvE.fetch()["value.index"].as<int32_t>(), 1);
+
+    Run({"testE", "0"}).success();
+    testEq(pvE.fetch()["value.index"].as<int32_t>(), 0);
+
+    Run({"testE", "42"}).success(); // can set arbitrary index
+    testEq(pvE.fetch()["value.index"].as<int32_t>(), 42);
+
+    return testDone();
+}