Merge lp://qastaging/~bregma/unity/use-xorg-gtest into lp://qastaging/unity

168 -Depends: ${shlibs:Depends},
169 - ${misc:Depends},
170 +Depends: ${misc:Depends}, ${shlibs:Depends}

As far I know distro guys preferred to keep these in multiline to avoid conflicts (even if these values shouldn't change for the packages we have).

However the change looks fine, but I've tried running it and... On first try I got a seg fault.
As "DISPLAY=:133 /usr/lib/nux/unity_support_test -p" shows, the problem here is the missing GLX support, and this can be easily been fixed by providing a xorg.conf containing this section:

Section "Files"
        ModulePath "/usr/lib/xorg/modules/extensions"
        ModulePath "/usr/lib/xorg/modules"
        ModulePath "/usr/lib/x86_64-linux-gnu/xorg/extra-modules"
        ModulePath "/usr/lib/xorg/extra-modules"
EndSection

Should we patch libxorg-gtest or can we provide a local config?

Also, when the test crashes, the dummy xorg instance is not stopped correctly of course, and so re-running test-gtest exposes the error: "A server is already running on :133." (why not using a random $DISPLAY?).
This is something that using a script to trigger Xorg would have been avoided.

> However the change looks fine, but I've tried running it and... On first try I got a seg fault.
> As "DISPLAY=:133 /usr/lib/nux/unity_support_test -p" shows, the problem here is the missing GLX support, and this can be easily been fixed by providing a xorg.conf containing this section:
>
> Section "Files"
> ModulePath "/usr/lib/xorg/modules/extensions"
> ModulePath "/usr/lib/xorg/modules"
> ModulePath "/usr/lib/x86_64-linux-gnu/xorg/extra-modules"
> ModulePath "/usr/lib/xorg/extra-modules"
> EndSection
>
> Should we patch libxorg-gtest or can we provide a local config?

Are you sure you don't just have a version of libgl1-mesa-dri that does not support software rendering through LLVMpipe?
 This branch builds and runs (until tests fail) out of the box on amd64 raring. Fixing
https://bugs.launchpad.net/ubuntu/+source/xorg-gtest/+bug/1110728 is required for the FTBFS on x86. I get a 'missing
GLX support' error on 12.10 because, well, it's missing GLX support. That's why I provided the --no-dummy-xserver
command-line option for testing. We're targeting raring, so this shouldn't be a serious problem.

> Also, when the test crashes, the dummy xorg instance is not stopped correctly of course, and so re-running test-gtest exposes the error: "A server is already running on :133." (why not using a random $DISPLAY?).
> This is something that using a script to trigger Xorg would have been avoided.

I pointed out the problem of having a hard-coded socket in the first point in the Description of the Change in this MP.
 This is a bug in xorg-gtest. The problem is not limited to non-cleanup on crash but will cause spurious failures if
other builds on the same machine are using xorg-gtest. The xorg-gtest upstream needs to be fixed.

It should also not crash if the X server doesn't start. I'll investigate that.

--
Stephen M. Webb <email address hidden>

This is a good step in the right direction towards making sure all the tests run in CI. Here is what I can point out at the moment:

353 +class UnityEnv: public xorg::testing::Environment
354 +{
355 +public:

...

399 + if (use_dummy_server_)
400 + {
401 + xorg::testing::Environment::SetUp();
402 + }

There could be stuff in xorg::testing::Environment::TearDown () and other virtual functions which depends on xorg::testing::Environment::SetUp () actually being executed, either override TearDown to only chain-through to xorg::testing::Environment::TearDown () in the case of use_dummy_server_ (ditto other virtual functions) or reconsider the inheritance hierarchy here.

Lots of random whitespace changes...

309 add_custom_target (check COMMAND ${TEST_COMMAND} DEPENDS test-unit test-gtest test-gtest-xless test-gtest-dbus test-gestures)
310 - add_custom_target (check-headless COMMAND ${TEST_COMMAND_HEADLESS} DEPENDS test-gtest-xless test-gtest-dbus test-gestures)
311 + add_custom_target (check-headless COMMAND ${TEST_COMMAND_HEADLESS} DEPENDS test-gtest-xless test-gtest test-gtest-dbus test-gestures)

It doesn't make sense to have a lot of redundant check targets. If we can run everything under check-headless now, why not just fold it all into check?

17 + execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE} xorg-gtest --variable=sourcedir
18 + OUTPUT_VARIABLE XORG_GTEST_ROOT_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
19 + execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE} xorg-gtest --variable=datarootdir
20 + OUTPUT_VARIABLE XORG_GTEST_DATA_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
21 +endif (ENABLE_X_SUPPORT)

