Mir

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

  1. fix-MirClientSurfaceTest-fragility
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 3874
Proposed branch: lp:~vanvugt/mir/fix-MirClientSurfaceTest-fragility
Merge into: lp:mir
Diff against target: 49 lines (+8/-4)
1 file modified
tests/unit-tests/client/test_client_mir_surface.cpp (+8/-4)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-MirClientSurfaceTest-fragility
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve
Alan Griffiths Approve
Review via email: mp+312895@code.launchpad.net

Commit message

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.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

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)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

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
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^^^
Jenkins failure is bug 1646558

Revision history for this message
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
498{498{
499 using namespace testing;499 using namespace testing;
500 auto mock_stream = std::make_shared<mtd::MockMirBufferStream>(); 500 auto mock_stream = std::make_shared<mtd::MockMirBufferStream>();
501 mir::frontend::BufferStreamId const mock_stream_id(2);
501 auto mock_input_platform = std::make_shared<NiceMock<MockClientInputPlatform>>();502 auto mock_input_platform = std::make_shared<NiceMock<MockClientInputPlatform>>();
502 ON_CALL(*mock_input_platform, create_input_receiver(_,_,_))503 ON_CALL(*mock_input_platform, create_input_receiver(_,_,_))
503 .WillByDefault(Return(std::make_shared<mt::TestDispatchable>([]{})));504 .WillByDefault(Return(std::make_shared<mt::TestDispatchable>([]{})));
504 ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mir::frontend::BufferStreamId(2)));505 ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mock_stream_id));
505506
506 geom::Size size(120, 124);507 geom::Size size(120, 124);
507 EXPECT_CALL(*mock_stream, set_size(size));508 EXPECT_CALL(*mock_stream, set_size(size));
@@ -509,16 +510,18 @@
509510
510 MirSurface surface{connection.get(), *client_comm_channel, nullptr,511 MirSurface surface{connection.get(), *client_comm_channel, nullptr,
511 mock_stream, mock_input_platform, spec, surface_proto, wh};512 mock_stream, mock_input_platform, spec, surface_proto, wh};
513 surface_map->insert(mock_stream_id, mock_stream);
512 surface.handle_event(*ev);514 surface.handle_event(*ev);
513 surface_map->erase(mir::frontend::BufferStreamId(2));515 surface_map->erase(mock_stream_id);
514}516}
515517
516TEST_F(MirClientSurfaceTest, resizes_streams_and_calls_callback_if_customized_streams)518TEST_F(MirClientSurfaceTest, resizes_streams_and_calls_callback_if_customized_streams)
517{519{
518 using namespace testing;520 using namespace testing;
519 auto mock_stream = std::make_shared<NiceMock<mtd::MockMirBufferStream>>();521 auto mock_stream = std::make_shared<NiceMock<mtd::MockMirBufferStream>>();
522 mir::frontend::BufferStreamId const mock_stream_id(2);
520 auto mock_input_platform = std::make_shared<NiceMock<MockClientInputPlatform>>();523 auto mock_input_platform = std::make_shared<NiceMock<MockClientInputPlatform>>();
521 ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mir::frontend::BufferStreamId(2)));524 ON_CALL(*mock_stream, rpc_id()).WillByDefault(Return(mock_stream_id));
522 ON_CALL(*mock_input_platform, create_input_receiver(_,_,_))525 ON_CALL(*mock_input_platform, create_input_receiver(_,_,_))
523 .WillByDefault(Return(std::make_shared<mt::TestDispatchable>([]{})));526 .WillByDefault(Return(std::make_shared<mt::TestDispatchable>([]{})));
524527
@@ -532,9 +535,10 @@
532 std::vector<ContentInfo> info =535 std::vector<ContentInfo> info =
533 { ContentInfo{ geom::Displacement{0,0}, 2, geom::Size{1,1}} };536 { ContentInfo{ geom::Displacement{0,0}, 2, geom::Size{1,1}} };
534 spec.streams = info;537 spec.streams = info;
538 surface_map->insert(mock_stream_id, mock_stream);
535 surface.modify(spec)->wait_for_all();539 surface.modify(spec)->wait_for_all();
536 surface.handle_event(*ev);540 surface.handle_event(*ev);
537 surface_map->erase(mir::frontend::BufferStreamId(2));541 surface_map->erase(mock_stream_id);
538}542}
539543
540TEST_F(MirClientSurfaceTest, parameters_are_unhooked_from_stream_sizes)544TEST_F(MirClientSurfaceTest, parameters_are_unhooked_from_stream_sizes)

Subscribers

People subscribed via source and target branches