Merge lp:~vanvugt/mir/fix-MirClientSurfaceTest-fragility into lp:mir

[Daniel van Vugt]

Fix fragile MirClientSurfaceTest which started failing when I made
almost unrelated changes to the client library.

Turns out the test is at fault for assuming its mock objects are in
the surface map when the test itself forgot to put them there.

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

FAILED: Continuous integration, rev:3874
https://mir-jenkins.ubuntu.com/job/mir-ci/2357/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/3072/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3138
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3130
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3130
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3130
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3101
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3101/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/3101
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3101/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3101/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3101
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3101/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/3101
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3101/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/3101
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3101/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Hmm, needing to duplicating code from MirConnection::stream_created() doesn't seem the most change-proof way of writing a *unit* test. I think the abstractions or the test (or, more likely, both) are wrong.

But this MP doesn't purport to fix the mess, just to make it work better. So OK.

review: Approve

[Daniel van Vugt (vanvugt)]

^^^
Jenkins failure is bug 1646558

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

review: Approve (continuous-integration)

=== modified file 'tests/unit-tests/client/test_client_mir_surface.cpp'
--- tests/unit-tests/client/test_client_mir_surface.cpp	2016-12-08 04:08:06 +0000
+++ tests/unit-tests/client/test_client_mir_surface.cpp	2016-12-12 03:28:06 +0000
@@ -498,10 +498,11 @@
 {
     using namespace testing;
     auto mock_stream = std::make_shared<mtd::MockMirBufferStream>(); 
+    mir::frontend::BufferStreamId const mock_stream_id(2);
     auto mock_input_platform = std::make_shared<NiceMock<MockClientInputPlatform>>();
     ON_CALL(*mock_input_platform, create_input_receiver(_,_,_))
         .WillByDefault(Return(std::make_shared<mt::TestDispatchable>([]{})));
-    ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mir::frontend::BufferStreamId(2)));
+    ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mock_stream_id));
 
     geom::Size size(120, 124);
     EXPECT_CALL(*mock_stream, set_size(size));
@@ -509,16 +510,18 @@
 
     MirSurface surface{connection.get(), *client_comm_channel, nullptr,
         mock_stream, mock_input_platform, spec, surface_proto, wh};
+    surface_map->insert(mock_stream_id, mock_stream);
     surface.handle_event(*ev);
-    surface_map->erase(mir::frontend::BufferStreamId(2));
+    surface_map->erase(mock_stream_id);
 }
 
 TEST_F(MirClientSurfaceTest, resizes_streams_and_calls_callback_if_customized_streams)
 {
     using namespace testing;
     auto mock_stream = std::make_shared<NiceMock<mtd::MockMirBufferStream>>();
+    mir::frontend::BufferStreamId const mock_stream_id(2);
     auto mock_input_platform = std::make_shared<NiceMock<MockClientInputPlatform>>();
-    ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mir::frontend::BufferStreamId(2)));
+    ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mock_stream_id));
     ON_CALL(*mock_input_platform, create_input_receiver(_,_,_))
         .WillByDefault(Return(std::make_shared<mt::TestDispatchable>([]{})));
 
@@ -532,9 +535,10 @@
     std::vector<ContentInfo> info =
         { ContentInfo{ geom::Displacement{0,0}, 2, geom::Size{1,1}} };
     spec.streams = info;
+    surface_map->insert(mock_stream_id, mock_stream);
     surface.modify(spec)->wait_for_all();
     surface.handle_event(*ev);
-    surface_map->erase(mir::frontend::BufferStreamId(2));
+    surface_map->erase(mock_stream_id);
 }
 
 TEST_F(MirClientSurfaceTest, parameters_are_unhooked_from_stream_sizes)