Update libtest

libtest/cmdline.cc

@@ -26,6 +26,7 @@ using namespace libtest;
 
 #include <cstdlib>
 #include <cstring>
+#include <cerrno>
 #include <fcntl.h>
 #include <fstream>
 #include <memory>
@@ -352,26 +353,24 @@ void Application::Pipe::reset()
   close(WRITE);
 
   int ret;
-  if ((ret= pipe(_fd)) < 0)
+  if (pipe(_fd) == -1)
   {
-    throw strerror(ret);
+    throw strerror(errno);
   }
   _open[0]= true;
   _open[1]= true;
 
   {
-    ret= fcntl(_fd[0], F_GETFL, 0);
-    if (ret == -1)
+    if ((ret= fcntl(_fd[0], F_GETFL, 0)) == -1)
     {
-      Error << "fcntl(F_GETFL) " << strerror(ret);
-      throw strerror(ret);
+      Error << "fcntl(F_GETFL) " << strerror(errno);
+      throw strerror(errno);
     }
 
-    ret= fcntl(_fd[0], F_SETFL, ret | O_NONBLOCK);
-    if (ret == -1)
+    if ((ret= fcntl(_fd[0], F_SETFL, ret | O_NONBLOCK)) == -1)
     {
-      Error << "fcntl(F_SETFL) " << strerror(ret);
-      throw strerror(ret);
+      Error << "fcntl(F_SETFL) " << strerror(errno);
+      throw strerror(errno);
     }
   }
 }
@@ -407,11 +406,12 @@ void Application::Pipe::close(const close_t& arg)
   if (_open[type])
   {
     int ret;
-    if ((ret= ::close(_fd[type])) < 0)
+    if (::close(_fd[type]) == -1)
     {
-      Error << "close(" << strerror(ret) << ")";
+      Error << "close(" << strerror(errno) << ")";
     }
     _open[type]= false;
+    _fd[type]= -1;
   }
 }
 
@@ -524,21 +524,19 @@ std::string Application::arguments()
 
 void Application::delete_argv()
 {
-  if (built_argv == NULL)
-  {
-    return;
-  }
-
-  for (size_t x= 0; x < _argc; x++)
+  if (built_argv)
   {
-    if (built_argv[x])
+    for (size_t x= 0; x < _argc; x++)
     {
-      ::free(built_argv[x]);
+      if (built_argv[x])
+      {
+        ::free(built_argv[x]);
+      }
     }
+    delete[] built_argv;
+    built_argv= NULL;
+    _argc= 0;
   }
-  delete[] built_argv;
-  built_argv= NULL;
-  _argc= 0;
 }
 
 

libtest/comparison.hpp

