[PATCH xorg-gtest 4/4] xserver: add XORG_GTEST_XSERVER_KEEPALIVE environment variable

[Peter Hutterer]

On Fri, Oct 26, 2012 at 09:22:45AM -0700, Chase Douglas wrote:
> On Thu, Oct 25, 2012 at 4:11 PM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > If set, XServer will ignore Terminate() and Kill() requests, and will not
> > die if the parent process dies. This enables a user to hook up gdb to a
> > server, wait for the test case to trigger some code in the server and
> > continue debugging from there, without the test case terminating the server.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  README                |  4 +++
> >  src/xserver.cpp       | 11 +++++++++
> >  test/xserver-test.cpp | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 82 insertions(+)
> >
> > diff --git a/README b/README
> > index 6c660b0..53475db 100644
> > --- a/README
> > +++ b/README
> > @@ -78,6 +78,10 @@ Environment variables
> >  ---------------------
> >  XORG_GTEST_XSERVER_SIGSTOP
> >    If set, an XServer object will raise a SIGSTOP signal after startup.
> > +XORG_GTEST_XSERVER_KEEPALIVE
> > +  If set, the XServer object will ignore calls to Terminate() or Kill().
> > +  This is useful for debugging a server without having the test terminate
> > +  the process while still looking at gdb output.
> >  XORG_GTEST_CHILD_STDOUT
> >    If set to any value, Process::Start() will _not_ close stdout/stdin/stderr
> >    for the forked child.
> > diff --git a/src/xserver.cpp b/src/xserver.cpp
> > index 9f91e7d..c7117f3 100644
> > --- a/src/xserver.cpp
> > +++ b/src/xserver.cpp
> > @@ -433,6 +433,11 @@ void xorg::testing::XServer::Start(const std::string &program) {
> >
> >    pid_t pid = Fork();
> >    if (pid == 0) {
> > +#ifdef __linux
> > +    if (getenv("XORG_GTEST_XSERVER_KEEPALIVE") != NULL)
> 
> I noticed that you use differing conventions here vs below. Here you
> check if the value != NULL, below you just check (value). They are
> obviously the same, just thought maybe you would want to clean it up
> before pushing. The reason I noticed was it made me wonder if you
> meant "== NULL" here, but I think it is correct as is.

Changed to be consistent with the others (i.e. drop the NULL comparison
altogether). Thanks.

Cheers,
   Peter

> 
> > +      prctl(PR_SET_PDEATHSIG, 0);
> > +#endif
> > +
> >      /* set SIGUSR1 handler to SIG_IGN, XServer tests for this and will
> >       * send SIGUSR1 when ready */
> >      sighandler_t old_handler;
> > @@ -490,6 +495,9 @@ void xorg::testing::XServer::Start(const std::string &program) {
> >  }
> >
> >  bool xorg::testing::XServer::Terminate(unsigned int timeout) {
> > +  if (getenv("XORG_GTEST_XSERVER_KEEPALIVE"))
> > +    return true;
> > +
> >    if (!Process::Terminate(timeout)) {
> >      std::cerr << "Warning: Failed to terminate Xorg server: "
> >                << std::strerror(errno) << "\n";
> > @@ -499,6 +507,9 @@ bool xorg::testing::XServer::Terminate(unsigned int timeout) {
> >  }
> >
> >  bool xorg::testing::XServer::Kill(unsigned int timeout) {
> > +  if (getenv("XORG_GTEST_XSERVER_KEEPALIVE"))
> > +    return true;
> > +
> >    if (!Process::Kill(timeout)) {
> >      std::cerr << "Warning: Failed to kill Xorg server: "
> >                << std::strerror(errno) << "\n";
> > diff --git a/test/xserver-test.cpp b/test/xserver-test.cpp
> > index 4088baa..ccbc2e9 100644
> > --- a/test/xserver-test.cpp
> > +++ b/test/xserver-test.cpp
> > @@ -207,6 +207,73 @@ TEST(XServer, IOErrorException)
> >    }, XIOError);
> >  }
> >
> > +TEST(XServer, KeepAlive)
> > +{
> > +  XORG_TESTCASE("If XORG_GTEST_XSERVER_KEEPALIVE is set,\n"
> > +                "XServer::Terminate() and XServer::Kill() have no "
> > +                "effect");
> > +
> > +  int pipefd[2];
> > +  ASSERT_NE(pipe(pipefd), -1);
> > +
> > +  if (fork() == 0) {
> > +    close(pipefd[0]);
> > +
> > +    ASSERT_EQ(setenv("XORG_GTEST_XSERVER_KEEPALIVE", "1", 1), 0);
> > +    ASSERT_TRUE(getenv("XORG_GTEST_XSERVER_KEEPALIVE") != NULL);
> > +
> > +    XServer server;
> > +    server.SetOption("-logfile", "/tmp/Xorg-keepalive.log");
> > +    server.SetOption("-noreset", "");
> > +    server.Start();
> > +    ASSERT_EQ(server.GetState(), Process::RUNNING);
> > +    ::Display *dpy = XOpenDisplay(server.GetDisplayString().c_str());
> > +    ASSERT_TRUE(dpy != NULL);
> > +
> > +    server.Terminate();
> > +    ASSERT_EQ(server.GetState(), Process::RUNNING);
> > +    server.Kill();
> > +    ASSERT_EQ(server.GetState(), Process::RUNNING);
> > +
> > +    char *buffer;
> > +    ASSERT_GT(asprintf(&buffer, "%d", server.Pid()), 0);
> > +    ASSERT_EQ(write(pipefd[1], buffer, strlen(buffer)), (int)strlen(buffer));
> > +    close(pipefd[1]);
> > +    free(buffer);
> > +    return;
> > +  }
> > +
> > +  sigset_t sig_mask;
> > +  sigemptyset(&sig_mask);
> > +  sigaddset(&sig_mask, SIGCHLD);
> > +  struct timespec tv = { 1, 0 };
> > +  sigprocmask(SIG_BLOCK, &sig_mask, NULL);
> > +
> > +  /* parent */
> > +  close(pipefd[1]);
> > +
> > +  char buffer[20] = {0};
> > +  ASSERT_GT(read(pipefd[0], buffer, sizeof(buffer)), 0);
> > +  close(pipefd[0]);
> > +
> > +  /* wait for forked child to die */
> > +  ASSERT_EQ(sigtimedwait(&sig_mask, NULL, &tv), SIGCHLD);
> > +
> > +  pid_t server_pid = atoi(buffer);
> > +
> > +  /* server must still be running, kill it */
> > +  ASSERT_EQ(kill(server_pid, 0), 0);
> > +  kill(server_pid, SIGTERM);
> > +
> > +  int i = 0;
> > +
> > +  while(kill(server_pid, 0) == 0 && i++ < 10)
> > +    usleep(50000);
> > +
> > +  ASSERT_EQ(kill(server_pid, 0), -1);
> > +  ASSERT_EQ(errno, ESRCH);
> > +}
> > +
> >  int main(int argc, char *argv[]) {
> >    testing::InitGoogleTest(&argc, argv);
> >    return RUN_ALL_TESTS();