Merge lp:~afrantzis/mir/fix-or-workaround-1523621 into lp:mir

[Alexandros Frantzis]

tests: Ensure client surface is in a steady state in tests using a nested setup (LP: #1523621)

tests: Ensure client surface is in a steady state in tests using a nested setup

Due to the many layers of asynchronous processing when using a nested setup, it's difficult to guarantee that the client surface will be in a steady state (visible, focused, ready for input etc) when we need it to. We work around this issue by using an implicit test to check for a steady state: we start sending input events from the device, and wait until we start receiving them on the client side. We have already employed this tactic in test_nested_input.cpp.

Note: Very rarely the new code fails, i.e., we never get input events while waiting for the steady state. This is not related to the test itself, but indicates a race somewhere in our infrastructure. In my testing the failure rate is at least two orders of magnitude lower than the current state of the code, so I think it's worth it over what we have now.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3764
https://mir-jenkins.ubuntu.com/job/mir-ci/1974/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2522
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2585
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2577
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2551/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2551
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2551/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1974/rebuild

review: Approve (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3765
https://mir-jenkins.ubuntu.com/job/mir-ci/1975/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2524
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2587
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2579
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2579
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2579
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2553
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2553/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2553
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2553/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2553
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2553/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2553
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2553/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2553
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2553/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2553
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2553/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1975/rebuild

review: Approve (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Sounds like an improvement to me.

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp	2016-10-12 06:03:15 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp	2016-10-13 14:31:18 +0000
@@ -54,6 +54,8 @@
 #include "mir/test/doubles/nested_mock_egl.h"
 #include "mir/test/fake_shared.h"
 
+#include <linux/input.h>
+#include <atomic>
 #include <future>
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
@@ -73,23 +75,6 @@
 
 namespace
 {
-struct InputReadyServerStatusListener : mir::ServerStatusListener
-{
-    std::shared_ptr<std::promise<void>> const input_ready;
-    InputReadyServerStatusListener(std::shared_ptr<std::promise<void>> const& promise)
-        : input_ready(promise)
-    {}
-
-    void paused() override {}
-    void resumed() override {}
-    void started() override {}
-    void ready_for_user_input() override
-    {
-        input_ready->set_value();
-    }
-    void stop_receiving_input() override {}
-};
-
 struct MockSessionMediatorReport : mf::SessionMediatorReport
 {
     MockSessionMediatorReport()
@@ -426,21 +411,31 @@
             { return std::make_shared<NiceMock<MockDisplayConfigurationPolicy>>(); });
     }
 
-    void wait_until_ready()
+    void wait_until_surface_ready(MirSurface* surface)
     {
-        auto future_status = ready_for_input->get_future().wait_for(10s);
-
-        if (future_status != std::future_status::ready)
-            BOOST_THROW_EXCEPTION(std::runtime_error("Nested Server never started"));
+        mir_surface_set_event_handler(surface, wait_for_key_a_event, this);
+
+        auto const dummy_events_received = mt::spin_wait_for_condition_or_timeout(
+            [this]
+            {
+                if (surface_ready) return true;
+                fake_input_device->emit_event(
+                    mi::synthesis::a_key_down_event().of_scancode(KEY_A));
+                fake_input_device->emit_event(
+                    mi::synthesis::a_key_up_event().of_scancode(KEY_A));
+                return false;
+            },
+            std::chrono::seconds{5});
+
+        EXPECT_TRUE(dummy_events_received);
+
+        mir_surface_set_event_handler(surface, nullptr, nullptr);
     }
 
 protected:
     NestedMirRunner(std::string const& connection_string, bool)
         : mtf::HeadlessNestedServerRunner(connection_string)
     {
-        server.override_the_server_status_listener([this]
-            { return std::make_shared<InputReadyServerStatusListener>(ready_for_input); });
-
         server.override_the_host_lifecycle_event_listener([this]
             { return the_mock_host_lifecycle_event_listener(); });
 
@@ -455,12 +450,27 @@
     }
 
 private:
+    static void wait_for_key_a_event(MirSurface*, MirEvent const* ev, void* context)
+    {
+        auto const nmr = static_cast<NestedMirRunner*>(context);
+        if (mir_event_get_type(ev) == mir_event_type_input)
+        {
+            auto const iev = mir_event_get_input_event(ev);
+            if (mir_input_event_get_type(iev) == mir_input_event_type_key)
+            {
+                auto const kev = mir_input_event_get_keyboard_event(iev);
+                if (mir_keyboard_event_scan_code(kev) == KEY_A)
+                    nmr->surface_ready = true;
+            }
+        }
+    }
+
     mir::CachedPtr<MockHostLifecycleEventListener> mock_host_lifecycle_event_listener;
     mir::CachedPtr<MockDisplayConfigurationPolicy> mock_display_configuration_policy_;
-    std::shared_ptr<std::promise<void>> ready_for_input = std::make_shared<std::promise<void>>();
-    mir::UniqueModulePtr<mtf::FakeInputDevice> fake_device{mtf::add_fake_input_device(mi::InputDeviceInfo{
+    mir::UniqueModulePtr<mtf::FakeInputDevice> fake_input_device{mtf::add_fake_input_device(mi::InputDeviceInfo{
         "test-devce", "test-device",
         mi::DeviceCapability::pointer | mi::DeviceCapability::keyboard | mi::DeviceCapability::alpha_numeric})};
+    std::atomic<bool> surface_ready{false};
 };
 
 struct NestedServer : mtf::HeadlessInProcessServer
@@ -948,12 +958,12 @@
     NestedMirRunner nested_mir{new_connection()};
 
     ClientWithAPaintedSurfaceAndABufferStream client(nested_mir);
+    nested_mir.wait_until_surface_ready(client.surface);
+
     auto const mock_cursor = the_mock_cursor();
 
     server.the_cursor_listener()->cursor_moved_to(489, 9);
 
-    nested_mir.wait_until_ready();
-
     // FIXME: In this test setup the software cursor will trigger scene_changed() on show(...).
     // Thus a new frame will be composed. Then a "FramePostObserver" in basic_surface.cpp will
     // react to the frame_posted callback by setting the cursor buffer again via show(..)
@@ -993,12 +1003,12 @@
     NestedMirRunner nested_mir{new_connection()};
 
     ClientWithAPaintedSurface client(nested_mir);
+    nested_mir.wait_until_surface_ready(client.surface);
 
     server.the_cursor_listener()->cursor_moved_to(489, 9);
 
-    nested_mir.wait_until_ready();
     // wait for the initial cursor show call..
-    condition.wait_for(long_timeout);
+    EXPECT_TRUE(condition.wait_for(long_timeout));
     condition.reset();
 
     // FIXME: In this test setup the software cursor will trigger scene_changed() on show(...).
@@ -1035,10 +1045,10 @@
 
     ClientWithAPaintedSurfaceAndABufferStream client(nested_mir);
     auto const mock_cursor = the_mock_cursor();
+    nested_mir.wait_until_surface_ready(client.surface);
 
     server.the_cursor_listener()->cursor_moved_to(489, 9);
 
-    nested_mir.wait_until_ready();
     Mock::VerifyAndClearExpectations(mock_cursor.get());
 
     // FIXME: In this test setup the software cursor will trigger scene_changed() on show(...).
@@ -1076,6 +1086,7 @@
     NestedMirRunner nested_mir{new_connection()};
 
     ClientWithAPaintedSurfaceAndABufferStream client(nested_mir);
+    nested_mir.wait_until_surface_ready(client.surface);
     auto const mock_cursor = the_mock_cursor();
 
     server.the_cursor_listener()->cursor_moved_to(489, 9);
@@ -1516,8 +1527,8 @@
 {
     using namespace std::chrono_literals;
     NestedMirRunner nested_mir{new_connection()};
-    nested_mir.wait_until_ready();
     ClientWithAPaintedSurface client(
         nested_mir, display_geometry.front().size, mir_pixel_format_xbgr_8888);
+    nested_mir.wait_until_surface_ready(client.surface);
     EXPECT_TRUE(nested_mir.passthrough_tracker->wait_for_passthrough_frames(1, 5s));
 }