Merge lp:~michihenning/unity/valgrind-tests into lp:unity/phablet

[Michi Henning]

Added valgrind target to cmake. To suppress valgrind testing for tests that don't need it, add the test to be omitted to CTestCustom.cmake.in.

Added valgrind target to cmake. To suppress valgrind testing for tests that don't need it, add the test to be omitted to CTestCustom.cmake.in.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:601
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/541/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/334
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/542
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/542/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/541
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/417
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/417/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1031

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/541/rebuild

review: Approve (continuous-integration)

[Michał Sawicz (saviq)]

There's no need for the valgrind build-dep, it's only needed when you run the valgrind target, no?

review: Needs Information

[Jussi Pakkanen (jpakkane)]

Instead of suppressing g_ memory functions you might want to do this:

http://stackoverflow.com/questions/4254610/valgrind-reports-memory-possibly-lost-when-using-glib-data-types

[Michi Henning (michihenning)]

> There's no need for the valgrind build-dep, it's only needed when you run the
> valgrind target, no?

OK, I didn't know what our policy was for packages that are not essential for the build and only needed for testing. I've removed the dependency on valgrind.

[Michi Henning (michihenning)]

> Instead of suppressing g_ memory functions you might want to do this:
>
> http://stackoverflow.com/questions/4254610/valgrind-reports-memory-possibly-
> lost-when-using-glib-data-types

That would be nice. My question is how to set the environment variables. Currently, I have:

 set(MEMORYCHECK_COMMAND_OPTIONS
        "--suppressions=${CMAKE_SOURCE_DIR}/valgrind-suppress --leak-check=full --num-callers=40 --error-exitcode=3"
 )
 add_custom_target(valgrind DEPENDS NightlyMemCheck)

This is pretty much straight from the cmake doc. The cmake doc is silent on how to run the memory check with the environment variables set. The cmake set(ENV{VAR} "value") construct doesn't work because that runs at configure time, not build time.

Do you have any suggestions? I'd rather not write yet another script...

[Michał Sawicz (saviq)]

W dniu 22.04.2013 13:41, Michi Henning pisze:
> That would be nice. My question is how to set the environment variables. Currently, I have:

Hey,

http://www.cmake.org/cmake/help/v2.8.9/cmake.html#command:set_target_properties

Should help.
--
Michał Sawicz <email address hidden>
Canonical Services Ltd.

[Michał Sawicz (saviq)]

W dniu 22.04.2013 13:43, Michał Sawicz pisze:
> Should help.

And since the doc isn't really verbose about the topic:

add_custom_target(foo)
set_target_properties(foo PROPERTIES ENVIRONMENT "VAR=value")

--
Michał Sawicz <email address hidden>
Canonical Services Ltd.

[Michi Henning (michihenning)]

> And since the doc isn't really verbose about the topic:
>
> add_custom_target(foo)
> set_target_properties(foo PROPERTIES ENVIRONMENT "VAR=value")

Thanks for that. According to the doc, the ENVIRONMENT property applies only to tests, not targets.

I just tried this:

    add_custom_target(valgrind DEPENDS NightlyMemCheck)
    set_target_properties(valgrind PROPERTIES ENVIRONMENT "G_DEBUG=gc-friendly")
    set_target_properties(valgrind PROPERTIES ENVIRONMENT "G_SLICE=always-malloc")

but that doesn't work. When I try to use NightlyMemCheck instead of the valgrind to attach the property, it fails for both set_target_properties and set_tests_properties :-(

[Michał Sawicz (saviq)]

W dniu 22.04.2013 14:02, Michi Henning pisze:
> Thanks for that. According to the doc, the ENVIRONMENT property applies only to tests, not targets.

Oh?

In that case I defer to Jussi...
--
Michał Sawicz <email address hidden>
Canonical Services Ltd.

[Jussi Pakkanen (jpakkane)]

If you want to

a) not have the environment variables set when running regular tests
b) have them set when running Valgrind

then the only way I can think of is to set them when invoking make/ninja. Something like this:

G_DEBUG=.. G_SLICE=.. make valgrind

Since valgrind is a non-standard target, you need to have a custom invocation command somewhere in Jenkins. Just change that.

The only alternative I can think of goes into the realm of unspeakable evil along the lines of

add_custom_command(valgrind COMMAND G_DEBUG=.. G_SLICE=.. make NightlyMemCheck)

