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.hutte...@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.doug...@canonical.com>
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