You'll probably want to grab the prefix and CPPflags too.

    execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=prefix xorg-gtest OUTPUT_VARIABLE _xorg_gtest_prefix)
    execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=includedir xorg-gtest OUTPUT_VARIABLE _xorg_gtest_include_dir)
    execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=sourcedir xorg-gtest OUTPUT_VARIABLE _xorg_gtest_source_dir)
    execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=CPPflags xorg-gtest OUTPUT_VARIABLE _xorg_gtest_cflags)

Another thing to watch out for: the xorg-gtest package in the repositories, the last time I checked at least, was very out of date, and had a race condition which would cause tests to fail randomly.

A final note: While this is a good transitional strategy, tests that truly run headless should always be preferred to tests that require an X server connection. The reason being that it forces the author to actually separate dependencies and reduce coupling, increases cause isolation and also speeds up test execution.

>
> > However the change looks fine, but I've tried running it and... On first try
> I got a seg fault.
> > As "DISPLAY=:133 /usr/lib/nux/unity_support_test -p" shows, the problem here
> is the missing GLX support, and this can be easily been fixed by providing a
> xorg.conf containing this section:
> >
> > Section "Files"
> > ModulePath "/usr/lib/xorg/modules/extensions"
> > ModulePath "/usr/lib/xorg/modules"
> > ModulePath "/usr/lib/x86_64-linux-gnu/xorg/extra-modules"
> > ModulePath "/usr/lib/xorg/extra-modules"
> > EndSection
> >
> > Should we patch libxorg-gtest or can we provide a local config?
>
> Are you sure you don't just have a version of libgl1-mesa-dri that does not
> support software rendering through LLVMpipe?
> This branch builds and runs (until tests fail) out of the box on amd64
> raring.

I have... It's always provided on new installs.
You're right, without these lines it works well in raring, but it doesn't in quantal.
I added them to my config since I had this failure when doing my first tests with the dummy server and my logs suggested that...

On 01/30/2013 11:35 PM, Sam Spilsbury wrote:
> 399 + if (use_dummy_server_)
> 400 + {
> 401 + xorg::testing::Environment::SetUp();
> 402 + }
>
> There could be stuff in xorg::testing::Environment::TearDown ()

Right, good catch.

> 309 add_custom_target (check COMMAND ${TEST_COMMAND} DEPENDS test-unit test-gtest test-gtest-xless test-gtest-dbus test-gestures)
> 310 - add_custom_target (check-headless COMMAND ${TEST_COMMAND_HEADLESS} DEPENDS test-gtest-xless test-gtest-dbus test-gestures)
> 311 + add_custom_target (check-headless COMMAND ${TEST_COMMAND_HEADLESS} DEPENDS test-gtest-xless test-gtest test-gtest-dbus test-gestures)
>
> It doesn't make sense to have a lot of redundant check targets. If we can run everything under check-headless now, why not just fold it all into check?

test-gtest is still not run under check-headless, because there are tests failing for reasons other than X and that
would be a blocker. I left the test-gtest build dependency in even though it's not needed but I can see it's confusing,
so I'll remove it. That should be done after all the failing tests are fixed.

> 17 + execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE} xorg-gtest --variable=sourcedir
> 18 + OUTPUT_VARIABLE XORG_GTEST_ROOT_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
> 19 + execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE} xorg-gtest --variable=datarootdir
> 20 + OUTPUT_VARIABLE XORG_GTEST_DATA_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
> 21 +endif (ENABLE_X_SUPPORT)
>
> You'll probably want to grab the prefix and CPPflags too.
>
> execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=prefix xorg-gtest OUTPUT_VARIABLE _xorg_gtest_prefix)
> execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=includedir xorg-gtest OUTPUT_VARIABLE _xorg_gtest_include_dir)
> execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=sourcedir xorg-gtest OUTPUT_VARIABLE _xorg_gtest_source_dir)
> execute_process (COMMAND ${PKG_CONFIG_EXECUTABLE} --variable=CPPflags xorg-gtest OUTPUT_VARIABLE _xorg_gtest_cflags)

It's pretty unusual to get those variables like that. Take a look at the dozens of other packages used in Unity. In
fact, it does look like there is yet another bug in xorg-gtest: the pkgconfig file is incorrectly formatted. That
needs to get fixed.

> Another thing to watch out for: the xorg-gtest package in the repositories, the last time I checked at least, was very out of date, and had a race condition which would cause tests to fail randomly.

You might want to check again. I've been in touch with the package maintainer in Ubuntu and he's fairly responsive; the
version in raring is up-to-date with upstream HEAD and contains patches that have also been submitted upstream.

--
Stephen M. Webb <email address hidden>

Changing to IN PROGRESS until xorg-gtest issues are resolved