[PATCH xorg-gtest 11/11] test: add xserver test for logfile removal

[Peter Hutterer]

On Thu, Aug 16, 2012 at 08:47:15AM -0700, Chase Douglas wrote:
> On 08/15/2012 11:36 PM, Peter Hutterer wrote:
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  test/Makefile.am      |  6 ++++-
> >  test/xserver-test.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 67 insertions(+), 1 deletion(-)
> >  create mode 100644 test/xserver-test.cpp
> >
> >diff --git a/test/Makefile.am b/test/Makefile.am
> >index 44c1027..ed6416a 100644
> >--- a/test/Makefile.am
> >+++ b/test/Makefile.am
> >@@ -25,7 +25,7 @@
> >  # SOFTWARE.
> >  #
> >
> >-noinst_PROGRAMS = process-test
> >+noinst_PROGRAMS = process-test xserver-test
> >
> >  AM_CPPFLAGS = $(GTEST_CPPFLAGS)
> >  AM_CXXFLAGS = $(BASE_CXXFLAGS)
> >@@ -41,6 +41,10 @@ process_test_SOURCES = process-test.cpp
> >  process_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
> >  process_test_LDADD =  $(tests_libraries)
> >
> >+xserver_test_SOURCES = xserver-test.cpp
> >+xserver_test_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/include
> >+xserver_test_LDADD =  $(tests_libraries)
> >+
> >  check_LIBRARIES = libgtest.a libxorg-gtest.a
> >
> >  # build googletest as static lib
> >diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp
> >new file mode 100644
> >index 0000000..ba6c462
> >--- /dev/null
> >+++ b/test/xserver-test.cpp
> >@@ -0,0 +1,62 @@
> >+#include <errno.h>
> >+#include <unistd.h>
> >+#include <sys/types.h>
> >+#include <sys/wait.h>
> >+#include <fstream>
> >+
> >+#include <gtest/gtest.h>
> >+#include <xorg/gtest/xorg-gtest.h>
> >+
> >+using namespace xorg::testing;
> >+
> >+TEST(XServer, LogRemoval)
> >+{
> >+  SCOPED_TRACE("TESTCASE: X server startup and log file removal on success and error");
> >+  std::string logfile = "/tmp/xorg-testing-xserver_____________.log";
> >+
> >+  /* make sure a previous failed test didn't leave it around */
> >+  unlink(logfile.c_str());
> >+
> >+  XServer server;
> >+  server.SetOption("-logfile", logfile);
> >+  server.Start();
> >+  server.Terminate(3000);
> >+  server.RemoveLogFile();
> >+
> >+  std::ifstream file(logfile.c_str());
> >+  ASSERT_FALSE(file.good());
> >+  file.close();
> >+
> >+  server.SetOption("-doesnotexist", "");
> >+  server.Start();
> >+  while (server.GetState() == Process::RUNNING)
> >+    usleep(5000);
> >+
> >+  ASSERT_EQ(server.GetState(), Process::FINISHED_FAILURE);
> >+  file.open(logfile.c_str());
> >+  ASSERT_FALSE(file.good()); /* server didn't leave the file behind */
> >+
> >+  /* now create it */
> >+  std::ofstream f(logfile.c_str());
> >+  file.open(logfile.c_str());
> >+  ASSERT_TRUE(file.good());
> >+  file.close();
> >+
> >+  /* must not remove it now */
> >+  server.RemoveLogFile();
> >+
> >+  file.open(logfile.c_str());
> >+  ASSERT_TRUE(file.good()); /* server didn't remove it */
> >+  file.close();
> >+
> >+  server.RemoveLogFile(true);
> >+  file.open(logfile.c_str());
> >+  ASSERT_FALSE(file.good()); /* server did remove it */
> >+  file.close();
> >+}
> >+
> >+
> >+int main(int argc, char *argv[]) {
> >+  testing::InitGoogleTest(&argc, argv);
> >+  return RUN_ALL_TESTS();
> >+}
> 
> I don't see why these tests shouldn't be compiled and linked into
> one binary with the other tests in process-test.cpp. I think it will
> make the tests easier to maintain over time. The simple way to do
> this would be:

The XServer test requires a working installation of X, the process test
doesn't and ideally that stays that way. Playing with gtest_filter is fun
but not that nice as just running specific binaries. I'd prefer to leave it
as-is until we have more tests and actually see a direction, right now it's
more of a bucket to throw tests into. 

Cheers,
   Peter

> * Remove the main() functions from each file
> * Compile gtest_main.cc
> * Link each test object file and gtest_main together
> 
> There's nothing wrong with this approach, so I'll give my r-b, but
> the above approach would generate faster test runs and more easily
> analyzed test results in case you want to feed them into something
> like jenkins.
> 
> Reviewed-by: Chase Douglas <chase.douglas at canonical.com>