From: Brian Aker Date: Sun, 26 Feb 2012 18:30:36 +0000 (-0800) Subject: Additions to testing to better check return values/etc for servers. X-Git-Tag: 1.0.5~26 X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=ad5d3efae1a492151ab2b08d370d86d27931c3a9;p=m6w6%2Flibmemcached Additions to testing to better check return values/etc for servers. --- diff --git a/example/t/memcached_light.cc b/example/t/memcached_light.cc index 032d6f90..ae996518 100644 --- a/example/t/memcached_light.cc +++ b/example/t/memcached_light.cc @@ -65,6 +65,8 @@ static test_return_t help_TEST(void *) static test_return_t basic_TEST(void *) { + test_skip(false, true); + char port_buffer[1024]; snprintf(port_buffer, sizeof(port_buffer), "--port=%d", int(libtest::default_port())); const char *memcached_light_args[]= { port_buffer, 0 }; diff --git a/libtest/blobslap_worker.cc b/libtest/blobslap_worker.cc index 8bbd0738..d8684058 100644 --- a/libtest/blobslap_worker.cc +++ b/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() + void 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() @@ -159,11 +154,9 @@ bool BlobslapWorker::build(int argc, const char *argv[]) for (int x= 1 ; x < argc ; x++) { - arg_buffer << " " << argv[x] << " "; + add_option(argv[x]); } - set_extra_args(arg_buffer.str()); - return true; } diff --git a/libtest/cmdline.cc b/libtest/cmdline.cc index 6ef40784..aae91110 100644 --- a/libtest/cmdline.cc +++ b/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 { @@ -218,16 +233,15 @@ Application::error_t Application::wait() { throw libtest::fatal(LIBYATL_DEFAULT_PARAM, "Pid mismatch, %d != %d", int(waited_pid), int(_pid)); } - exit_code= error_t(exited_successfully(status)); + 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; } diff --git a/libtest/cmdline.h b/libtest/cmdline.h index d1019d54..39ba35a2 100644 --- a/libtest/cmdline.h +++ b/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; diff --git a/libtest/gearmand.cc b/libtest/gearmand.cc index bcb7f8aa..caa75f65 100644 --- a/libtest/gearmand.cc +++ b/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,6 +195,11 @@ public: return true; } + void has_port_option() const + { + return true; + } + bool build(int argc, const char *argv[]); }; @@ -203,18 +209,16 @@ bool Gearmand::build(int argc, const char *argv[]) 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++) { - arg_buffer << " " << argv[x] << " "; + add_option(argv[x]); } - set_extra_args(arg_buffer.str()); - return true; } diff --git a/libtest/memcached.cc b/libtest/memcached.cc index 7d25153f..7e396c30 100644 --- a/libtest/memcached.cc +++ b/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 true; + } + + 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() @@ -300,25 +318,23 @@ bool Memcached::build(int argc, const char *argv[]) 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; } diff --git a/libtest/server.cc b/libtest/server.cc index 9d492b9b..0fd1bbf3 100644 --- a/libtest/server.cc +++ b/libtest/server.cc @@ -42,42 +42,6 @@ static inline std::string &rtrim(std::string &s) #include #include -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) @@ -153,21 +117,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); @@ -185,23 +134,22 @@ bool Server::start() } assert(not has_pid()); - _running.clear(); - if (command(_running) == false) + Application app(name(), is_libtool()); + if (args(app) == false) { Error << "Could not build command()"; return false; } - if (is_valgrind() or is_helgrind()) + if (Application::SUCCESS != app.run()) { - _running+= " &"; + Error << "Application::run()"; + return false; } - int ret= system(_running.c_str()); - if (exited_successfully(ret, _running) == false) + if (Application::SUCCESS != app.wait()) { - Error << "system(" << _running << ") failed: " << strerror(errno); - _running.clear(); + Error << "Application::wait()"; return false; } @@ -210,7 +158,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 +179,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 +207,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 +292,66 @@ 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 (_is_socket and has_socket_file_option()) { if (not set_socket_file()) { 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; diff --git a/libtest/server.h b/libtest/server.h index b50b0b2e..9c8f048b 100644 --- a/libtest/server.h +++ b/libtest/server.h @@ -21,6 +21,8 @@ #pragma once +#include + #include #include #include @@ -33,6 +35,9 @@ namespace libtest { struct Server { +private: + typedef std::vector< std::pair > 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("--socket="); + 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() @@ -114,6 +166,9 @@ public: virtual bool build(int 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 { return _port; @@ -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(); }; diff --git a/libtest/server_container.cc b/libtest/server_container.cc index 528fcd1f..3c84ad85 100644 --- a/libtest/server_container.cc +++ b/libtest/server_container.cc @@ -216,7 +216,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 +315,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())