Merge lp:~vanvugt/mir/fix-1522031 into lp:mir

[Daniel van Vugt]

Avoid running timing-sensitive performance tests under Valgrind, as
they are likely to fail (and we don't care). (LP: #1522031)

[Alexandros Frantzis (afrantzis)]

We shouldn't block these tests from being run under valgrind if someone wants to do so. I would prefer just to not run them under valgrind for our default test runs.

We can do this by adding the tests to the test_no_memcheck_filter at https://bazaar.launchpad.net/~mir-team/mir/development-branch/view/head:/cmake/MirCommon.cmake#L66

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

I wonder though, if these tests should be moved to another suite altogether. Perhaps mir_performance_tests or a new suite?

[Alan Griffiths (alan-griffiths)]

How can this compile if valgrind isn't installed?

review: Disapprove

[Alan Griffiths (alan-griffiths)]

> I wonder though, if these tests should be moved to another suite altogether.
> Perhaps mir_performance_tests or a new suite?

Yes, putting performance tests in mir_performance_tests would reduce surprises.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3176
http://jenkins.qa.ubuntu.com/job/mir-ci/5775/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5204
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4110
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5152
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/100/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/104
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/104/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/104
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/104/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5152
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5152/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7682
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25812
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/98/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5775/rebuild

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

Marking tests passed when they don't run seems wrong.

+1 on moving them to mir_performance_tests

review: Needs Fixing

Unmerged revisions

3176. By Daniel van Vugt on 2015-12-04

Avoid running timing-sensitive performance tests under Valgrind, as
they are likely to fail (and we don't care). (LP: #1522031)

=== modified file 'tests/acceptance-tests/test_latency.cpp'
--- tests/acceptance-tests/test_latency.cpp	2015-10-20 03:30:00 +0000
+++ tests/acceptance-tests/test_latency.cpp	2015-12-04 09:24:23 +0000
@@ -26,6 +26,8 @@
 #include "mir/test/doubles/null_display_sync_group.h"
 #include "mir/test/fake_shared.h"
 
+#include <valgrind/valgrind.h>
+
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 #include <deque>
@@ -195,6 +197,14 @@
 
 TEST_F(ClientLatency, triple_buffered_client_uses_all_buffers)
 {
+    if (RUNNING_ON_VALGRIND)
+    {
+        VALGRIND_PRINTF("Running under valgrind so this real-time "
+                        "multi-process performance test will be skipped "
+                        "(not reliably fast enough to be realistic)\n");
+        return;
+    }
+
     using namespace testing;
 
     auto stream = mir_surface_get_buffer_stream(surface);
@@ -231,6 +241,14 @@
 
 TEST_F(ClientLatency, throttled_input_rate_yields_lower_latency)
 {
+    if (RUNNING_ON_VALGRIND)
+    {
+        VALGRIND_PRINTF("Running under valgrind so this real-time "
+                        "multi-process performance test will be skipped "
+                        "(not reliably fast enough to be realistic)\n");
+        return;
+    }
+
     using namespace testing;
 
     int const throttled_input_rate = refresh_rate - 1;