Merge lp:~raof/mir/hate-hate-hate-on-the-cmake into lp:mir

[Chris Halse Rogers]

Make memcheck-test work with DISABLE_GTEST_TEST_DISCOVERY (ie: on the CI infrastructure)

memcheck-test is a test to check that valgrind fails with an error when run against a program that generates a known-bad memory access. So, for the test to pass, valgrind needs to return 1, which it does, but this normally indicates that a test has *failed*, so we need to invert the test success condition.

We were trying to do this by effectively embedding a shell script into CMakeList.txt, but
CMake's escaping rules make this roughly impossible, or at least mind-bendingly awkward.

Use a real, if trivial, shell script to invert the return value instead.

Fixes https://bugs.launchpad.net/mir/+bug/1291876

Make memcheck-test work with DISABLE_GTEST_TEST_DISCOVERY (ie: on the CI infrastructure)

The trick, of course, is to use CMake as little as possible

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1473
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1064/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1228
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1226
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/810
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/796
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/796/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/801
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/801/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/811
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/811/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/755
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4749

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1064/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

The need for fail_on_success.sh sounds suspicious. Can we not make valgrind emit correct error codes?!

review: Needs Information

[Chris Halse Rogers (raof)]

Not really, no. We'd want it to emit an error code on _success_, and - surprisingly enough - it doesn't have an option to do that :)

[Daniel van Vugt (vanvugt)]

What about valgrind --error-exitcode=1 ?

review: Needs Information

[Chris Halse Rogers (raof)]

Does exactly the *opposite* of what we want. (And we already have that option set)

[Chris Halse Rogers (raof)]

Specifically: test-memcheck is a test to test that valgrind *fails with an error* when run against a program that generates a known-bad memory access. So, for the test to pass, valgrind needs to return 1; which it does, but this normally indicates that a test has *failed*, so we need to invert the test success condition.

Which fail_on_success.sh does. Is it's name confusing?

[Daniel van Vugt (vanvugt)]

Fair enough. Just trying to establish the reasoning as it's not explained in the commit message or description...

review: Approve

[Alan Griffiths (alan-griffiths)]

Yeah "fail_on_success.sh" is about implementation, not intent. Maybe "assert_fails.sh" would be better?

review: Approve

[Kevin DuBois (kdub)]

okay

review: Approve

=== modified file 'cmake/MirCommon.cmake'
--- cmake/MirCommon.cmake	2014-03-06 06:05:17 +0000
+++ cmake/MirCommon.cmake	2014-03-13 13:36:06 +0000
@@ -82,7 +82,15 @@
 function (mir_add_memcheck_test)
   if (ENABLE_MEMCHECK_OPTION)
       if(DISABLE_GTEST_TEST_DISCOVERY)
-          ADD_TEST("memcheck-test" "sh" "-c" "${VALGRIND_EXECUTABLE} ${VALGRIND_ARGS} ${CMAKE_BINARY_DIR}/mir_gtest/mir_test_memory_error; if [ $? != 0 ]; then exit 0; else exit 1; fi")
+	add_custom_target(
+	  memcheck_test ALL
+	)
+	ADD_TEST("memcheck-test" ${CMAKE_BINARY_DIR}/mir_gtest/fail_on_success.sh ${VALGRIND_EXECUTABLE} ${VALGRIND_ARGS} ${CMAKE_BINARY_DIR}/mir_gtest/mir_test_memory_error)
+	add_dependencies(
+	  memcheck_test
+
+	  mir_test_memory_error
+	)
       else()
         add_custom_target(
           memcheck_test ALL
 
=== modified file 'cmake/src/mir/CMakeLists.txt'
--- cmake/src/mir/CMakeLists.txt	2013-03-13 04:54:15 +0000
+++ cmake/src/mir/CMakeLists.txt	2014-03-13 13:36:06 +0000
@@ -17,3 +17,8 @@
   mir_test_memory_error PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/mir_gtest
 )
+
+file(INSTALL ${CMAKE_CURRENT_SOURCE_DIR}/fail_on_success.sh
+  DESTINATION ${CMAKE_BINARY_DIR}/mir_gtest
+  USE_SOURCE_PERMISSIONS
+)
 
=== added file 'cmake/src/mir/fail_on_success.sh'
--- cmake/src/mir/fail_on_success.sh	1970-01-01 00:00:00 +0000
+++ cmake/src/mir/fail_on_success.sh	2014-03-13 13:36:06 +0000
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+$@
+
+if [ $? -eq 0 ] ; then
+    exit 1;
+fi
+exit 0;