I'm not sure if this works (whether CMake passes the entire thing to shell without thinking) but I sincerely hope that is does not as this is awful in so many different ways.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:602
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/697/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/514
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/699
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/699/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/697
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/573
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/573/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/577
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1228

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/697/rebuild

review: Approve (continuous-integration)

[Michi Henning (michihenning)]

> If you want to
>
> a) not have the environment variables set when running regular tests
> b) have them set when running Valgrind
>
> then the only way I can think of is to set them when invoking make/ninja.
> Something like this:
>
> G_DEBUG=.. G_SLICE=.. make valgrind
>
> Since valgrind is a non-standard target, you need to have a custom invocation
> command somewhere in Jenkins. Just change that.

Hmmm... I'd rather not have to type all that every time I want to run the tests with leak checking.

> The only alternative I can think of goes into the realm of unspeakable evil
> along the lines of

Yes, not pretty.

So, what to do now? We can leave it as is and put up with the suppression we'll have to add periodically. Or we can throw out the the NightlyMemCheck thing and write our own custom target with a Python wrapper to set the variables and then run the executable with valgrind. But doing that won't be trivial either, I suspect...

[Jussi Pakkanen (jpakkane)]

I would merge this as-is. It's better than nothing and there does not seem to be an obvious clean solution for the corner cases. If a good solution pops up, we can fix it later.

[Michał Sawicz (saviq)]

@Jussi can you vote / approve in that case?

[Jussi Pakkanen (jpakkane)]

Whoops, sorry.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/241/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/554
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/241
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/241/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-autolanding/241
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/175
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/175/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/175
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1271

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

autolanding got stuck

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/243/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/564
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/243
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/243/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-autolanding/243
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/177
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/177/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/177
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1279

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-04-18 18:21:23 +0000
+++ CMakeLists.txt	2013-04-22 11:43:26 +0000
@@ -43,6 +43,23 @@
 add_custom_target(cppcheck COMMAND cppcheck --enable=all -q --error-exitcode=2
                                        ${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/tests)
 
+#
+# Definitions for testing with valgrind.
+#
+
+configure_file(CTestCustom.cmake.in CTestCustom.cmake) # Tests in CTestCustom.cmake are skipped for valgrind
+
+find_program(MEMORYCHECK_COMMAND NAMES valgrind)
+if (MEMORYCHECK_COMMAND)
+    set(MEMORYCHECK_COMMAND_OPTIONS
+        "--suppressions=${CMAKE_SOURCE_DIR}/valgrind-suppress --leak-check=full --num-callers=40 --error-exitcode=3"
+    )
+    add_custom_target(valgrind DEPENDS NightlyMemCheck)
+else()
+    message(WARNING "Cannot find valgrind: valgrind target will not be available")
+endif()
+
+
 include(FindPkgConfig)
 find_package(Qt5Qml)
 find_package(Qt5Quick)
@@ -104,6 +121,7 @@
 set(LIBS ${UNITY_API_LIB} ${OTHER_API_LIBS})
 
 # Tests
+include(CTest)
 enable_testing()
 
 # Autopilot tests

=== added file 'CTestCustom.cmake.in'
--- CTestCustom.cmake.in	1970-01-01 00:00:00 +0000
+++ CTestCustom.cmake.in	2013-04-22 11:43:26 +0000
@@ -0,0 +1,16 @@
+#
+# Tests listed here will not be run by the valgrind target.
+#
+
+SET(CTEST_CUSTOM_MEMCHECK_IGNORE
+    cleanincludes
+    testAnimationControllerWithSignals
+    testCarousel
+    testClock
+    testCrossFadeImage
+    testIndicatorItem
+    testMathLocal
+    testRatingStars
+    testSideStage
+    testTimeLocal
+    whitespace)

=== added file 'valgrind-suppress'
--- valgrind-suppress	1970-01-01 00:00:00 +0000
+++ valgrind-suppress	2013-04-22 11:43:26 +0000
@@ -0,0 +1,13 @@
+{
+    g_main_leak
+    Memcheck:Leak
+    fun:memalign
+    fun:posix_memalign
+    obj:/lib/x86_64-linux-gnu/libglib-2.0.so.0.3600.0
+    fun:g_slice_alloc
+    fun:g_slice_alloc0
+    fun:g_thread_self
+    fun:g_main_context_acquire
+    fun:g_main_context_push_thread_default
+    ...
+}