Merge lp:~robertcarr/mir/cleanup-session-manager-unit-test into lp:~mir-team/mir/trunk

[Robert Carr]

Reduce code duplication in session manager unit test to ease refactoring.

Reduce code duplication in session manager unit test to ease refactoring.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:460
http://jenkins.qa.ubuntu.com/job/mir-ci/1144/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-ci/./build=pbuilder,distribution=quantal,flavor=amd64/1144/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/1144//rebuild/?

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

34 + std::shared_ptr<msess::SessionManager> session_manager;

We could make this a normal object instead of a shared_ptr, by using the Test fixture constructor, instead of SetUp(), to initialize the fixture.

In general, I prefer using the constructor since it allows us to initialize members objects (like session_manager in this case) and also to have const members variables. Of course, there are a few cases when we need to use SetUp(), as mentioned in the linked FAQ below.

See https://code.google.com/p/googletest/wiki/V1_6_FAQ#Should_I_use_the_constructor/destructor_of_the_test_fixture_or_t

review: Needs Fixing

[Robert Carr (robertcarr)]

Good idea!

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:461
http://jenkins.qa.ubuntu.com/job/mir-ci/1149/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-ci/./build=pbuilder,distribution=quantal,flavor=amd64/1149/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/1149//rebuild/?

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

22 +
23 +

A couple of extra spaces introduce here, but nothing to block on.

review: Approve

[Alan Griffiths (alan-griffiths)]

Apart from the merge conflicts looks OK.

review: Approve

[Robert Carr (robertcarr)]

Trunk merged.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:463
http://jenkins.qa.ubuntu.com/job/mir-ci/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/2//console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/mir-ci/2//rebuild/?

review: Approve (continuous-integration)

=== modified file 'tests/unit-tests/sessions/test_session_manager.cpp'
--- tests/unit-tests/sessions/test_session_manager.cpp	2013-03-06 12:54:33 +0000
+++ tests/unit-tests/sessions/test_session_manager.cpp	2013-03-06 18:15:25 +0000
@@ -64,22 +64,30 @@
     MOCK_METHOD1(set_focus_to, void(std::shared_ptr<msh::Session> const&));
 };
 
-}
-
-TEST(SessionManager, open_and_close_session)
+struct SessionManagerSetup : public testing::Test
 {
-    using namespace ::testing;
+    SessionManagerSetup()
+      : session_manager(mt::fake_shared(surface_factory),
+                        mt::fake_shared(container),
+                        mt::fake_shared(sequence),
+                        mt::fake_shared(focus_setter))
+    {
+    }
+
     mtd::MockSurfaceFactory surface_factory;
     MockSessionContainer container;
     MockFocusSequence sequence;
     MockFocusSetter focus_setter;
 
-    msh::SessionManager session_manager(
-        mt::fake_shared(surface_factory),
-        mt::fake_shared(container),
-        mt::fake_shared(sequence),
-        mt::fake_shared(focus_setter));
-
+    msh::SessionManager session_manager;
+};
+
+}
+
+TEST_F(SessionManagerSetup, open_and_close_session)
+{
+    using namespace ::testing;
+ 
     EXPECT_CALL(container, insert_session(_)).Times(1);
     EXPECT_CALL(container, remove_session(_)).Times(1);
     EXPECT_CALL(focus_setter, set_focus_to(_));
@@ -91,19 +99,9 @@
     session_manager.close_session(session);
 }
 
