[PATCH xorg-gtest 01/16] Add a new class representing the X server

Peter Hutterer peter.hutterer at who-t.net
Tue Jul 3 22:26:33 PDT 2012


On Tue, Jul 03, 2012 at 07:59:46AM -0700, Chase Douglas wrote:
> On 07/02/2012 11:44 PM, Peter Hutterer wrote:
> >This class is a Process, so it's a drop-in replacement for the current
> >Environment, but it provides a few methods to talk to the server being
> >started.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  configure.ac                            |    2 +-
> >  include/Makefile.am                     |    1 +
> >  include/xorg/gtest/xorg-gtest-xserver.h |  113 ++++++++++++++++++
> >  include/xorg/gtest/xorg-gtest.h         |    1 +
> >  src/Makefile.am                         |    1 +
> >  src/environment.cpp                     |   23 ++--
> >  src/xorg-gtest-all.cpp                  |    1 +
> >  src/xserver.cpp                         |  192 +++++++++++++++++++++++++++++++
> >  8 files changed, 323 insertions(+), 11 deletions(-)
> >  create mode 100644 include/xorg/gtest/xorg-gtest-xserver.h
> >  create mode 100644 src/xserver.cpp
> >
> >diff --git a/configure.ac b/configure.ac
> >index 6e9e458..e46cc0b 100644
> >--- a/configure.ac
> >+++ b/configure.ac
> >@@ -25,7 +25,7 @@ XORG_DEFAULT_OPTIONS
> >  XORG_ENABLE_INTEGRATION_TESTS([yes])
> >  XORG_WITH_DOXYGEN
> >
> >-PKG_CHECK_MODULES(X11, x11)
> >+PKG_CHECK_MODULES(X11, x11 xi)
> >
> >  # Check for Google Test
> >  CHECK_GTEST
> >diff --git a/include/Makefile.am b/include/Makefile.am
> >index 9ff5f2a..4a6ce03 100644
> >--- a/include/Makefile.am
> >+++ b/include/Makefile.am
> >@@ -34,6 +34,7 @@ nobase_include_HEADERS = \
> >  	xorg/gtest/xorg-gtest-environment.h \
> >  	xorg/gtest/xorg-gtest-process.h \
> >  	xorg/gtest/xorg-gtest-test.h \
> >+	xorg/gtest/xorg-gtest-xserver.h \
> >  	xorg/gtest/evemu/xorg-gtest-device.h \
> >  	xorg/gtest/xorg-gtest.h \
> >  	$(compat_headers)
> >diff --git a/include/xorg/gtest/xorg-gtest-xserver.h b/include/xorg/gtest/xorg-gtest-xserver.h
> >new file mode 100644
> >index 0000000..78f72ce
> >--- /dev/null
> >+++ b/include/xorg/gtest/xorg-gtest-xserver.h
> >@@ -0,0 +1,113 @@
> >+/*******************************************************************************
> >+ *
> >+ * X testing environment - Google Test helper class to communicate with the
> >+ * server
> >+ *
> >+ * Copyright (C) 2012 Red Hat, Inc.
> >+ *
> >+ * Permission is hereby granted, free of charge, to any person obtaining a copy
> >+ * of this software and associated documentation files (the "Software"), to deal
> >+ * in the Software without restriction, including without limitation the rights
> >+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> >+ * copies of the Software, and to permit persons to whom the Software is
> >+ * furnished to do so, subject to the following conditions:
> >+ *
> >+ * The above copyright notice and this permission notice (including the next
> >+ * paragraph) shall be included in all copies or substantial portions of the
> >+ * Software.
> >+ *
> >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> >+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> >+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> >+ * SOFTWARE.
> >+ *
> >+ ******************************************************************************/
> >+
> >+
> >+#ifndef XORG_GTEST_XSERVER_H
> >+#define XORG_GTEST_XSERVER_H
> >+
> >+#include <gtest/gtest.h>
> >+#include <xorg/gtest/xorg-gtest.h>
> >+#include <X11/Xlib.h>
> >+
> >+namespace xorg {
> >+namespace testing {
> >+
> >+/**
> >+ * @class XServer xorg-gtest_xserver.h xorg/gtest/xorg-gtest_xserver.h
> >+ *
> >+ * Miscellaneous interfaces to communicate with the X server.
> >+ */
> >+class XServer : public xorg::testing::Process {
> >+  public:
> >+    XServer();
> >+
> >+    /**
> >+     * @param [in] display_number The display number the server runs on
> >+     */
> 
> Please provide a brief documentation for each function.
> 
> >+    void SetDisplayNumber(unsigned int display_number);
> >+
> >+    /**
> >+     * Get the display string that may be used for XOpenDisplay to this
> >+     * server. This string is effectively :display_number.
> >+     *
> >+     * @return The display string used for XOpenDisplay() to this server.
> >+     */
> >+    const char* GetDisplayString(void);
> >+
> >+    /**
> >+     * Wait for a specific device to be added to the server.
> >+     *
> >+     * @param [in] display The X display connection
> >+     * @param [in] name    The name of the device to wait for
> >+     * @param [in] timeout The timeout in milliseconds
> >+     *
> >+     * @return Whether the device was added
> >+     */
> >+    static bool WaitForDevice(::Display *display, const std::string &name, time_t timeout = 1000);
> >+
> >+    /**
> >+     * Wait for an event on the X connection.
> >+     *
> >+     * @param [in] display The X display connection
> >+     * @param [in] timeout The timeout in milliseconds
> >+     *
> >+     * @return Whether an event is available
> >+     */
> >+    static bool WaitForEvent(::Display *display, time_t timeout = 1000);
> >+
> >+    /**
> >+     * Wait for an event of a specific type on the X connection.
> >+     *
> >+     * All events preceding the matching event are discarded. If no event was found
> >+     * before the timeout expires, all events in the queue will have been discarded.
> >+     *
> >+     * @param [in] display The X display connection
> >+     * @param [in] type      The X core protocol event type
> >+     * @param [in] extension The X extension opcode of a generic event, or -1 for
> >+     *                       any generic event
> >+     * @param [in] evtype    The X extension event type of a generic event, or -1
> >+     *                       for any event of the given extension
> >+     * @param [in] timeout   The timeout in milliseconds
> >+     *
> >+     * @return Whether an event is available
> >+     */
> >+    static bool WaitForEventOfType(::Display *display, int type, int extension, int evtype, time_t timeout = 1000);
> >+
> >+  private:
> >+    struct Private;
> >+    std::auto_ptr<Private> d_;
> >+
> >+    /* Disable copy constructor, assignment operator */
> >+    XServer(const XServer&);
> >+    XServer& operator=(const XServer&);
> >+
> >+};
> >+} // namespace testing
> >+} // namespace xorg
> >+
> >+#endif /* XORG_GTEST_XSERVER_H */
> >diff --git a/include/xorg/gtest/xorg-gtest.h b/include/xorg/gtest/xorg-gtest.h
> >index 9d57e9e..6a621bd 100644
> >--- a/include/xorg/gtest/xorg-gtest.h
> >+++ b/include/xorg/gtest/xorg-gtest.h
> >@@ -27,6 +27,7 @@
> >
> >  #include "xorg-gtest-environment.h"
> >  #include "xorg-gtest-process.h"
> >+#include "xorg-gtest-xserver.h"
> >  #include "xorg-gtest-test.h"
> >
> >  #ifdef HAVE_EVEMU
> >diff --git a/src/Makefile.am b/src/Makefile.am
> >index 0b93df0..3d7824d 100644
> >--- a/src/Makefile.am
> >+++ b/src/Makefile.am
> >@@ -29,6 +29,7 @@ libxorg_gtest_sources = \
> >  	device.cpp \
> >  	process.cpp \
> >  	test.cpp \
> >+	xserver.cpp \
> >  	xorg-gtest-all.cpp
> >
> >  libxorg_gtest_main_sources = \
> >diff --git a/src/environment.cpp b/src/environment.cpp
> >index c29d4a8..43e1e70 100644
> >--- a/src/environment.cpp
> >+++ b/src/environment.cpp
> >@@ -27,6 +27,7 @@
> >
> >  #include "xorg/gtest/xorg-gtest-environment.h"
> >  #include "xorg/gtest/xorg-gtest-process.h"
> >+#include "xorg/gtest/xorg-gtest-xserver.h"
> >  #include "defines.h"
> >
> >  #include <sys/types.h>
> >@@ -52,7 +53,7 @@ struct xorg::testing::Environment::Private {
> >    std::string path_to_log_file;
> >    std::string path_to_server;
> >    int display;
> >-  Process process;
> >+  XServer server;
> >  };
> >
> >  xorg::testing::Environment::Environment()
> >@@ -140,13 +141,15 @@ void xorg::testing::Environment::SetUp() {
> >      throw std::runtime_error(message);
> >    }
> >
> >-  d_->process.Start(d_->path_to_server, d_->path_to_server.c_str(),
> >+  d_->server.Start(d_->path_to_server, d_->path_to_server.c_str(),
> >                      display_string,
> >                      "-logverbose", "10",
> >                      "-logfile", d_->path_to_log_file.c_str(),
> >                      "-config", d_->path_to_conf.c_str(),
> >                      NULL);
> >
> >+  d_->server.SetDisplayNumber(d_->display);
> >+
> 
> You're setting the display number *after* you've started the
> display. The display number has already been passed in as
> display_string above. What are you trying to do?

In this patch, setting the display number doesn't really have any effect
anyway. I admit that rebasing this patch series into sensible chunks was
hard and at least one rebase seems to have gone wrong without me noticing.
That probably explains some of the quirkyness, I'll re-check and clean all
those up (take this as placeholder email instead of me replying to all
separately :)

I've updated the documentation and moved the display number assignment up to
before Start.

Cheers,
  Peter
 
> >    Process::SetEnv("DISPLAY", display_string, true);
> >
> >    for (int i = 0; i < 10; ++i) {
> >@@ -158,8 +161,8 @@ void xorg::testing::Environment::SetUp() {
> >      }
> >
> >      int status;
> >-    int pid = waitpid(d_->process.Pid(), &status, WNOHANG);
> >-    if (pid == d_->process.Pid()) {
> >+    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;
> >@@ -183,12 +186,12 @@ void xorg::testing::Environment::SetUp() {
> >  }
> >
> >  void xorg::testing::Environment::TearDown() {
> >-  if (d_->process.Terminate()) {
> >+  if (d_->server.Terminate()) {
> >      for (int i = 0; i < 10; i++) {
> >        int status;
> >-      int pid = waitpid(d_->process.Pid(), &status, WNOHANG);
> >+      int pid = waitpid(d_->server.Pid(), &status, WNOHANG);
> >
> >-      if (pid == d_->process.Pid())
> >+      if (pid == d_->server.Pid())
> >          return;
> >
> >        sleep(1); /* Give the dummy X server more time to shut down */
> >@@ -199,15 +202,15 @@ void xorg::testing::Environment::TearDown() {
> >  }
> >
> >  void xorg::testing::Environment::Kill() {
> >-  if (!d_->process.Kill())
> >+  if (!d_->server.Kill())
> >      std::cerr << "Warning: Failed to kill dummy Xorg server: "
> >                << std::strerror(errno) << "\n";
> >
> >    for (int i = 0; i < 10; i++) {
> >      int status;
> >-    int pid = waitpid(d_->process.Pid(), &status, WNOHANG);
> >+    int pid = waitpid(d_->server.Pid(), &status, WNOHANG);
> >
> >-    if (pid == d_->process.Pid())
> >+    if (pid == d_->server.Pid())
> >        return;
> >
> >        sleep(1); /* Give the dummy X server more time to shut down */
> >diff --git a/src/xorg-gtest-all.cpp b/src/xorg-gtest-all.cpp
> >index fab6425..85e17c0 100644
> >--- a/src/xorg-gtest-all.cpp
> >+++ b/src/xorg-gtest-all.cpp
> >@@ -27,6 +27,7 @@
> >
> >  #include "src/environment.cpp"
> >  #include "src/process.cpp"
> >+#include "src/xserver.cpp"
> >  #include "src/test.cpp"
> >
> >  #ifdef HAVE_EVEMU
> >diff --git a/src/xserver.cpp b/src/xserver.cpp
> >new file mode 100644
> >index 0000000..28130d7
> >--- /dev/null
> >+++ b/src/xserver.cpp
> >@@ -0,0 +1,192 @@
> >+/*******************************************************************************
> >+ *
> >+ * X testing environment - Google Test helper class to communicate with the
> >+ * server
> >+ *
> >+ * Copyright (C) 2012 Red Hat, Inc.
> >+ *
> >+ * Permission is hereby granted, free of charge, to any person obtaining a copy
> >+ * of this software and associated documentation files (the "Software"), to deal
> >+ * in the Software without restriction, including without limitation the rights
> >+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> >+ * copies of the Software, and to permit persons to whom the Software is
> >+ * furnished to do so, subject to the following conditions:
> >+ *
> >+ * The above copyright notice and this permission notice (including the next
> >+ * paragraph) shall be included in all copies or substantial portions of the
> >+ * Software.
> >+ *
> >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> >+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> >+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> >+ * SOFTWARE.
> >+ *
> >+ ******************************************************************************/
> >+
> >+#include "xorg/gtest/xorg-gtest_xserver.h"
> >+
> >+#include <sys/types.h>
> >+#include <sys/wait.h>
> >+#include <unistd.h>
> >+
> >+#include <algorithm>
> >+#include <cerrno>
> >+#include <csignal>
> >+#include <cstdio>
> >+#include <cstdlib>
> >+#include <cstring>
> >+#include <stdexcept>
> >+#include <vector>
> >+
> >+#include <X11/extensions/XInput2.h>
> >+
> >+struct xorg::testing::XServer::Private {
> >+  unsigned int display_number;
> >+  std::string display_string;
> >+};
> >+
> >+xorg::testing::XServer::XServer() : d_(new Private) {
> >+  SetDisplayNumber(d_->display_number);
> >+}
> >+
> >+void xorg::testing::XServer::SetDisplayNumber(unsigned int display_number) {
> >+    d_->display_number = display_number;
> >+
> >+    std::stringstream s;
> >+    s << ":" << display_number;
> >+    d_->display_string = s.str();
> >+}
> >+
> >+const char* xorg::testing::XServer::GetDisplayString(void) {
> >+  return d_->display_string.c_str();
> >+}
> >+
> >+bool xorg::testing::XServer::WaitForEvent(::Display *display, time_t timeout)
> >+{
> >+    fd_set fds;
> >+    FD_ZERO(&fds);
> >+
> >+    int display_fd = ConnectionNumber(display);
> >+
> >+    XSync(display, False);
> >+
> >+    if (XPending(display))
> >+        return true;
> >+    else {
> >+        FD_SET(display_fd, &fds);
> >+
> >+        struct timeval timeval = {
> >+            static_cast<time_t>(timeout / 1000),
> >+            static_cast<time_t>(timeout % 1000),
> >+        };
> >+
> >+        int ret;
> >+        if (timeout)
> >+            ret = select(display_fd + 1, &fds, NULL, NULL, &timeval);
> >+        else
> >+            ret = select(display_fd + 1, &fds, NULL, NULL, NULL);
> >+
> >+        if (ret < 0)
> >+            throw std::runtime_error("Failed to select on X fd");
> >+
> >+        if (ret == 0)
> >+            return false;
> >+
> >+        return XPending(display);
> >+    }
> >+}
> >+
> >+bool xorg::testing::XServer::WaitForEventOfType(::Display *display, int type, int extension,
> >+                                                int evtype, time_t timeout)
> >+{
> >+    while (WaitForEvent(display)) {
> >+        XEvent event;
> >+        if (!XPeekEvent(display, &event))
> >+            throw std::runtime_error("Failed to peek X event");
> >+
> >+        if (event.type != type) {
> >+            if (XNextEvent(display, &event) != Success)
> >+                throw std::runtime_error("Failed to remove X event");
> >+            continue;
> >+        }
> >+
> >+        if (event.type != GenericEvent || extension == -1)
> >+            return true;
> >+
> >+        XGenericEvent *generic_event = reinterpret_cast<XGenericEvent*>(&event);
> >+
> >+        if (generic_event->extension != extension) {
> >+            if (XNextEvent(display, &event) != Success)
> >+                throw std::runtime_error("Failed to remove X event");
> >+            continue;
> >+        }
> >+
> >+        if (evtype == -1 || generic_event->evtype == evtype)
> >+            return true;
> >+
> >+        if (XNextEvent(display, &event) != Success)
> >+            throw std::runtime_error("Failed to remove X event");
> >+    }
> >+
> >+    return false;
> >+}
> >+
> >+bool xorg::testing::XServer::WaitForDevice(::Display *display, const std::string &name,
> >+                                           time_t timeout)
> >+{
> >+    int opcode;
> >+    int event_start;
> >+    int error_start;
> >+
> >+    if (!XQueryExtension(display, "XInputExtension", &opcode, &event_start,
> >+                         &error_start))
> >+        throw std::runtime_error("Failed to query XInput extension");
> >+
> >+    while (WaitForEventOfType(display, GenericEvent, opcode,
> >+                              XI_HierarchyChanged)) {
> >+        XEvent event;
> >+        if (XNextEvent(display, &event) != Success)
> >+            throw std::runtime_error("Failed to get X event");
> >+
> >+        XGenericEventCookie *xcookie =
> >+            reinterpret_cast<XGenericEventCookie*>(&event.xcookie);
> >+        if (!XGetEventData(display, xcookie))
> >+            throw std::runtime_error("Failed to get X event data");
> >+
> >+        XIHierarchyEvent *hierarchy_event =
> >+            reinterpret_cast<XIHierarchyEvent*>(xcookie->data);
> >+
> >+        if (!(hierarchy_event->flags & XIDeviceEnabled)) {
> >+            XFreeEventData(display, xcookie);
> >+            continue;
> >+        }
> >+
> >+        bool device_found = false;
> >+        for (int i = 0; i < hierarchy_event->num_info; i++) {
> >+            if (!(hierarchy_event->info[i].flags & XIDeviceEnabled))
> >+                continue;
> >+
> >+            int num_devices;
> >+            XIDeviceInfo *device_info =
> >+                XIQueryDevice(display, hierarchy_event->info[i].deviceid,
> >+                              &num_devices);
> >+            if (num_devices != 1 || !device_info)
> >+                throw std::runtime_error("Failed to query device");
> >+
> >+            if (name.compare(device_info[0].name) == 0) {
> >+                device_found = true;
> >+                break;
> >+            }
> >+        }
> >+
> >+        XFreeEventData(display, xcookie);
> >+
> >+        if (device_found)
> >+            return true;
> >+    }
> >+
> >+    return false;
> >+}
> >
> 


More information about the xorg-devel mailing list