Merge lp:~compiz-team/compiz/compiz.fix_1178514 into lp:compiz/0.9.10

[Sam Spilsbury]

Try to launch tests on another server if there's tests running in parallel.

Provide our own compiz_xorg_gtest_main and subclass xorg::testing::Environment
to try and launch tests on another display if there's tests running on
one already.

This isn't by any means perfect - there are still race conditions
surrounding XOpenDisplay and parallel test runs but it makes a more smaller
time gap for conditions such as:
 1. Client has a server grab on the display we're checking
    and won't let go.
 2. Two servers get launched on one port and one set of tests
    interfere with the other.

It also means that we're now unable to configure the display port,
log file and config file on the command line. But we weren't using that
anyways.

Finally, the logs now point to /tmp/Compiz.Xorg.GTest.displaynum.log

(LP: #1178514)

Try to launch tests on another server if there's tests running in parallel.

Provide our own compiz_xorg_gtest_main and subclass xorg::testing::Environment
to try and launch tests on another display if there's tests running on
one already.

This isn't by any means perfect - there are still race conditions
surrounding XOpenDisplay and parallel test runs but it makes a more smaller
time gap for conditions such as:
 1. Client has a server grab on the display we're checking
    and won't let go.
 2. Two servers get launched on one port and one set of tests
    interfere with the other.

It also means that we're now unable to configure the display port,
log file and config file on the command line. But we weren't using that
anyways.

Finally, the logs now point to /tmp/Compiz.Xorg.GTest.displaynum.log

(LP: #1178514)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3690
http://jenkins.qa.ubuntu.com/job/compiz-ci/170/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/211
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/27
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/25

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/170/rebuild

review: Approve (continuous-integration)

[MC Return (mc-return)]

Maybe a newline after:

137 + Display *check = XOpenDisplay (ss.str ().c_str ());

This looks like it is wrongly indented:

172 + if (environment)
173 + environment->Kill ();

here also:

215 + for (unsigned i = 0; i < sizeof(signals) / sizeof(signals[0]); ++i)
216 + if (sigaction(signals[i], &action, NULL))
217 + std::cerr << "Warning: Failed to set signal handler for signal "

Note: I did just check the .diff attached here -> maybe it just looks wrong...

[lp:~compiz-team/compiz/compiz.fix_1178514]

3691. By Sam Spilsbury on 2013-05-10

Fix indentation

[Sam Spilsbury (smspillaz)]

On Fri, May 10, 2013 at 3:11 PM, MC Return <email address hidden> wrote:
> Maybe a newline after:
>
> 137 + Display *check = XOpenDisplay (ss.str ().c_str ());
>
> This looks like it is wrongly indented:
>
> 172 + if (environment)
> 173 + environment->Kill ();
>
> here also:
>
> 215 + for (unsigned i = 0; i < sizeof(signals) / sizeof(signals[0]); ++i)
> 216 + if (sigaction(signals[i], &action, NULL))
> 217 + std::cerr << "Warning: Failed to set signal handler for signal "
>
> Note: I did just check the .diff attached here -> maybe it just looks wrong...

Ah, thanks for that. I've been working on a two-space project for a
few days now and didn't notice those.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3691
http://jenkins.qa.ubuntu.com/job/compiz-ci/171/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/212/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/28/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/28
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/26/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/171/rebuild

review: Needs Fixing (continuous-integration)

[MC Return (mc-return)]

I think this change in r3691 is wrong:

55 std::stringstream ss;
 55 std::stringstream
5656 ss << ":" << displayNumber;
57 Display *check = XOpenDisplay (ss.str ().c_str ());
 57 Display *check = XOpenDisplay (ss.str ().c_

You need to initialize std::stringstream ss; seperately...

[Sam Spilsbury (smspillaz)]

On Fri, May 10, 2013 at 5:36 PM, MC Return <email address hidden> wrote:
> I think this change in r3691 is wrong:
>
> 55 std::stringstream ss;
> 55 std::stringstream
> 5656 ss << ":" << displayNumber;
> 57 Display *check = XOpenDisplay (ss.str ().c_str ());
> 57 Display *check = XOpenDisplay (ss.str ().c_
>
> You need to initialize std::stringstream ss; seperately...

stringstream can only be initialized with a string buffer, or its
default constructor creates a new one. You can't put any content into
a stringstream from its constructor.

> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1178514/+merge/163257
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1178514 into lp:compiz.

--
Sam Spilsbury

[Sam Spilsbury (smspillaz)]

Wait, nope, scratch that, I'm an idiot. Fixed.

[lp:~compiz-team/compiz/compiz.fix_1178514]

3692. By Sam Spilsbury on 2013-05-10

Fix accidentally removed variable

[MC Return (mc-return)]

> Wait, nope, scratch that, I'm an idiot. Fixed.

I ran into the same problem, when I cleaned up OpenGL (has not been proposed for merging yet), that is why I know you have to do it...

[MC Return (mc-return)]

LGTM now AFAICT.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:3692
http://jenkins.qa.ubuntu.com/job/compiz-ci/172/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/213/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/29/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/29
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/27/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/172/rebuild

review: Needs Fixing (continuous-integration)

[lp:~compiz-team/compiz/compiz.fix_1178514]

3693. By Sam Spilsbury on 2013-05-11

Fix truncated line

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3693
http://jenkins.qa.ubuntu.com/job/compiz-ci/173/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/214
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/28

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/173/rebuild

review: Approve (continuous-integration)

[Sam Spilsbury (smspillaz)]

review: Approve

=== modified file 'plugins/decor/src/pixmap-requests/tests/integration/xorg-gtest/CMakeLists.txt'
--- plugins/decor/src/pixmap-requests/tests/integration/xorg-gtest/CMakeLists.txt	2013-04-22 14:30:55 +0000
+++ plugins/decor/src/pixmap-requests/tests/integration/xorg-gtest/CMakeLists.txt	2013-05-11 01:07:32 +0000
@@ -18,7 +18,7 @@
 
     set (COMPIZ_DECOR_PIXMAP_PROTOCOL_XORG_INTEGRATION_TEST_LIBRARIES
          xorg_gtest_all
-         xorg_gtest_main
+         compiz_xorg_gtest_main
          compiz_xorg_gtest_system_test
          compiz_decor_pixmap_requests
          compiz_decor_pixmap_requests_mock

=== modified file 'tests/acceptance-tests/xorg-gtest/tests/CMakeLists.txt'
--- tests/acceptance-tests/xorg-gtest/tests/CMakeLists.txt	2013-04-22 14:30:55 +0000
+++ tests/acceptance-tests/xorg-gtest/tests/CMakeLists.txt	2013-05-11 01:07:32 +0000
@@ -7,7 +7,7 @@
 set (COMPIZ_XORG_ACCEPTANCE_TEST_LIBRARIES
      compiz_xorg_gtest_system_test
      xorg_gtest_all
-     xorg_gtest_main
+     compiz_xorg_gtest_main
      ${GTEST_BOTH_LIBRARIES}
      ${XORG_SERVER_LIBRARIES}
      ${X11_XI_LIBRARIES})

=== modified file 'tests/system/xorg-gtest/tests/CMakeLists.txt'
--- tests/system/xorg-gtest/tests/CMakeLists.txt	2013-04-22 14:30:55 +0000
+++ tests/system/xorg-gtest/tests/CMakeLists.txt	2013-05-11 01:07:32 +0000
@@ -31,7 +31,7 @@
     set (COMPIZ_XORG_GTEST_LIBRARIES
 	 compiz_xorg_gtest_system_test
 	 xorg_gtest_all
-	 xorg_gtest_main
+	 compiz_xorg_gtest_main
          ${GTEST_BOTH_LIBRARIES}
 	 ${XORG_SERVER_LIBRARIES}
 	 ${X11_XI_LIBRARIES})

=== modified file 'tests/xorg-gtest/CMakeLists.txt'
--- tests/xorg-gtest/CMakeLists.txt	2013-04-22 14:30:55 +0000
+++ tests/xorg-gtest/CMakeLists.txt	2013-05-11 01:07:32 +0000
@@ -24,14 +24,11 @@
 set (_xorg_gtest_all_srcs
      ${XORG_SERVER_GTEST_SRC}/src/xorg-gtest-all.cpp)
 
-set (_xorg_gtest_main_srcs
-     ${XORG_SERVER_GTEST_SRC}/src/xorg-gtest_main.cpp)
-
 add_library (xorg_gtest_all STATIC
 	     ${_xorg_gtest_all_srcs})
 
-add_library (xorg_gtest_main STATIC
-	     ${_xorg_gtest_main_srcs})
+add_library (compiz_xorg_gtest_main STATIC
+             ${CMAKE_CURRENT_SOURCE_DIR}/src/compiz_xorg_gtest_main.cpp)
 
 add_library (compiz_xorg_gtest_system_test STATIC
 	     ${CMAKE_CURRENT_SOURCE_DIR}/src/compiz-xorg-gtest.cpp)
@@ -43,13 +40,13 @@
 		       ${GTEST_BOTH_LIBRARIES}
 		       ${XORG_SERVER_GTEST_LIBRARIES})
 
-target_link_libraries (xorg_gtest_main
+target_link_libraries (compiz_xorg_gtest_main
 		       ${GTEST_BOTH_LIBRARIES}
 		       ${XORG_SERVER_GTEST_LIBRARIES})
 
 target_link_libraries (compiz_xorg_gtest_system_test
 		       xorg_gtest_all
-		       xorg_gtest_main
+		       compiz_xorg_gtest_main
 		       ${GTEST_BOTH_LIBRARIES}
 		       ${XORG_SERVER_LIBRARIES}
 		       ${COMPIZ_XORG_GTEST_COMMUNICATOR_LIBRARY}

=== added file 'tests/xorg-gtest/src/compiz_xorg_gtest_main.cpp'
--- tests/xorg-gtest/src/compiz_xorg_gtest_main.cpp	1970-01-01 00:00:00 +0000
+++ tests/xorg-gtest/src/compiz_xorg_gtest_main.cpp	2013-05-11 01:07:32 +0000
@@ -0,0 +1,154 @@
+/*
+ * Compiz XOrg GTest Decoration Pixmap Protocol Integration Tests
+ *
+ * Copyright (C) 2013 Sam Spilsbury.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Authored By:
+ * Sam Spilsbury <[e-mail]>
+ */
+
+#include <csignal>
+
+#include <sstream>
+#include <stdexcept>
+
+#include <gtest/gtest.h>
+#include "xorg/gtest/xorg-gtest-environment.h"
+
+#include <X11/Xlib.h>
+
+namespace compiz
+{
+namespace testing
+{
+class XorgEnvironment :
+    public xorg::testing::Environment
+{
+    public:
+
+	virtual void SetUp ()
+	{
+	    /* Another hack - if the server fails
+	     * to start then just set another display
+	     * number until it does */
+	    const int MaxConnections = 255;
+	    int displayNumber = 0;
+	    bool serverRunningOnDisplay = true;
+
+	    while (serverRunningOnDisplay &&
+		   displayNumber < MaxConnections)
+	    {
+		std::stringstream ss;
+		ss << ":" << displayNumber;
+		Display *check = XOpenDisplay (ss.str ().c_str ());
+
+		if (!check)
+		    serverRunningOnDisplay = false;
+		else
+		{
+		    XCloseDisplay (check);
+		    ++displayNumber;
+		}
+	    }
+
+	    if (displayNumber == MaxConnections)
+		throw std::runtime_error ("couldn't find a socket "
+					  "to launch on");
+
+	    std::stringstream logFile;
+	    logFile << "/tmp/Compiz.Xorg.GTest." << displayNumber << ".log";
+
+	    SetDisplayNumber (displayNumber);
+	    SetLogFile (logFile.str ());
+	    xorg::testing::Environment::SetUp ();
+	}
+};
+}
+}
+
+/* X testing environment - Google Test environment feat. dummy x server
+ * Copyright (C) 2011, 2012 Canonical Ltd.
+ */
+compiz::testing::XorgEnvironment *environment = NULL;
+
+namespace
+{
+
+void SignalHandler (int signum)
+{
+    if (environment)
+	environment->Kill ();
+
+    /* This will call the default handler because we used SA_RESETHAND */
+    raise (signum);
+}
+
+void SetupSignalHandlers ()
+{
+    static const int signals[] =
+    {
+	SIGHUP,
+	SIGTERM,
+	SIGQUIT,
+	SIGILL,
+	SIGABRT,
+	SIGFPE,
+	SIGSEGV,
+	SIGPIPE,
+	SIGALRM,
+	SIGTERM,
+	SIGUSR1,
+	SIGUSR2,
+	SIGBUS,
+	SIGPOLL,
+	SIGPROF,
+	SIGSYS,
+	SIGTRAP,
+	SIGVTALRM,
+	SIGXCPU,
+	SIGXFSZ,
+	SIGIOT,
+	SIGSTKFLT,
+	SIGIO,
+	SIGPWR,
+	SIGUNUSED,
+    };
+
+    struct sigaction action;
+    action.sa_handler = SignalHandler;
+    sigemptyset(&action.sa_mask);
+    action.sa_flags = SA_RESETHAND;
+
+    for (unsigned i = 0; i < sizeof(signals) / sizeof(signals[0]); ++i)
+	if (sigaction(signals[i], &action, NULL))
+	    std::cerr << "Warning: Failed to set signal handler for signal "
+		      << signals[i] << "\n";
+}
+}
+
+int main (int argc, char **argv)
+{
+    ::testing::InitGoogleTest (&argc, argv);
+
+    SetupSignalHandlers ();
+
+    environment = new compiz::testing::XorgEnvironment ();
+    ::testing::AddGlobalTestEnvironment (environment);
+
+    return RUN_ALL_TESTS ();
+}
+