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

[Chase Douglas]

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:

* 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>