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

[Chase Douglas]

On 07/11/2012 03:16 PM, Peter Hutterer wrote:
> 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.

Oh, right. I didn't realize all the existing classes were documented 
like that. You're right that we should be uniform, and this isn't a big 
enough deal to worry about patching anyway :).

> [...]
>
>>> +#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.

Ok

-- Chase