-TEST(SessionManager, closing_session_removes_surfaces)
+TEST_F(SessionManagerSetup, closing_session_removes_surfaces)
 {
     using namespace ::testing;
-    mtd::MockSurfaceFactory surface_factory;
-    MockSessionContainer container;
-    MockFocusSequence sequence;
-    MockFocusSetter mechanism;
-
-    msh::SessionManager session_manager(
-        mt::fake_shared(surface_factory),
-        mt::fake_shared(container),
-        mt::fake_shared(sequence),
-        mt::fake_shared(mechanism));
 
     EXPECT_CALL(surface_factory, create_surface(_)).Times(1);
     std::shared_ptr<ms::BufferBundle> buffer_bundle(new mtd::NullBufferBundle());
@@ -117,8 +115,8 @@
     EXPECT_CALL(container, insert_session(_)).Times(1);
     EXPECT_CALL(container, remove_session(_)).Times(1);
 
-    EXPECT_CALL(mechanism, set_focus_to(_)).Times(1);
-    EXPECT_CALL(mechanism, set_focus_to(std::shared_ptr<msh::Session>())).Times(1);
+    EXPECT_CALL(focus_setter, set_focus_to(_)).Times(1);
+    EXPECT_CALL(focus_setter, set_focus_to(std::shared_ptr<msh::Session>())).Times(1);
 
     EXPECT_CALL(sequence, default_focus()).WillOnce(Return((std::shared_ptr<msh::Session>())));
 
@@ -128,64 +126,34 @@
     session_manager.close_session(session);
 }
 
-TEST(SessionManager, new_applications_receive_focus)
+TEST_F(SessionManagerSetup, new_applications_receive_focus)
 {
     using namespace ::testing;
-    mtd::MockSurfaceFactory surface_factory;
-    MockSessionContainer container;
-    MockFocusSequence sequence;
-    MockFocusSetter mechanism;
     std::shared_ptr<msh::Session> new_session;
 
-    msh::SessionManager session_manager(
-        mt::fake_shared(surface_factory),
-        mt::fake_shared(container),
-        mt::fake_shared(sequence),
-        mt::fake_shared(mechanism));
-
     EXPECT_CALL(container, insert_session(_)).Times(1);
-    EXPECT_CALL(mechanism, set_focus_to(_)).WillOnce(SaveArg<0>(&new_session));
+    EXPECT_CALL(focus_setter, set_focus_to(_)).WillOnce(SaveArg<0>(&new_session));
 
     auto session = session_manager.open_session("Visual Basic Studio");
     EXPECT_EQ(session, new_session);
 }
 
-TEST(SessionManager, apps_selected_by_id_receive_focus)
+TEST_F(SessionManagerSetup, apps_selected_by_id_receive_focus)
 {
     using namespace ::testing;
-    mtd::MockSurfaceFactory surface_factory;
-    NiceMock<MockSessionContainer> container;
-    MockFocusSequence sequence;
-    NiceMock<MockFocusSetter> mechanism;
-
-    msh::SessionManager session_manager(
-        mt::fake_shared(surface_factory),
-        mt::fake_shared(container),
-        mt::fake_shared(sequence),
-        mt::fake_shared(mechanism));
 
     auto session1 = session_manager.open_session("Visual Basic Studio");
     auto session2 = session_manager.open_session("IntelliJ IDEA");
 
     session_manager.tag_session_with_lightdm_id(session1, 1);
 
-    EXPECT_CALL(mechanism, set_focus_to(session1));
+    EXPECT_CALL(focus_setter, set_focus_to(session1));
     session_manager.focus_session_with_lightdm_id(1);
 }
 
-TEST(SessionManager, closing_apps_selected_by_id_changes_focus)
+TEST_F(SessionManagerSetup, closing_apps_selected_by_id_changes_focus)
 {
     using namespace ::testing;
-    mtd::MockSurfaceFactory surface_factory;
-    NiceMock<MockSessionContainer> container;
-    MockFocusSequence sequence;
-    NiceMock<MockFocusSetter> mechanism;
-
-    msh::SessionManager session_manager(
-        mt::fake_shared(surface_factory),
-        mt::fake_shared(container),
-        mt::fake_shared(sequence),
-        mt::fake_shared(mechanism));
 
     auto session1 = session_manager.open_session("Visual Basic Studio");
     auto session2 = session_manager.open_session("IntelliJ IDEA");
@@ -194,7 +162,7 @@
     session_manager.focus_session_with_lightdm_id(1);
 
     EXPECT_CALL(sequence, default_focus()).WillOnce(Return(session2));
-    EXPECT_CALL(mechanism, set_focus_to(session2));
+    EXPECT_CALL(focus_setter, set_focus_to(session2));
 
     session_manager.close_session(session1);
 }