[PATCH xorg-gtest v2 1/8] Add a new class representing the X server
Chase Douglas
chase.douglas at canonical.com
Wed Jul 11 11:35:47 PDT 2012
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.
> + *
> + * Miscellaneous interfaces to communicate with the X server.
> + */
> +class XServer : public xorg::testing::Process {
> + public:
> + XServer();
> +
> + /**
> + * 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.
> + *
> + * @param [in] display_number The display number the server runs on
> + */
> + void SetDisplayNumber(unsigned int display_number);
> +
> + /**
> + * Get the display number from this server. If the server was not
> + * started yet, this function returns the display number the server will
> + * be started on.
> + *
> + * @return The numeric display number this server runs on
> + */
> + unsigned int GetDisplayNumber(void);
> +
> + /**
> + * 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 std::string& 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
Minor niggle: this ^^ param documentation is two columns left of the
rest :).
> + * @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 b02a6a2..9a524ab 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,7 +141,8 @@ 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.SetDisplayNumber(d_->display);
> + 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(),
> @@ -158,8 +160,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 +185,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 +201,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..c24cee0
> --- /dev/null
> +++ b/src/xserver.cpp
> @@ -0,0 +1,196 @@
> +/*******************************************************************************
> + *
> + * 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);
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).
> +}
> +
> +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