Fix merge up of libtest.

libtest/blobslap_worker.cc

@@ -122,24 +122,19 @@ public:
     return "benchmark/blobslap_worker";
   }
 
-  const char *pid_file_option()
-  {
-    return "--pid-file=";
-  }
-
   const char *daemon_file_option()
   {
     return "--daemon";
   }
 
-  const char *log_file_option()
+  bool has_port_option() const
   {
-    return "--log-file=";
+    return true;
   }
 
-  const char *port_option()
+  bool has_log_file_option() const
   {
-    return "--port=";
+    return true;
   }
 
   bool is_libtool()
@@ -147,23 +142,21 @@ public:
     return true;
   }
 
-  bool build(int argc, const char *argv[]);
+  bool build(size_t argc, const char *argv[]);
 };
 
 
 #include <sstream>
 
-bool BlobslapWorker::build(int argc, const char *argv[])
+bool BlobslapWorker::build(size_t argc, const char *argv[])
 {
   std::stringstream arg_buffer;
 
-  for (int x= 1 ; x < argc ; x++)
+  for (size_t x= 0 ; x < argc ; x++)
   {
-    arg_buffer << " " << argv[x] << " ";
+    add_option(argv[x]);
   }
 
-  set_extra_args(arg_buffer.str());
-
   return true;
 }
 

libtest/cmdline.cc

@@ -68,6 +68,21 @@ namespace {
     return arg_buffer.str();
   }
 
+  static Application::error_t int_to_error_t(int arg)
+  {
+    switch (arg)
+    {
+    case 127:
+      return Application::INVALID;
+
+    case 0:
+      return Application::SUCCESS;
+
+    default:
+    case 1:
+      return Application::FAILURE;
+    }
+  }
 }
 
 namespace libtest {
@@ -214,16 +229,17 @@ Application::error_t Application::wait()
     }
     else
     {
-      assert(waited_pid == _pid);
-      exit_code= error_t(exited_successfully(status));
+      if (waited_pid != _pid)
+      {
+        throw libtest::fatal(LIBYATL_DEFAULT_PARAM, "Pid mismatch, %d != %d", int(waited_pid), int(_pid));
+      }
+      exit_code= int_to_error_t(exited_successfully(status));
     }
   }
 
   if (exit_code == Application::INVALID)
   {
-#if 0
     Error << print_argv(built_argv, _argc, _pid);
-#endif
   }
 
   return exit_code;
@@ -408,9 +424,8 @@ int exec_cmdline(const std::string& command, const char *args[], bool use_libtoo
   {
     return int(ret);
   }
-  ret= app.wait();
 
-  return int(ret);
+  return int(app.wait());
 }
 
 const char *gearmand_binary() 

libtest/cmdline.h

@@ -129,6 +129,9 @@ static inline std::ostream& operator<<(std::ostream& output, const enum Applicat
     case Application::INVALID:
       output << "127";
       break;
+
+    default:
+      output << "EXIT_UNKNOWN";
   }
 
   return output;

libtest/gearmand.cc

@@ -164,24 +164,25 @@ public:
     return GEARMAND_BINARY;
   }
 
-  const char *pid_file_option()
-  {
-    return "--pid-file=";
-  }
-
   const char *daemon_file_option()
   {
     return "--daemon";
   }
 
-  const char *log_file_option()
+  void log_file_option(Application& app, const std::string& arg)
   {
-    return "--verbose=DEBUG --log-file=";
+    if (arg.empty() == false)
+    {
+      std::string buffer("--log-file=");
+      buffer+= arg;
+      app.add_option("--verbose=DEBUG");
+      app.add_option(buffer);
+    }
   }
 
-  const char *port_option()
+  bool has_log_file_option() const
   {
-    return "--port=";
+    return true;
   }
 
   bool is_libtool()
@@ -194,27 +195,30 @@ public:
     return true;
   }
 
-  bool build(int argc, const char *argv[]);
+  bool has_port_option() const
+  {
+    return true;
+  }
+
+  bool build(size_t argc, const char *argv[]);
 };
 
