[PATCH xorg-gtest 05/16] xserver: move testing startup to the XServer object

[Peter Hutterer]

On Thu, Jul 05, 2012 at 01:46:42PM -0700, Chase Douglas wrote:
> On 07/03/2012 10:34 PM, Peter Hutterer wrote:
> >On Tue, Jul 03, 2012 at 10:30:28AM -0700, Chase Douglas wrote:
> >>On 07/02/2012 11:44 PM, Peter Hutterer wrote:
> >>>Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >>
> >>It makes sense to move the startup to the XServer object, but the
> >>environment should then contain an XServer object and start it. The
> >>point of the environment is that it starts up an xserver
> >>automatically.
> >>
> >>The xorg-gtest user is free to not use the environment. But when
> >>they use the environment, it should start a server.
> >
> >It still does, it's just cut off from the Environment::SetUp() hunk though.
> >see below
> >
> >>>---
> >>>  include/xorg/gtest/xorg-gtest-xserver.h |    2 ++
> >>>  src/environment.cpp                     |   35 ---------------------------
> >>>  src/xserver.cpp                         |   40 +++++++++++++++++++++++++++++++
> >>>  3 files changed, 42 insertions(+), 35 deletions(-)
> >>>
> >>>diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
> >>>index 5c5ce99..52a2fd0 100644
> >>>--- a/include/xorg/gtest/xorg-gtest-xserver.h
> >>>+++ b/include/xorg/gtest/xorg-gtest-xserver.h
> >>>@@ -129,6 +129,8 @@ class XServer : public xorg::testing::Process {
> >>>      XServer(const XServer&);
> >>>      XServer& operator=(const XServer&);
> >>>
> >>>+    void TestStartup(void);
> >>>+
> >>>  };
> >>>  } // namespace testing
> >>>  } // namespace xorg
> >>>diff --git a/src/environment.cpp b/src/environment.cpp
> >>>index 7ed23b3..69972a4 100644
> >>>--- a/src/environment.cpp
> >>>+++ b/src/environment.cpp
> >>>@@ -106,41 +106,6 @@ void xorg::testing::Environment::SetUp() {
> >>>    static char display_string[6];
> >>>    snprintf(display_string, 6, ":%d", d_->display);
> >>>
> >>>-  Display* test_display = XOpenDisplay(display_string);
> >>>-  if (test_display) {
> >>>-    XCloseDisplay(test_display);
> >>>-    std::string message;
> >>>-    message += "A server is already running on ";
> >>>-    message += display_string;
> >>>-    message += ".";
> >>>-    throw std::runtime_error(message);
> >>>-  }
> >>>-
> >>>-  /* The Xorg server won't start unless the log file and the old log file are
> >>>-   * writable. */
> >>>-  std::ofstream log_test;
> >>>-  log_test.open(d_->path_to_log_file.c_str(), std::ofstream::out);
> >>>-  log_test.close();
> >>>-  if (log_test.fail()) {
> >>>-    std::string message;
> >>>-    message += "X.org server log file ";
> >>>-    message += d_->path_to_log_file;
> >>>-    message += " is not writable.";
> >>>-    throw std::runtime_error(message);
> >>>-  }
> >>>-
> >>>-  std::string old_log_file = d_->path_to_log_file.c_str();
> >>>-  old_log_file += ".old";
> >>>-  log_test.open(old_log_file.c_str(), std::ofstream::out);
> >>>-  log_test.close();
> >>>-  if (log_test.fail()) {
> >>>-    std::string message;
> >>>-    message += "X.org old server log file ";
> >>>-    message += old_log_file;
> >>>-    message += " is not writable.";
> >>>-    throw std::runtime_error(message);
> >>>-  }
> >>>-
> >>>    d_->server.SetDisplayNumber(d_->display);
> >>>    d_->server.SetLogfilePath(d_->path_to_log_file);
> >>>    d_->server.SetConfigPath(d_->path_to_conf);
> >
> >is followed by
> >     d_->server.Start(d_->path_to_server);
> >     d_->server.WaitForConnections();
> >
> >So nothing should change over the previous code. I haven't modified the
> >xi2.cpp code in the server which uses this environment and it still works.
> 
> Ahh, right, the code still starts the server, but shouldn't we be
> calling the server TestStartup() method before? As it is in this
> patch, we've removed a bunch of code, moved it to a method of a
> different object, and haven't called the method to ensure the same
> code is executed. Unless I'm misreading again :).

hard to see from the patch I guess, but the new sequence is
    Environment::SetUp()
       ...
       XServer::Start()
            XServer::TestStartup()
            Process::Start()

Cheers,
  Peter

> 
> >>>diff --git a/src/xserver.cpp b/src/xserver.cpp
> >>>index 38394f3..1a46dbb 100644
> >>>--- a/src/xserver.cpp
> >>>+++ b/src/xserver.cpp
> >>>@@ -41,6 +41,7 @@
> >>>  #include <cstring>
> >>>  #include <stdexcept>
> >>>  #include <vector>
> >>>+#include <fstream>
> >>>
> >>>  #include <X11/Xlib.h>
> >>>  #include <X11/extensions/XInput2.h>
> >>>@@ -250,7 +251,46 @@ void xorg::testing::XServer::WaitForConnections(void) {
> >>>    throw std::runtime_error("Unable to open connection to dummy X server");
> >>>  }
> >>>
> >>>+void xorg::testing::XServer::TestStartup(void) {
> >>>+  Display* test_display = XOpenDisplay(GetDisplayString());
> >>>+  if (test_display) {
> >>>+    XCloseDisplay(test_display);
> >>>+    std::string message;
> >>>+    message += "A server is already running on ";
> >>>+    message += GetDisplayString();
> >>>+    message += ".";
> >>>+    throw std::runtime_error(message);
> >>>+  }
> >>>+
> >>>+  /* The Xorg server won't start unless the log file and the old log file are
> >>>+   * writable. */
> >>>+  std::ofstream log_test;
> >>>+  log_test.open(d_->path_to_logfile.c_str(), std::ofstream::out);
> >>>+  log_test.close();
> >>>+  if (log_test.fail()) {
> >>>+    std::string message;
> >>>+    message += "X.org server log file ";
> >>>+    message += d_->path_to_logfile;
> >>>+    message += " is not writable.";
> >>>+    throw std::runtime_error(message);
> >>>+  }
> >>>+
> >>>+  std::string old_log_file = d_->path_to_logfile.c_str();
> >>>+  old_log_file += ".old";
> >>>+  log_test.open(old_log_file.c_str(), std::ofstream::out);
> >>>+  log_test.close();
> >>>+  if (log_test.fail()) {
> >>>+    std::string message;
> >>>+    message += "X.org old server log file ";
> >>>+    message += old_log_file;
> >>>+    message += " is not writable.";
> >>>+    throw std::runtime_error(message);
> >>>+  }
> >>>+
> >>>+}
> >>>+
> >>>  void xorg::testing::XServer::Start(std::string &program) {
> >>>+  TestStartup();
> >>>    Process::Start(program, program.c_str(),
> >>>                   GetDisplayString(),
> >>>                   "-logverbose", "10",
> >>>
> >>
> >_______________________________________________
> >xorg-devel at lists.x.org: X.Org development
> >Archives: http://lists.x.org/archives/xorg-devel
> >Info: http://lists.x.org/mailman/listinfo/xorg-devel
> >
>