@@ -34,6 +34,8 @@
 
 namespace libtest {
 
+bool _in_valgrind(const char *file, int line, const char *func);
+
 template <class T_comparable, class T_hint>
 bool _compare_truth_hint(const char *file, int line, const char *func, T_comparable __expected, const char *assertation_label,  T_hint __hint)
 {
@@ -47,11 +49,15 @@ bool _compare_truth_hint(const char *file, int line, const char *func, T_compara
 }
 
 template <class T1_comparable, class T2_comparable>
-bool _compare(const char *file, int line, const char *func, const T1_comparable& __expected, const T2_comparable& __actual)
+bool _compare(const char *file, int line, const char *func, const T1_comparable& __expected, const T2_comparable& __actual, bool use_io)
 {
   if (__expected != __actual)
   {
-    libtest::stream::make_cerr(file, line, func) << "Expected \"" << __expected << "\" got \"" << __actual << "\"";
+    if (use_io)
+    {
+      libtest::stream::make_cerr(file, line, func) << "Expected \"" << __expected << "\" got \"" << __actual << "\"";
+    }
+
     return false;
   }
 
@@ -83,11 +89,14 @@ bool _truth(const char *file, int line, const char *func, T_comparable __truth)
 }
 
 template <class T1_comparable, class T2_comparable, class T_hint>
-bool _compare_hint(const char *file, int line, const char *func, T1_comparable __expected, T2_comparable __actual, T_hint __hint)
+bool _compare_hint(const char *file, int line, const char *func, T1_comparable __expected, T2_comparable __actual, T_hint __hint, bool io_error= true)
 {
   if (__expected != __actual)
   {
-    libtest::stream::make_cerr(file, line, func) << "Expected \"" << __expected << "\" got \"" << __actual << "\"" << " Additionally: \"" << __hint << "\"";
+    if (io_error)
+    {
+      libtest::stream::make_cerr(file, line, func) << "Expected \"" << __expected << "\" got \"" << __actual << "\"" << " Additionally: \"" << __hint << "\"";
+    }
 
     return false;
   }

libtest/include.am

@@ -21,7 +21,8 @@
 # 
 
 LIBTOOL_COMMAND= ${abs_top_builddir}/libtool --mode=execute
-VALGRIND_COMMAND= $(LIBTOOL_COMMAND) valgrind --error-exitcode=1 --leak-check=yes --show-reachable=yes --track-fds=yes --malloc-fill=A5 --free-fill=DE
+VALGRIND_EXEC_COMMAND= $(LIBTOOL_COMMAND) valgrind --error-exitcode=1 --leak-check=yes --show-reachable=yes --track-fds=yes --malloc-fill=A5 --free-fill=DE
+VALGRIND_COMMAND= TESTS_ENVIRONMENT="valgrind" $(VALGRIND_EXEC_COMMAND)
 HELGRIND_COMMAND= $(LIBTOOL_COMMAND) valgrind --tool=helgrind --read-var-info=yes --error-exitcode=1 --read-var-info=yes
 DRD_COMMAND= $(LIBTOOL_COMMAND) valgrind --tool=drd
 GDB_COMMAND= $(LIBTOOL_COMMAND) gdb -f -x libtest/run.gdb
@@ -33,7 +34,7 @@ export DRD_COMMAND
 export GDB_COMMAND
 
 valgrind:
-	@echo make check TESTS_ENVIRONMENT="\"$(VALGRIND_COMMAND)\""
+	@echo make check TESTS_ENVIRONMENT="\"$(VALGRIND_EXEC_COMMAND)\""
 
 gdb:
 	@echo make check TESTS_ENVIRONMENT="\"$(GDB_COMMAND)\""
@@ -100,6 +101,7 @@ noinst_LTLIBRARIES+= libtest/libtest.la
 libtest_libtest_la_SOURCES= \
 			    libtest/binaries.cc \
 			    libtest/cmdline.cc \
+			    libtest/comparison.cc \
 			    libtest/core.cc \
 			    libtest/cpu.cc \
 			    libtest/dream.cc \
@@ -210,7 +212,7 @@ test-unittest: libtest/unittest
 	@libtest/unittest
 
 valgrind-unittest: libtest/unittest
-	@$(VALGRIND_COMMAND) libtest/unittest
+	@$(VALGRIND_COMMAND) libtest/unittest TESTS_ENVIRONMENT="valgrind"
 
 gdb-unittest: libtest/unittest
 	@$(GDB_COMMAND) libtest/unittest

libtest/memcached.cc

@@ -88,7 +88,8 @@ public:
     // Memcached is slow to start, so we need to do this
     if (pid_file().empty() == false)
     {
-      if (error_is_ok and not wait_for_pidfile())
+      if (error_is_ok and
+          wait_for_pidfile() == false)
       {
         Error << "Pidfile was not found:" << pid_file();
         return -1;
@@ -238,7 +239,7 @@ public:
     // Memcached is slow to start, so we need to do this
     if (pid_file().empty() == false)
     {
-      if (error_is_ok and not wait_for_pidfile())
+      if (error_is_ok and wait_for_pidfile() == false)
       {
         Error << "Pidfile was not found:" << pid_file();
         return -1;
@@ -369,9 +370,10 @@ public:
   pid_t get_pid(bool error_is_ok)
   {
     // Memcached is slow to start, so we need to do this
-    if (not pid_file().empty())
+    if (pid_file().empty() == false)
     {
-      if (error_is_ok and not wait_for_pidfile())
+      if (error_is_ok and 
+          wait_for_pidfile() == false)
       {
         Error << "Pidfile was not found:" << pid_file();
         return -1;

libtest/server.cc

@@ -66,7 +66,7 @@ std::ostream& operator<<(std::ostream& output, const Server &arg)
     output << " Socket:" <<  arg.socket();
   }
 
-  if (not arg.running().empty())
+  if (arg.running().empty() == false)
   {
     output << " Exec:" <<  arg.running();
   }
@@ -74,7 +74,10 @@ std::ostream& operator<<(std::ostream& output, const Server &arg)
   return output;  // for multiple << operators
 }
 
+#define MAGIC_MEMORY 123570
+
 Server::Server(const std::string& host_arg, const in_port_t port_arg, bool is_socket_arg) :
+  _magic(MAGIC_MEMORY),
   _is_socket(is_socket_arg),
   _pid(-1),
   _port(port_arg),
@@ -90,6 +93,11 @@ Server::~Server()
   }
 }
 
+bool Server::validate()
+{
+  return _magic == MAGIC_MEMORY;
+}
+
 // If the server exists, kill it
 bool Server::cycle()
 {
@@ -97,7 +105,8 @@ bool Server::cycle()
 
   // Try to ping, and kill the server #limit number of times
   pid_t current_pid;
-  while (--limit and is_pid_valid(current_pid= get_pid()))
+  while (--limit and 
+         is_pid_valid(current_pid= get_pid()))
   {
     if (kill(current_pid))
     {
@@ -168,7 +177,7 @@ bool Server::start()
 
   if (Application::SUCCESS !=  (ret= app.wait()))
   {
-    Error << "Application::wait() " << app.print() << " " << ret;
+    Error << "Application::wait() " << _running << " " << ret;
     return false;
   }
 

libtest/server.h

@@ -39,6 +39,7 @@ private:
   typedef std::vector< std::pair<std::string, std::string> > Options;
 
 private:
+  uint64_t _magic;
   bool _is_socket;
   std::string _socket;
   std::string _sasl;
@@ -229,6 +230,8 @@ public:
   bool start();
   bool command(libtest::Application& app);
 
+  bool validate();
+
 protected:
   bool set_pid_file();
   Options _options;

libtest/server_container.cc

@@ -121,11 +121,26 @@ void server_startup_st::restart()
   }
 }
 
+#define MAGIC_MEMORY 123575
+server_startup_st::server_startup_st() :
+  _magic(MAGIC_MEMORY),
+  _socket(false),
+  _sasl(false),
+  _count(5),
+  udp(0)
+{ }
+
 server_startup_st::~server_startup_st()
 {
   shutdown_and_remove();
 }
 
+bool server_startup_st::validate()
+{
+  return _magic == MAGIC_MEMORY;
+}
+
+
 bool server_startup_st::is_debug() const
 {
   return bool(getenv("LIBTEST_MANUAL_GDB"));

libtest/server_container.h

@@ -35,6 +35,7 @@ namespace libtest {
 class server_startup_st
 {
 private:
+  uint64_t _magic;
   std::string server_list;
   bool _socket;
   bool _sasl;
@@ -47,12 +48,10 @@ public:
   uint8_t udp;
   std::vector<Server *> servers;
 
-  server_startup_st() :
-    _socket(false),
-    _sasl(false),
-    _count(5),
-    udp(0)
-  { }
+  server_startup_st();
+  ~server_startup_st();
+
+  bool validate();
 
   bool start_socket_server(const std::string& server_type, const in_port_t try_port, int argc, const char *argv[]);
 
@@ -114,8 +113,6 @@ public:
 
   void push_server(Server *);
   Server *pop_server();
-
-  ~server_startup_st();
 };
 
 bool server_startup(server_startup_st&, const std::string&, in_port_t try_port, int argc, const char *argv[]);

libtest/test.h

@@ -89,10 +89,19 @@ do \
 } while (0)
 #define test_true_hint test_true_got
 
-#define test_skip(A,B) \
+#define test_skip(__expected, __actual) \
 do \
 { \
-  if ((A) != (B)) \
+  if (libtest::_compare(__FILE__, __LINE__, __func__, ((__expected)), ((__actual)), false) == false) \
+  { \
+    return TEST_SKIPPED; \
+  } \
+} while (0)
+
+#define test_skip_valgrind() \
+do \
+{ \
+  if (libtest::_in_valgrind(__FILE__, __LINE__, __func__)) \
   { \
     return TEST_SKIPPED; \
   } \
@@ -132,7 +141,7 @@ do \
 #define test_compare(__expected, __actual) \
 do \
 { \
-  if (not libtest::_compare(__FILE__, __LINE__, __func__, ((__expected)), ((__actual)))) \
+  if (libtest::_compare(__FILE__, __LINE__, __func__, ((__expected)), ((__actual)), true) == false) \
   { \
     libtest::create_core(); \
     return TEST_FAILURE; \
@@ -166,7 +175,7 @@ do \
 #define test_compare_warn(__expected, __actual) \
 do \
 { \
-  void(libtest::_compare(__FILE__, __LINE__, __func__, (__expected), (__actual))); \
+  void(libtest::_compare(__FILE__, __LINE__, __func__, (__expected), (__actual)), true); \
 } while (0)
 
 #define test_compare_warn_hint(__expected, __actual, __hint) \

libtest/unittest.cc

@@ -63,7 +63,7 @@ static test_return_t GDB_COMMAND_test(void *)
 
 static test_return_t test_success_equals_one_test(void *)
 {
-  test_skip(HAVE_LIBMEMCACHED, true);
+  test_skip(HAVE_LIBMEMCACHED, 1);
 #if defined(HAVE_LIBMEMCACHED) && HAVE_LIBMEMCACHED 
   test_zero(MEMCACHED_SUCCESS);
 #endif
@@ -110,7 +110,7 @@ static test_return_t local_not_test(void *)
   }
 
   // unsetenv() will cause issues with valgrind
-  _compare(__FILE__, __LINE__, __func__, 0, unsetenv("LIBTEST_LOCAL"));
+  _compare(__FILE__, __LINE__, __func__, 0, unsetenv("LIBTEST_LOCAL"), true);
   test_compare(0, unsetenv("LIBTEST_LOCAL"));
   test_false(test_is_local());
 
@@ -228,14 +228,14 @@ static test_return_t _compare_gearman_return_t_test(void *)
 static test_return_t gearmand_cycle_test(void *object)
 {
   server_startup_st *servers= (server_startup_st*)object;
-  test_true(servers);
+  test_true(servers and servers->validate());
 
 #if defined(HAVE_GEARMAND_BINARY) && HAVE_GEARMAND_BINARY
   test_true(has_gearmand_binary());
-#else
-  test_skip(true, has_gearmand_binary());
 #endif
 
+  test_skip(true, has_gearmand_binary());
+
   test_true(server_startup(*servers, "gearmand", get_free_port(), 0, NULL));
 
   return TEST_SUCCESS;
@@ -253,6 +253,29 @@ static test_return_t memcached_light_cycle_TEST(void *object)
   return TEST_SUCCESS;
 }
 
+static test_return_t skip_shim(bool a, bool b)
+{
+  test_skip(a, b);
+  return TEST_SUCCESS;
+}
+
+static test_return_t test_skip_true_TEST(void *object)
+{
+  test_compare(true, true);
+  test_compare(false, false);
+  test_compare(TEST_SUCCESS, skip_shim(true, true));
+  test_compare(TEST_SUCCESS, skip_shim(false, false));
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t test_skip_false_TEST(void *object)
+{
+  test_compare(TEST_SKIPPED, skip_shim(true, false));
+  test_compare(TEST_SKIPPED, skip_shim(false, true));
+  return TEST_SUCCESS;
+}
+
 static test_return_t memcached_cycle_test(void *object)
 {
   server_startup_st *servers= (server_startup_st*)object;
@@ -293,10 +316,7 @@ static test_return_t memcached_sasl_test(void *object)
   server_startup_st *servers= (server_startup_st*)object;
   test_true(servers);
 
-  if (getenv("TESTS_ENVIRONMENT"))
-  {
-    return TEST_SKIPPED;
-  }
+  test_skip(false, bool(getenv("TESTS_ENVIRONMENT")));
 
   if (MEMCACHED_SASL_BINARY)
   {
@@ -361,27 +381,19 @@ static test_return_t application_true_fubar_BINARY(void *)
 
 static test_return_t application_doesnotexist_BINARY(void *)
 {
+  test_skip_valgrind();
+
   Application true_app("doesnotexist");
 
   const char *args[]= { "--fubar", 0 };
 #if defined(TARGET_OS_OSX) && TARGET_OS_OSX
   test_compare(Application::INVALID, true_app.run(args));
+  test_compare(Application::FAILURE, true_app.wait());
 #else
   test_compare(Application::SUCCESS, true_app.run(args));
+  test_compare(Application::INVALID, true_app.wait());
 #endif
-  // Behavior is different if we are running under valgrind
-  if (getenv("TESTS_ENVIRONMENT") and strstr(getenv("TESTS_ENVIRONMENT"), "valgrind"))
-  {
-    test_compare(Application::FAILURE, true_app.wait());
-  }
-  else
-  {
-#if defined(TARGET_OS_OSX) && TARGET_OS_OSX
-    test_compare(Application::FAILURE, true_app.wait());
-#else
-    test_compare(Application::INVALID, true_app.wait());
-#endif
-  }
+
   test_compare(0, true_app.stdout_result().size());
 
   return TEST_SUCCESS;
@@ -706,6 +718,12 @@ test_st memcached_tests[] ={
   {0, 0, 0}
 };
 
+test_st test_skip_TESTS[] ={
+  {"true, true", 0, test_skip_true_TEST },
+  {"true, false", 0, test_skip_false_TEST },
+  {0, 0, 0}
+};
+
 test_st environment_tests[] ={
   {"LIBTOOL_COMMAND", 0, LIBTOOL_COMMAND_test },
   {"VALGRIND_COMMAND", 0, VALGRIND_COMMAND_test },
@@ -825,6 +843,7 @@ test_st http_tests[] ={
 collection_st collection[] ={
   {"environment", 0, 0, environment_tests},
   {"return values", 0, 0, tests_log},
+  {"test_skip()", 0, 0, test_skip_TESTS },
   {"local", 0, 0, local_log},
   {"directories", 0, 0, directories_tests},
   {"comparison", 0, 0, comparison_tests},