[PATCH xorg-gtest 15/16] test: add SetDisplayString()

Chase Douglas chase.douglas at canonical.com
Tue Jul 3 11:34:03 PDT 2012


On 07/02/2012 11:44 PM, Peter Hutterer wrote:
> Don't rely on the environment alone, take the display string if it's been
> set.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>   include/xorg/gtest/xorg-gtest-test.h |    7 +++++++
>   src/test.cpp                         |    8 +++++++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/xorg/gtest/xorg-gtest-test.h b/include/xorg/gtest/xorg-gtest-test.h
> index 3b78a17..3338c80 100644
> --- a/include/xorg/gtest/xorg-gtest-test.h
> +++ b/include/xorg/gtest/xorg-gtest-test.h
> @@ -88,6 +88,13 @@ class Test : public ::testing::Test {
>      */
>     ::Display* Display() const;
>
> +  /**
> +   * Set the display string used for XOpenDisplay()
> +   *

I think we need to add a detailed comment like:

"""
In order to set the display string, create a subclass and override 
SetUp(). In the subclass SetUp(), set the display string and then call 
xorg::testing::Test::Setup().
"""

IIRC, that's the only way this method can be used effectively.

> +   * @param display The string representing the display connection, or NULL
> +   */
> +  void SetDisplayString(const char *display);

Let's pass in a const std::string& instead. It's easy to convert to a c 
string in the implementation, you can still pass a string literal into 
the function (it gets converted automatically), and it's a more natural 
c++ api.

> +
>     /** @cond Implementation */
>     struct Private;
>     std::auto_ptr<Private> d_;
> diff --git a/src/test.cpp b/src/test.cpp
> index e3e65e4..499915b 100644
> --- a/src/test.cpp
> +++ b/src/test.cpp
> @@ -33,16 +33,18 @@
>
>   struct xorg::testing::Test::Private {
>     ::Display* display;
> +  const char *display_string;
>   };
>
>   xorg::testing::Test::Test() : d_(new Private) {
>     d_->display = NULL;
> +  d_->display_string = NULL;
>   }
>
>   xorg::testing::Test::~Test() {}
>
>   void xorg::testing::Test::SetUp() {
> -  d_->display = XOpenDisplay(NULL);
> +  d_->display = XOpenDisplay(d_->display_string);
>     if (!d_->display)
>       throw std::runtime_error("Failed to open connection to display");
>   }
> @@ -56,3 +58,7 @@ void xorg::testing::Test::TearDown() {
>   ::Display* xorg::testing::Test::Display() const {
>     return d_->display;
>   }
> +
> +void xorg::testing::Test::SetDisplayString(const char *display) {
> +  d_->display_string = display;
> +}
>



More information about the xorg-devel mailing list