Merge lp:~afrantzis/mir/fix-1462033-NestedInput-test-race into lp:mir

[Alexandros Frantzis]

tests: Fix race in NestedInput test (LP: #1462033)

Because we are testing a nested setup, it's difficult to guarantee
that the nested framebuffer surface (and consenquently the client surface
contained in it) is going to be ready (i.e., exposed and focused) to receive
events when we send them. We work around this issue by first sending some
dummy events and waiting until we receive one of them.

tests: Fix race in NestedInput test (LP: #1462033)

Because we are testing a nested setup, it's difficult to guarantee
that the nested framebuffer surface (and consenquently the client surface
contained in it) is going to be ready (i.e., exposed and focused) to receive
events when we send them. We work around this issue by first sending some
dummy events and waiting until we receive one of them.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2660
http://jenkins.qa.ubuntu.com/job/mir-ci/4095/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2842
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/355
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2790/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/251
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/251/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2790
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2790/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5622/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21187

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

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

"Temporary failure resolving 'ports.ubuntu.com'". Rebuilding.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2660
http://jenkins.qa.ubuntu.com/job/mir-ci/4101/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2848
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/361
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2796/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/257
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/257/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2796
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2796/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5628/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21195

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

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

49+ EXPECT_CALL(*nested_mir.mock_event_filter, handle(mt::KeyOfScanCode(KEY_A))).
50+ Times(AtLeast(0)).
51+ WillRepeatedly(DoAll(InvokeWithoutArgs(increase_key_a_events), Return(true)));
Why not "Times(AtLeast(1))" ?

review: Needs Information

[Alexandros Frantzis (afrantzis)]

> Why not "Times(AtLeast(1))" ?

Fixed, thanks.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2661
http://jenkins.qa.ubuntu.com/job/mir-ci/4103/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2851
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/364
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2799
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/259
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/259/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2799
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2799/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5631
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21201

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

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

OK.

review: Approve

[Daniel van Vugt (vanvugt)]

Not an easy problem to solve, and impossible to solve reliably when you add a real human into the mix... Because there's a delay in the user seeing the screen and reacting, still sometimes you will click on something that's not there any more.

Seems reasonable.

review: Approve

=== modified file 'tests/acceptance-tests/test_nested_input.cpp'
--- tests/acceptance-tests/test_nested_input.cpp	2015-05-19 21:34:34 +0000
+++ tests/acceptance-tests/test_nested_input.cpp	2015-06-15 17:38:30 +0000
@@ -28,6 +28,7 @@
 #include "mir_test_framework/any_surface.h"
 #include "mir_test_doubles/nested_mock_egl.h"
 #include "mir_test/event_factory.h"
+#include "mir_test/event_matchers.h"
 #include "mir_test/fake_shared.h"
 #include "mir_test/wait_condition.h"
 #include "mir_test/spin_wait.h"
@@ -38,6 +39,7 @@
 #include <gmock/gmock.h>
 
 #include <linux/input.h>
+#include <atomic>
 
 namespace mi = mir::input;
 namespace mis = mi::synthesis;
@@ -112,15 +114,6 @@
         connection = mir_connect_sync(connect_string.c_str(), __PRETTY_FUNCTION__);
         surface = mtf::make_any_surface(connection);
         mir_buffer_stream_swap_buffers_sync(mir_surface_get_buffer_stream(surface));
-
-        bool became_exposed_and_focused = mir::test::spin_wait_for_condition_or_timeout(
-        [this]
-        {
-            return mir_surface_get_visibility(surface) == mir_surface_visibility_exposed
-                && mir_surface_get_focus(surface) == mir_surface_focused;
-        },
-        std::chrono::seconds{10});
-        EXPECT_TRUE(became_exposed_and_focused);
     }
 
     ~ExposedSurface()
@@ -143,12 +136,36 @@
 {
     NestedServerWithMockEventFilter nested_mir{new_connection()};
     ExposedSurface client(nested_mir.new_connection());
+    std::atomic<int> num_key_a_events{0};
+
+    auto const increase_key_a_events = [&num_key_a_events] { ++num_key_a_events; };
 
     InSequence seq;
-    EXPECT_CALL(*nested_mir.mock_event_filter, handle(_)).Times(AtLeast(1)).WillOnce(Return(true));
-    EXPECT_CALL(*nested_mir.mock_event_filter, handle(_)).Times(AtLeast(1)).WillOnce(
-            DoAll(mt::WakeUp(&all_events_received), Return(true)));
-
+    EXPECT_CALL(*nested_mir.mock_event_filter, handle(mt::KeyOfScanCode(KEY_A))).
+        Times(AtLeast(1)).
+        WillRepeatedly(DoAll(InvokeWithoutArgs(increase_key_a_events), Return(true)));
+
+    EXPECT_CALL(*nested_mir.mock_event_filter, handle(mt::KeyOfScanCode(KEY_RIGHTSHIFT))).
+        Times(2).
+        WillOnce(Return(true)).
+        WillOnce(DoAll(mt::WakeUp(&all_events_received), Return(true)));
+
+    // Because we are testing a nested setup, it's difficult to guarantee
+    // that the nested framebuffer surface (and consenquently the client surface
+    // contained in it) is going to be ready (i.e., exposed and focused) to receive
+    // events when we send them. We work around this issue by first sending some
+    // dummy events and waiting until we receive one of them.
+    auto const dummy_events_received = mt::spin_wait_for_condition_or_timeout(
+        [&]
+        {
+            if (num_key_a_events > 0) return true;
+            fake_keyboard->emit_event(mis::a_key_down_event().of_scancode(KEY_A));
+            fake_keyboard->emit_event(mis::a_key_up_event().of_scancode(KEY_A));
+            return false;
+        },
+        std::chrono::seconds{5});
+
+    EXPECT_TRUE(dummy_events_received);
 
     fake_keyboard->emit_event(mis::a_key_down_event().of_scancode(KEY_RIGHTSHIFT));
     fake_keyboard->emit_event(mis::a_key_up_event().of_scancode(KEY_RIGHTSHIFT));