[PATCH xorg-gtest v2 1/8] Add a new class representing the X server

Peter Hutterer peter.hutterer at who-t.net
Wed Jul 11 15:16:38 PDT 2012


On Wed, Jul 11, 2012 at 11:35:47AM -0700, Chase Douglas wrote:
> On 07/10/2012 08:28 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.
> >
> >SetDisplayNumber() is called, but currently unused by the server.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >Changes to v1:
> >- squashed a few minor patches in so this class is a bit more complete out
> >of the box.
> >
> >  configure.ac                            |    2 +-
> >  include/Makefile.am                     |    1 +
> >  include/xorg/gtest/xorg-gtest-xserver.h |  126 ++++++++++++++++++++
> >  include/xorg/gtest/xorg-gtest.h         |    1 +
> >  src/Makefile.am                         |    1 +
> >  src/environment.cpp                     |   22 ++--
> >  src/xorg-gtest-all.cpp                  |    1 +
> >  src/xserver.cpp                         |  196 +++++++++++++++++++++++++++++++
> >  8 files changed, 339 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 ea29c50..d9b3072 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..56dacf6
> >--- /dev/null
> >+++ b/include/xorg/gtest/xorg-gtest-xserver.h
> >@@ -0,0 +1,126 @@
> >+/*******************************************************************************
> >+ *
> >+ * 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
> 
> Minor niggle: doxygen is smart enough to know that we're documenting
> a class without having to tell it. I like leaving out the "@class"
> since it just clutters things up.

we can do that change but since we'd have to update all other header files
too (they all use @class atm) it's better to do this in one go, stating the
reason for doing so.

[...]

> >+#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);
> 
> If I'm reading this correctly, d_ will be allocated with unset
> values. d_->display_number will potentially be any integer. Then, we
> call SetDisplayNumber() with the same random integer.
> 
> We should be calling SetDisplayNumber(DEFAULT_DISPLAY).

oops, rebase fallout, at some point I had display_number initialised. I've
added 
  d_->display_number = DEFAULT_DISPLAY;
before the SetDisplayNumber() call.

Cheers,
  Peter


> 
> >+}
> >+
> >+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();
> >+}
> >+
> >+unsigned int xorg::testing::XServer::GetDisplayNumber(void) {
> >+  return d_->display_number;
> >+}
> >+
> >+const std::string& xorg::testing::XServer::GetDisplayString(void) {
> >+  return d_->display_string;
> >+}
> >+
> >+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;
> >+}
> >
> 
> Everything else looks good!
> 
> Reviewed-by: Chase Douglas <chase.douglas at canonical.com>


More information about the xorg-devel mailing list