Merge lp:~vanvugt/mir/fix-1375211-v2 into lp:mir

[Daniel van Vugt]

Fix input receiver tests with mocked clocks accidentally taking real
time (not mocked time) to complete (LP: #1375211)

Version 2: This one is more tolerant of CI glitches and slowdowns, which should avoid spurious test failures.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1959
http://jenkins.qa.ubuntu.com/job/mir-ci/1732/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2015
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2022
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1956
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/70
        deb: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/70/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/893
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/893/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2968
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14101

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

24 +TEST_F(AndroidInputReceiverSetup, zero_timeout_is_less_than_1ms)

More robust than the last version, but still somewhat fragile. Let's see if this does the trick for CI/autolanding. Hopefully we will be able to properly mock time in the input subsystem in the future.

49 if (!is_sometimes_less_than_1ms &&
50 + duration < std::chrono::milliseconds(1))
51 + is_sometimes_less_than_1ms = true;

Could be "is_sometimes_less_than_1ms |= (duration < std::chrono::milliseconds(1));"

review: Approve

[Alan Griffiths (alan-griffiths)]

Making the test less likely to fail doesn't make it completely reliable.

I'm not convinced that the test is testing anything that we can (or need) to guarantee - can we just get rid of it?

review: Needs Information

[Daniel van Vugt (vanvugt)]

Yeah my original fix had no test. But I like the idea of having some regression test. Especially since I've messed it up in the past.

[Alan Griffiths (alan-griffiths)]

> Yeah my original fix had no test. But I like the idea of having some
> regression test. Especially since I've messed it up in the past.

I understand the desire to validate but this specific test is effectively testing that the code runs "fast enough" on the test machine. With unpredictable loads, valgrind and slow hardware this has to be uncertain.

We don't need many tests that "work 99.9% of the time" to create annoying problems. (It has cost us time to identify and eliminate ones that have crept in previously and we don't need to let more in.)

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1961
http://jenkins.qa.ubuntu.com/job/mir-ci/1742/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/2032
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/2039
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1969
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/80
        deb: http://jenkins.qa.ubuntu.com/job/mir-utopic-amd64-ci/80/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/906
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/906/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2977
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14171

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Fixed, per Alan's request.

[Alexandros Frantzis (afrantzis)]

Still OK.

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

=== modified file 'src/common/input/android/android_input_receiver.cpp'
--- src/common/input/android/android_input_receiver.cpp	2014-10-03 03:02:09 +0000
+++ src/common/input/android/android_input_receiver.cpp	2014-10-06 03:05:53 +0000
@@ -153,7 +153,10 @@
         // But the batch won't get flushed until the next frame interval,
         // so be sure to use a non-zero sleep time to avoid spinning the CPU
         // for the whole interval...
-        reduced_timeout = std::chrono::milliseconds(1);
+
+        // During tests with mocked clocks we may already have zero...
+        if (reduced_timeout != std::chrono::milliseconds::zero())
+            reduced_timeout = std::chrono::milliseconds(1);
     }
 
     auto result = looper->pollOnce(reduced_timeout.count());