-bool Gearmand::build(int argc, const char *argv[])
+bool Gearmand::build(size_t argc, const char *argv[])
 {
   std::stringstream arg_buffer;
 
   if (getuid() == 0 or geteuid() == 0)
   {
-    arg_buffer << " -u root ";
+    add_option("-u", "root");
   }
 
-  arg_buffer << " --listen=localhost ";
+  add_option("--listen=localhost");
 
-  for (int x= 1 ; x < argc ; x++)
+  for (size_t x= 0 ; x < argc ; x++)
   {
-    arg_buffer << " " << argv[x] << " ";
+    add_option(argv[x]);
   }
 
-  set_extra_args(arg_buffer.str());
-
   return true;
 }
 

libtest/memcached.cc

@@ -160,9 +160,12 @@ public:
     return MEMCACHED_BINARY;
   }
 
-  const char *pid_file_option()
+  virtual void pid_file_option(Application& app, const std::string& arg)
   {
-    return "-P ";
+    if (arg.empty() == false)
+    {
+      app.add_option("-P", arg);
+    }
   }
 
   const char *socket_file_option() const
@@ -175,14 +178,29 @@ public:
     return "-d";
   }
 
-  const char *log_file_option()
+  virtual void port_option(Application& app, in_port_t arg)
   {
-    return NULL;
+    char buffer[30];
+    snprintf(buffer, sizeof(buffer), "%d", int(arg));
+    app.add_option("-p", buffer); 
+  }
+
+  bool has_port_option() const
+  {
+    return true;
   }
 
-  const char *port_option()
+  bool has_socket_file_option() const
   {
-    return "-p ";
+    return has_socket();
+  }
+
+  void socket_file_option(Application& app, const std::string& socket_arg)
+  {
+    if (socket_arg.empty() == false)
+    {
+      app.add_option("-s", socket_arg);
+    }
   }
 
   bool is_libtool()
@@ -201,7 +219,7 @@ public:
     return true;
   }
 
-  bool build(int argc, const char *argv[]);
+  bool build(size_t argc, const char *argv[]);
 };
 
 class MemcachedSaSL : public Memcached
@@ -294,31 +312,29 @@ public:
 
 #include <sstream>
 
-bool Memcached::build(int argc, const char *argv[])
+bool Memcached::build(size_t argc, const char *argv[])
 {
   std::stringstream arg_buffer;
 
   if (getuid() == 0 or geteuid() == 0)
   {
-    arg_buffer << " -u root ";
+    add_option("-u", "root");
   }
 
-  arg_buffer << " -l localhost ";
-  arg_buffer << " -m 128 ";
-  arg_buffer << " -M ";
+  add_option("-l", "localhost");
+  add_option("-m", "128");
+  add_option("-M");
 
   if (sasl())
   {
-    arg_buffer << sasl();
+    add_option(sasl());
   }
 
   for (int x= 1 ; x < argc ; x++)
   {
-    arg_buffer << " " << argv[x] << " ";
+    add_option(argv[x]);
   }
 
-  set_extra_args(arg_buffer.str());
-
   return true;
 }
 

libtest/server.cc

@@ -42,42 +42,6 @@ static inline std::string &rtrim(std::string &s)
 #include <libtest/stream.h>
 #include <libtest/killpid.h>
 
-extern "C" {
-  static bool exited_successfully(int status, const std::string &command)
-  {
-    if (status == 0)
-    {
-      return true;
-    }
-
-    if (WIFEXITED(status) == true)
-    {
-      int ret= WEXITSTATUS(status);
-
-      if (ret == 0)
-      {
-        return true;
-      }
-      else if (ret == EXIT_FAILURE)
-      {
-        libtest::Error << "Command executed, but returned EXIT_FAILURE: " << command;
-      }
-      else
-      {
-        libtest::Error << "Command executed, but returned " << ret;
-      }
-    }
-    else if (WIFSIGNALED(status) == true)
-    {
-      int ret_signal= WTERMSIG(status);
-      libtest::Error << "Died from signal " << strsignal(ret_signal);
-    }
-
-    return false;
-  }
-}
-
-
 namespace libtest {
 
 std::ostream& operator<<(std::ostream& output, const Server &arg)
@@ -106,7 +70,6 @@ std::ostream& operator<<(std::ostream& output, const Server &arg)
     output << " Exec:" <<  arg.running();
   }
 
