Merge lp://qastaging/~compiz-team/compiz/compiz.fix_1120009.3 into lp://qastaging/compiz/0.9.9

Proposed by Sam Spilsbury
Status: Merged
Approved by: Sam Spilsbury
Approved revision: 3573
Merged at revision: 3612
Proposed branch: lp://qastaging/~compiz-team/compiz/compiz.fix_1120009.3
Merge into: lp://qastaging/compiz/0.9.9
Diff against target: 1009 lines (+531/-151)
20 files modified
CMakeLists.txt (+38/-38)
cmake/CompizCommon.cmake (+18/-1)
cmake/FindGoogleTest.cmake (+46/-0)
cmake/FindXorgGTest.cmake (+43/-0)
cmake/GoogleTest.cmake (+20/-0)
cmake/XorgGTest.cmake (+14/-0)
cmake/src/compiz/CMakeLists.txt (+4/-0)
cmake/src/compiz/compiz_discover_gtest_tests.cpp (+36/-9)
debian/changelog (+30/-0)
debian/control (+3/-0)
debian/patches/ccp_plugin.patch (+49/-5)
debian/patches/ubuntu-config.patch (+24/-0)
debian/rules (+4/-4)
tests/system/xorg-gtest/CMakeLists.txt (+3/-31)
tests/system/xorg-gtest/tests/CMakeLists.txt (+4/-4)
tests/system/xorg-gtest/tests/compiz_xorg_gtest_icccm.cpp (+0/-7)
tests/xorg-gtest/CMakeLists.txt (+52/-0)
tests/xorg-gtest/src/CMakeLists.txt (+2/-51)
tests/xorg-gtest/src/compiz-xorg-gtest.cpp (+48/-1)
tests/xorg-gtest/src/xorg_gtest_wrapper.cpp (+93/-0)
To merge this branch: bzr merge lp://qastaging/~compiz-team/compiz/compiz.fix_1120009.3
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Andrea Azzarone Approve
Brandon Schaefer Pending
MC Return Pending
Review via email: mp+148839@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2013-02-15.

Commit message

Enable xorg-gtest tests by default and build in CI.

This change also refactors a number of parts of the code to find and build the tests. It puts the logic to find Google Test and Xorg GTest in their own cmake files, and also splits the find logic with the build logic. Finally, it makes the xorg-gtest variables available to all subdirectories and not just the ones under tests/.

This change also refreshes some distro patches. A new option was added to not auto-load the ccp plugin, and also modifies one of the tests depending on a modified setting value.

(LP: #1120009)

Description of the change

Enable xorg-gtest tests by default and build in CI.

This change also refactors a number of parts of the code to find and build the tests. It puts the logic to find Google Test and Xorg GTest in their own cmake files, and also splits the find logic with the build logic. Finally, it makes the xorg-gtest variables available to all subdirectories and not just the ones under tests/

This change also refreshes some distro patches. A new option was added to not auto-load the ccp plugin, and also modifies one of the tests depending on a modified setting value.

(LP: #1120009)

To post a comment you must log in.
Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

More useful tests can only be good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote : Posted in a previous version of this proposal

Cool, looks good to me. Yay for fixing that mis leading warning message

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Argh. I thought we upgraded xorg-gtest in the distro to 0.8.

I'll have to leave this as WIP until that happens.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Okay, I've been informed that we have 0.7 in the distro, and that is only in raring. I've bumped the required version and mmrazik has bumped the distro in CI.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Seems like the tests are failing because the xserver isn't sending back the events we expect. Debugging this now.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Okay, I think the cause of the failures was the ccp-autoloading distro patch. That was just causing problems with the child process, but our test framework doesn't handle the case of a crashing compiz yet. I've filed bug 1124843 about that.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

(Resubmitted with the correct candidate branch)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Martin Mrazik (mrazik) wrote : Posted in a previous version of this proposal

I killed the previous job and triggered a new one to check if disabling tests on clang works.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

No problem, thanks for the update.

On Thu, Feb 14, 2013 at 4:46 PM, Martin Mrazik
<email address hidden> wrote:
> I killed the previous job and triggered a new one to check if disabling tests on clang works.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1120009.3/+merge/148371
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Okay, I've really gone thermonuclear on this. There was a race condition causing the memchecked tests to fail more often, and it turns out the XOpenDisplay here is inherently racey for some reason. So I've added a method to brute-force until we get a connection rather than just dying randomly. Its ugly, but it'll do. Also if you close the stdout weird stuff happens, so we're running the tests through a wrapper to not do that.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

The good news is that the xorg-gtest tests are now passing. Corrected a small error in the non xorg-gtest case.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

LGTM.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

The next "fun" hitch is that xorg-gtest doesn't build on arm. So we'll have to disable it there. sucks.

Revision history for this message
Sam Spilsbury (smspillaz) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
MC Return (mc-return) wrote :

Text conflict in debian/changelog.

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

re-acking after fixing that conflict

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Re-acked - some new code that came in wouldn't build in the pbuilders, that's fixed now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches