[PATCH xorg-gtest v2 3/8] xserver: add WaitForConnections()

[Chase Douglas]

On 07/10/2012 08:28 PM, Peter Hutterer wrote:
> Moved from Environment to XServer class
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> No changes to v1
>
>   include/xorg/gtest/xorg-gtest-xserver.h |    5 +++++
>   src/environment.cpp                     |   33 +----------------------------
>   src/xserver.cpp                         |   35 +++++++++++++++++++++++++++++++
>   3 files changed, 41 insertions(+), 32 deletions(-)
>
> diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
> index 58fdc53..1ba2a0f 100644
> --- a/include/xorg/gtest/xorg-gtest-xserver.h
> +++ b/include/xorg/gtest/xorg-gtest-xserver.h
> @@ -47,6 +47,11 @@ class XServer : public xorg::testing::Process {
>       XServer();
>
>       /**
> +     * Waits until this server is ready to take connections.
> +     */
> +    void WaitForConnections(void);
> +
> +    /**
>        * Set the display number for this server. This number must be set
>        * before the server is started to have any effect.
>        * If unset, the default display number is used.
> diff --git a/src/environment.cpp b/src/environment.cpp
> index 2f1c4e2..7573b62 100644
> --- a/src/environment.cpp
> +++ b/src/environment.cpp
> @@ -150,40 +150,9 @@ void xorg::testing::Environment::SetUp() {
>                       "-logfile", d_->path_to_log_file.c_str(),
>                       "-config", d_->path_to_conf.c_str(),
>                       NULL);
> +  d_->server.WaitForConnections();
>
>     Process::SetEnv("DISPLAY", display_string, true);
> -
> -  for (int i = 0; i < 10; ++i) {
> -    test_display = XOpenDisplay(NULL);
> -
> -    if (test_display) {
> -      XCloseDisplay(test_display);
> -      return;
> -    }
> -
> -    int status;
> -    int pid = waitpid(d_->server.Pid(), &status, WNOHANG);
> -    if (pid == d_->server.Pid()) {
> -      std::string message;
> -      message += "X server failed to start on display ";
> -      message += display_string;
> -      message += ". Ensure that the \"dummy\" video driver is installed.\n"
> -                 "If the X.org server is older than 1.12, "
> -                 "tests will need to be run as root.\nCheck ";
> -      message += d_->path_to_log_file;
> -      message += " for any errors";
> -      throw std::runtime_error(message);
> -    } else if (pid == 0) {
> -      sleep(1); /* Give the dummy X server some time to start */
> -    } else if (pid == -1) {
> -      throw std::runtime_error("Could not get status of dummy X server "
> -                               "process");
> -    } else {
> -      throw std::runtime_error("Invalid child PID returned by Process::Wait()");
> -    }
> -  }
> -
> -  throw std::runtime_error("Unable to open connection to dummy X server");
>   }
>
>   void xorg::testing::Environment::TearDown() {
> diff --git a/src/xserver.cpp b/src/xserver.cpp
> index f36bd10..62bd48f 100644
> --- a/src/xserver.cpp
> +++ b/src/xserver.cpp
> @@ -43,6 +43,7 @@
>   #include <vector>
>   #include <map>
>
> +#include <X11/Xlib.h>
>   #include <X11/extensions/XInput2.h>
>
>   struct xorg::testing::XServer::Private {
> @@ -218,6 +219,40 @@ bool xorg::testing::XServer::WaitForDevice(::Display *display, const std::string
>       return false;
>   }
>
> +void xorg::testing::XServer::WaitForConnections(void) {
> +  for (int i = 0; i < 10; ++i) {
> +    Display *test_display = XOpenDisplay(GetDisplayString().c_str());
> +
> +    if (test_display) {
> +      XCloseDisplay(test_display);
> +      return;
> +    }
> +
> +    int status;
> +    int pid = waitpid(Pid(), &status, WNOHANG);
> +    if (pid == Pid()) {
> +      std::string message;
> +      message += "X server failed to start on display ";
> +      message +=  GetDisplayString();
> +      message += ". Ensure that the \"dummy\" video driver is installed.\n"
> +                 "If the X.org server is older than 1.12, "
> +                 "tests will need to be run as root.\nCheck ";
> +      message += d_->options["-logfile"];
> +      message += " for any errors";
> +      throw std::runtime_error(message);
> +    } else if (pid == 0) {
> +      sleep(1); /* Give the dummy X server some time to start */
> +    } else if (pid == -1) {
> +      throw std::runtime_error("Could not get status of dummy X server "
> +                               "process");
> +    } else {
> +      throw std::runtime_error("Invalid child PID returned by Process::Wait()");
> +    }
> +  }
> +
> +  throw std::runtime_error("Unable to open connection to dummy X server");
> +}
> +
>   void xorg::testing::XServer::SetOption(std::string key, std::string value) {
>     d_->options[key] = value;
>   }
>

Looks good to me!

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