-
   return output;  // for multiple << operators
 }
 
@@ -153,21 +116,6 @@ bool Server::cycle()
   return true;
 }
 
-// Grab a one off command
-bool Server::command(std::string& command_arg)
-{
-  rebuild_base_command();
-
-  command_arg+= _base_command;
-
-  if (args(command_arg))
-  {
-    return true;
-  }
-
-  return false;
-}
-
 bool Server::wait_for_pidfile() const
 {
   Wait wait(pid_file(), 4);
@@ -183,25 +131,30 @@ bool Server::start()
     Error << "Could not kill() existing server during start() pid:" << _pid;
     return false;
   }
-  assert(not has_pid());
 
-  _running.clear();
-  if (command(_running) == false)
+  if (has_pid() == false)
+  {
+    fatal_message("has_pid() failed, programer error");
+  }
+
+  Application app(executable(), is_libtool());
+  if (args(app) == false)
   {
     Error << "Could not build command()";
     return false;
   }
 
-  if (is_valgrind() or is_helgrind())
+  Application::error_t ret;
+  if (Application::SUCCESS !=  (ret= app.run()))
   {
-    _running+= " &";
+    Error << "Application::run() " << ret;
+    return false;
   }
+  _running= app.print();
 
-  int ret= system(_running.c_str());
-  if (exited_successfully(ret, _running) == false)
+  if (Application::SUCCESS !=  (ret= app.wait()))
   {
-    Error << "system(" << _running << ") failed: " << strerror(errno);
-    _running.clear();
+    Error << "Application::wait() " << app.print() << " " << ret;
     return false;
   }
 
@@ -210,7 +163,7 @@ bool Server::start()
     dream(5, 50000);
   }
 
