Mir

Merge lp:~bregma/mir/lp-1195265 into lp:~mir-team/mir/trunk

Proposed by Stephen M. Webb
Status: Merged
Approved by: Stephen M. Webb
Approved revision: no longer in the source branch.
Merged at revision: 854
Proposed branch: lp:~bregma/mir/lp-1195265
Merge into: lp:~mir-team/mir/trunk
Diff against target: 136 lines (+22/-12)
4 files modified
cmake/MirCommon.cmake (+2/-1)
debian/control (+9/-9)
debian/rules (+8/-1)
tests/integration-tests/CMakeLists.txt (+3/-1)
To merge this branch: bzr merge lp:~bregma/mir/lp-1195265
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Didier Roche-Tolomelli Needs Fixing
Kevin DuBois (community) Approve
Alan Griffiths Needs Information
Review via email: mp+174478@code.launchpad.net

Commit message

disable running the integration test suite on arm architecture during the packaging builds (lp: #1195265)

Description of the change

(1) lets packaging builds complete even if valgrind is not installed
(2) disables the running of the integration test suite on arm architecture during the packaging builds because they hang and the results are meaningless (lp: #1195265)

With this change I can get a successful package build on at least my arm hardware.

To post a comment you must log in.
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: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I looked at the bug report, but it isn't clear to me why this is the right approach (as opposed to correcting some tests).

~~~~

8 - add_test(${EXECUTABLE} ${VALGRIND_EXECUTABLE} ${VALGRIND_ARGS} "${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE}")
9 + if(VALGRIND_EXECUTABLE)
10 + add_test(${EXECUTABLE} ${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE})
11 + else()
12 + add_test(${EXECUTABLE} ${VALGRIND_EXECUTABLE} ${VALGRIND_ARGS} "${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE}")
13 + endif()

If valgrind isn't available then ${VALGRIND_EXECUTABLE} and ${VALGRIND_ARGS}" expands to nothing - so why is this needed?

~~~~

55 +if (NOT MIR_DISABLE_INTEGRATION_TESTS STREQUAL "YES")

This doesn't work if is, for example, "ON". Better to write:

if (NOT MIR_DISABLE_INTEGRATION_TESTS)

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

looks good to me

review: Approve
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

as per IRC:
17:28:21 didrocks | bregma: if you: +ifeq ($(DEB_HOST_ARCH),powerpc)
17:28:32 didrocks | bregma: please reenable arch: any in debian/control

Please, feel free to globally approve after that one

review: Needs Fixing
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: Approve (continuous-integration)
Revision history for this message
Stephen M. Webb (bregma) wrote :

Globally approving on didrock's advice.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmake/MirCommon.cmake'
2--- cmake/MirCommon.cmake 2013-07-02 00:19:26 +0000
3+++ cmake/MirCommon.cmake 2013-07-15 18:52:23 +0000
4@@ -29,9 +29,10 @@
5 function (mir_discover_tests EXECUTABLE)
6 if(BUILD_ANDROID OR DISABLE_GTEST_TEST_DISCOVERY)
7 add_test(${EXECUTABLE} ${VALGRIND_EXECUTABLE} ${VALGRIND_ARGS} "${EXECUTABLE_OUTPUT_PATH}/${EXECUTABLE}")
8+
9 if (${ARGC} GREATER 1)
10 set_property(TEST ${EXECUTABLE} PROPERTY ENVIRONMENT ${ARGN})
11- endif ()
12+ endif()
13 else()
14 set(CHECK_TEST_DISCOVERY_TARGET_NAME "check_discover_tests_in_${EXECUTABLE}")
15 set(TEST_DISCOVERY_TARGET_NAME "discover_tests_in_${EXECUTABLE}")
16
17=== modified file 'debian/control'
18--- debian/control 2013-07-10 23:45:31 +0000
19+++ debian/control 2013-07-15 18:52:23 +0000
20@@ -36,7 +36,7 @@
21
22 Package: libmirprotobuf0
23 Section: libs
24-Architecture: i386 amd64 armhf arm64
25+Architecture: any
26 Multi-Arch: same
27 Pre-Depends: ${misc:Pre-Depends}
28 Depends: ${misc:Depends},
29@@ -49,7 +49,7 @@
30
31 Package: libmirprotobuf-dev
32 Section: libdevel
33-Architecture: i386 amd64 armhf arm64
34+Architecture: any
35 Multi-Arch: same
36 Pre-Depends: ${misc:Pre-Depends}
37 Depends: libmirprotobuf0 (= ${binary:Version}),
38@@ -64,7 +64,7 @@
39
40 Package: libmirserver0
41 Section: libs
42-Architecture: i386 amd64 armhf arm64
43+Architecture: any
44 Multi-Arch: same
45 Pre-Depends: ${misc:Pre-Depends}
46 Depends: ${misc:Depends},
47@@ -77,7 +77,7 @@
48
49 Package: mircommon-dev
50 Section: libdevel
51-Architecture: i386 amd64 armhf arm64
52+Architecture: any
53 Multi-Arch: same
54 Pre-Depends: ${misc:Pre-Depends}
55 Depends: ${misc:Depends},
56@@ -89,7 +89,7 @@
57
58 Package: libmirserver-dev
59 Section: libdevel
60-Architecture: i386 amd64 armhf arm64
61+Architecture: any
62 Multi-Arch: same
63 Pre-Depends: ${misc:Pre-Depends}
64 Depends: libmirserver0 (= ${binary:Version}),
65@@ -106,7 +106,7 @@
66
67 Package: libmirclient1
68 Section: libs
69-Architecture: i386 amd64 armhf arm64
70+Architecture: any
71 Multi-Arch: same
72 Pre-Depends: ${misc:Pre-Depends}
73 Depends: ${misc:Depends},
74@@ -119,7 +119,7 @@
75
76 Package: libmirclient-dev
77 Section: libdevel
78-Architecture: i386 amd64 armhf arm64
79+Architecture: any
80 Multi-Arch: same
81 Pre-Depends: ${misc:Pre-Depends}
82 Depends: libmirclient1 (= ${binary:Version}),
83@@ -133,7 +133,7 @@
84 Contains header files required to develop against Mir.
85
86 Package: mir-demos
87-Architecture: i386 amd64 armhf arm64
88+Architecture: any
89 Depends: ${misc:Depends},
90 ${shlibs:Depends},
91 Recommends: mir-doc
92@@ -153,7 +153,7 @@
93 This package installs the mir API documentation.
94
95 Package: mir-test-tools
96-Architecture: i386 amd64 armhf arm64
97+Architecture: any
98 Multi-Arch: same
99 Pre-Depends: ${misc:Pre-Depends}
100 Depends: ${misc:Depends},
101
102=== modified file 'debian/rules'
103--- debian/rules 2013-07-11 12:14:44 +0000
104+++ debian/rules 2013-07-15 18:52:23 +0000
105@@ -21,11 +21,18 @@
106 ifeq ($(DEB_HOST_ARCH),armhf)
107 dh_auto_configure -- \
108 $(COMMON_CONFIGURE_OPTIONS) \
109- -DMIR_PLATFORM=android
110+ -DMIR_PLATFORM=android \
111+ -DMIR_DISABLE_INTEGRATION_TESTS=YES
112+else
113+ifeq ($(DEB_HOST_ARCH),powerpc)
114+ dh_auto_configure -- \
115+ $(COMMON_CONFIGURE_OPTIONS) \
116+ -DMIR_ENABLE_TESTS=NO
117 else
118 dh_auto_configure -- \
119 $(COMMON_CONFIGURE_OPTIONS)
120 endif
121+endif
122
123 # TODO: we'll use a symbol file once mir is abi stable
124 override_dh_makeshlibs:
125
126=== modified file 'tests/integration-tests/CMakeLists.txt'
127--- tests/integration-tests/CMakeLists.txt 2013-07-03 23:42:13 +0000
128+++ tests/integration-tests/CMakeLists.txt 2013-07-15 18:52:23 +0000
129@@ -61,4 +61,6 @@
130 ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
131 )
132
133-mir_discover_tests(integration-tests)
134+if (NOT MIR_DISABLE_INTEGRATION_TESTS)
135+ mir_discover_tests(integration-tests)
136+endif()

Subscribers

People subscribed via source and target branches