[PATCH xorg-gtest v2 5/8] xserver: move testing startup to the XServer object

[Chase Douglas]

On 07/10/2012 08:28 PM, Peter Hutterer wrote:
> No real functional changes, TestStartup() is called by XServer::Start()
> before the process is started, so from the Environment's POV, everything
> stays the same.

This is much easier to understand now, in comparison to how these 
changes were split across a couple patches before. Thanks!

>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> No real hanges that I can recall
>
>   include/xorg/gtest/xorg-gtest-xserver.h |    2 ++
>   src/environment.cpp                     |   35 --------------------------
>   src/xserver.cpp                         |   41 +++++++++++++++++++++++++++++++
>   3 files changed, 43 insertions(+), 35 deletions(-)
>
> diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
> index 0b7e7a0..f31ce8a 100644
> --- a/include/xorg/gtest/xorg-gtest-xserver.h
> +++ b/include/xorg/gtest/xorg-gtest-xserver.h
> @@ -148,6 +148,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 1521564..608abaf 100644
> --- a/src/environment.cpp
> +++ b/src/environment.cpp

You can probably remove the #include <fstream> from the top of 
environment.cpp now.

> @@ -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.SetOption("-logfile", d_->path_to_log_file);
>     d_->server.SetOption("-config", d_->path_to_conf);
> diff --git a/src/xserver.cpp b/src/xserver.cpp
> index 2f9c364..f263c63 100644
> --- a/src/xserver.cpp
> +++ b/src/xserver.cpp
> @@ -42,6 +42,7 @@
>   #include <stdexcept>
>   #include <vector>
>   #include <map>
> +#include <fstream>
>
>   #include <X11/Xlib.h>
>   #include <X11/extensions/XInput2.h>
> @@ -253,10 +254,50 @@ 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().c_str());
> +  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_->options["-logfile"].c_str(), std::ofstream::out);
> +  log_test.close();
> +  if (log_test.fail()) {
> +    std::string message;
> +    message += "X.org server log file ";
> +    message += d_->options["-config"];
> +    message += " is not writable.";
> +    throw std::runtime_error(message);
> +  }
> +
> +  std::string old_log_file = d_->options["-config"];
> +  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) {
>     if (program.empty())
>       program = d_->path_to_server;
>
> +  TestStartup();
> +
>     std::vector<std::string> args;
>     std::map<std::string, std::string>::iterator it;
>
>

Looks good to me!

Reviewed-by: Chase Douglas <chase.douglas at canonical.com>