-  if (pid_file_option() and pid_file().empty() == false)
+  if (pid_file().empty() == false)
   {
     Wait wait(pid_file(), 8);
 
@@ -231,7 +184,7 @@ bool Server::start()
   if (pinged == false)
   {
     // If we happen to have a pid file, lets try to kill it
-    if (pid_file_option() and pid_file().empty() == false)
+    if (pid_file().empty() == false)
     {
       Error << "We are going to kill it off";
       kill_file(pid_file());
@@ -259,6 +212,16 @@ pid_t Server::pid()
   return _pid;
 }
 
+void Server::add_option(const std::string& arg)
+{
+  _options.push_back(std::make_pair(arg, std::string()));
+}
+
+void Server::add_option(const std::string& name, const std::string& value)
+{
+  _options.push_back(std::make_pair(name, value));
+}
+
 bool Server::set_socket_file()
 {
   char file_buffer[FILENAME_MAX];
@@ -334,106 +297,67 @@ bool Server::set_log_file()
   return true;
 }
 
-void Server::rebuild_base_command()
+bool Server::args(Application& app)
 {
-  _base_command.clear();
-  if (is_libtool())
-  {
-    _base_command+= libtool();
-    _base_command+= " --mode=execute ";
-  }
-
-  if (is_debug() and getenv("GDB_COMMAND"))
-  {
-    _base_command+= getenv("GDB_COMMAND");
-    _base_command+= " ";
-  }
-  else if (is_valgrind() and getenv("VALGRIND_COMMAND"))
-  {
-    _base_command+= getenv("VALGRIND_COMMAND");
-    _base_command+= " ";
-  }
-  else if (is_helgrind() and getenv("HELGRIND_COMMAND"))
-  {
-    _base_command+= getenv("HELGRIND_COMMAND");
-    _base_command+= " ";
-  }
-
-  if (is_libtool())
-  {
-    if (getenv("PWD"))
-    {
-      _base_command+= getenv("PWD");
-      _base_command+= "/";
-    }
-  }
-
-  _base_command+= executable();
-}
-
-void Server::set_extra_args(const std::string &arg)
-{
-  _extra_args= arg;
-}
-
-bool Server::args(std::string& options)
-{
-  std::stringstream arg_buffer;
 
   // Set a log file if it was requested (and we can)
-  if (getenv("LIBTEST_LOG") and log_file_option())
+  if (getenv("LIBTEST_LOG") and has_log_file_option())
   {
     if (not set_log_file())
     {
       return false;
     }
 
-    arg_buffer << " " << log_file_option() << _log_file;
+    log_file_option(app, _log_file);
   }
 
   if (getenv("LIBTEST_SYSLOG") and has_syslog())
   {
-    arg_buffer << " --syslog";
+    app.add_option("--syslog");
   }
 
   // Update pid_file
-  if (pid_file_option())
   {
     if (_pid_file.empty() and set_pid_file() == false)
     {
       return false;
     }
 
-    arg_buffer << " " << pid_file_option() << pid_file(); 
+    pid_file_option(app, pid_file());
   }
 
   assert(daemon_file_option());
   if (daemon_file_option() and not is_valgrind() and not is_helgrind())
   {
-    arg_buffer << " " << daemon_file_option();
+    app.add_option(daemon_file_option());
   }
 
-  if (_is_socket and socket_file_option())
+  if (has_socket_file_option())
   {
-    if (not set_socket_file())
+    Error << "adding socket option ";
+    if (set_socket_file() == false)
     {
       return false;
     }
 
-    arg_buffer << " " << socket_file_option() << "\"" <<  _socket << "\"";
+    socket_file_option(app, _socket);
   }
 
-  assert(port_option());
-  if (port_option() and _port > 0)
+  if (has_port_option())
   {
-    arg_buffer << " " << port_option() << _port;
+    port_option(app, _port);
   }
 
-  options+= arg_buffer.str();
-
-  if (not _extra_args.empty())
+  for (Options::const_iterator iter= _options.begin(); iter != _options.end(); iter++)
   {
-    options+= _extra_args;
+    if ((*iter).second.empty() == false)
+    {
+      app.add_option((*iter).first, (*iter).second);
+    }
+    else
+    {
+      app.add_option((*iter).first);
+    }
   }
 
   return true;

libtest/server.h

@@ -21,6 +21,8 @@
 
 #pragma once
 
+#include <libtest/cmdline.h>
+
 #include <cassert>
 #include <cstdio>
 #include <cstring>
@@ -33,6 +35,9 @@
 namespace libtest {
 
 struct Server {
+private:
+  typedef std::vector< std::pair<std::string, std::string> > Options;
+
 private:
   bool _is_socket;
   std::string _socket;
@@ -55,20 +60,67 @@ public:
 
   virtual const char *name()= 0;
   virtual const char *executable()= 0;
-  virtual const char *port_option()= 0;
-  virtual const char *pid_file_option()= 0;
   virtual const char *daemon_file_option()= 0;
-  virtual const char *log_file_option()= 0;
   virtual bool is_libtool()= 0;
 
-  virtual bool broken_socket_cleanup()
+  virtual bool has_socket_file_option() const
+  {
+    return false;
+  }
+
+  virtual void socket_file_option(Application& app, const std::string& socket_arg)
+  {
+    if (socket_arg.empty() == false)
+    {
+      std::string buffer("--socket=");
+      buffer+= socket_arg;
+      app.add_option(buffer);
+    }
+  }
+
+  bool has_log_file_option() const
+  {
+    return false;
+  }
+
+  virtual void log_file_option(Application& app, const std::string& arg)
+  {
+    if (arg.empty() == false)
+    {
+      std::string buffer("--log-file=");
+      buffer+= arg;
+      app.add_option(buffer);
+    }
+  }
+
+  virtual void pid_file_option(Application& app, const std::string& arg)
+  {
+    if (arg.empty() == false)
+    {
+      std::string buffer("--pid-file=");
+      buffer+= arg;
+      app.add_option(buffer);
+    }
+  }
+
+  virtual bool has_port_option() const
   {
     return false;
   }
 
-  virtual const char *socket_file_option() const
+  virtual void port_option(Application& app, in_port_t arg)
   {
-    return NULL;
+    if (arg > 0)
+    {
+      char buffer[1024];
+      snprintf(buffer, sizeof(buffer), "--port=%d", int(arg));
+      app.add_option(buffer);
+    }
+  }
+
+  virtual bool broken_socket_cleanup()
+  {
+    return false;
   }
 
   virtual bool broken_pid_file()
@@ -112,7 +164,10 @@ public:
 
   virtual pid_t get_pid(bool error_is_ok= false)= 0;
 
-  virtual bool build(int argc, const char *argv[])= 0;
+  virtual bool build(size_t argc, const char *argv[])= 0;
+
+  void add_option(const std::string&);
+  void add_option(const std::string&, const std::string&);
 
   in_port_t port() const
   {
@@ -137,9 +192,7 @@ public:
     _log_file.clear();
   }
 
-  void set_extra_args(const std::string &arg);
-
-  bool args(std::string& options);
+  bool args(Application&);
 
   pid_t pid();
 
@@ -174,10 +227,11 @@ public:
 
   bool kill(pid_t pid_arg);
   bool start();
-  bool command(std::string& command_arg);
+  bool command(libtest::Application& app);
 
 protected:
   bool set_pid_file();
+  Options _options;
 
 private:
   bool is_helgrind() const;
@@ -185,7 +239,6 @@ private:
   bool is_debug() const;
   bool set_log_file();
   bool set_socket_file();
-  void rebuild_base_command();
   void reset_pid();
 };
 

libtest/server_container.cc

@@ -144,7 +144,10 @@ bool server_startup_st::is_helgrind() const
 bool server_startup(server_startup_st& construct, const std::string& server_type, in_port_t try_port, int argc, const char *argv[])
 {
   Outn();
-  (void)try_port;
+  if (try_port <= 0)
+  {
+    libtest::fatal(LIBYATL_DEFAULT_PARAM, "was passed the invalid port number %d", int(try_port));
+  }
 
   libtest::Server *server= NULL;
   if (0)
@@ -216,7 +219,9 @@ bool server_startup(server_startup_st& construct, const std::string& server_type
     Out << "Pausing for startup, hit return when ready.";
     std::string gdb_command= server->base_command();
     std::string options;
+#if 0
     Out << "run " << server->args(options);
+#endif
     getchar();
   }
   else if (server->start() == false)
@@ -313,7 +318,9 @@ bool server_startup_st::start_socket_server(const std::string& server_type, cons
     Out << "Pausing for startup, hit return when ready.";
     std::string gdb_command= server->base_command();
     std::string options;
+#if 0
     Out << "run " << server->args(options);
+#endif
     getchar();
   }
   else if (not server->start())

libtest/unittest.cc

@@ -236,8 +236,7 @@ static test_return_t gearmand_cycle_test(void *object)
   test_skip(true, has_gearmand_binary());
 #endif
 
-  const char *argv[1]= { "cycle_gearmand" };
-  test_true(server_startup(*servers, "gearmand", get_free_port(), 1, argv));
+  test_true(server_startup(*servers, "gearmand", get_free_port(), 0, NULL));
 
   return TEST_SUCCESS;
 }
@@ -250,8 +249,7 @@ static test_return_t memcached_cycle_test(void *object)
   if (MEMCACHED_BINARY and HAVE_LIBMEMCACHED) 
   {
     test_true(has_memcached_binary());
-    const char *argv[1]= { "cycle_memcached" };
-    test_true(server_startup(*servers, "memcached", get_free_port(), 1, argv));
+    test_true(server_startup(*servers, "memcached", get_free_port(), 0, NULL));
 
     return TEST_SUCCESS;
   }
@@ -269,8 +267,7 @@ static test_return_t memcached_socket_cycle_test(void *object)
     if (HAVE_LIBMEMCACHED)
     {
       test_true(has_memcached_binary());
-      const char *argv[1]= { "cycle_memcached" };
-      test_true(servers->start_socket_server("memcached", get_free_port(), 1, argv));
+      test_true(servers->start_socket_server("memcached", get_free_port(), 0, NULL));
 
       return TEST_SUCCESS;
     }
@@ -294,8 +291,7 @@ static test_return_t memcached_sasl_test(void *object)
     if (HAVE_LIBMEMCACHED)
     {
       test_true(has_memcached_sasl_binary());
-      const char *argv[1]= { "cycle_memcached_sasl" };
-      test_true(server_startup(*servers, "memcached-sasl", get_free_port(), 1, argv));
+      test_true(server_startup(*servers, "memcached-sasl", get_free_port(), 0, NULL));
 
       return TEST_SUCCESS;
     }