Merge lp:~alan-griffiths/mir/workaround-1200236 into lp:~mir-team/mir/trunk

[Alan Griffiths]

tests: rework test to prevent test input events being missed before handler is registered

tests: rework test to prevent test input events being missed before handler is registered

[Alan Griffiths (alan-griffiths)]

This is only a workaround for bug-1200236 as I think the issue fixed here should result in intermittent test failure, not in a hang.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:845
http://jenkins.qa.ubuntu.com/job/mir-ci/987/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1279
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1164
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/225
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/225/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/623

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

looks ok, but it really is just a workaround for the test. We could use the 'pipe' mechanism in the swapinterval of first-frame-sync tests to synchronize at the right points.

Registering the handler in the callback should ensure that the handler is good to go before the server is notified that the surface has been created, so it will work, and it is better than having that bug :)

review: Approve

[Kevin DuBois (kdub)]

> This is only a workaround for bug-1200236 as I think the issue fixed here
> should result in intermittent test failure, not in a hang.

I think the failures I saw were time outs after 1 minute ( events_received.wait_for_at_most_seconds(60); ), so I suppose technically not a hang :)

[Thomas Voß (thomas-voss)]

LGTM.

review: Approve

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2013-07-10 17:06:37 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2013-07-11 13:54:24 +0000
@@ -125,6 +125,20 @@
     {
     }
 
+    virtual void surface_created(MirSurface* new_surface) override
+    {
+        ClientConfig::surface_created(new_surface);
+
+        MirEventDelegate const event_delegate =
+        {
+            handle_input,
+            this
+        };
+
+        // Set this in the callback, not main thread to avoid missing test events
+        mir_surface_set_event_handler(surface, &event_delegate);
+    }
+
     static void handle_input(MirSurface* /* surface */, MirEvent const* ev, void* context)
     {
         auto client = static_cast<InputClient *>(context);
@@ -132,10 +146,8 @@
         client->handler->handle_input(ev);
     }
 
-    virtual void expect_input(mt::WaitCondition&)
-    {
-    }
-    
+    virtual void expect_input(mt::WaitCondition&) = 0;
+
     virtual MirSurfaceParameters parameters()
     {
         MirSurfaceParameters const request_params =
@@ -161,32 +173,25 @@
             this));
          ASSERT_TRUE(connection != NULL);
 
-         MirEventDelegate const event_delegate =
-         {
-             handle_input,
-             this
-         };
          auto request_params = parameters();
          mir_wait_for(mir_connection_create_surface(connection, &request_params, create_surface_callback, this));
 
-         mir_surface_set_event_handler(surface, &event_delegate);
-
          events_received.wait_for_at_most_seconds(60);
 
          mir_surface_release_sync(surface);
-         
+
          mir_connection_release(connection);
 
          // ClientConfiguration d'tor is not called on client side so we need this
          // in order to not leak the Mock object.
          handler.reset();
     }
-    
+
     std::shared_ptr<MockInputHandler> handler;
     mt::WaitCondition events_received;
-    
+
     std::string const surface_name;
-    
+
     static int const surface_width = 100;
     static int const surface_height = 100;
 };