Code review comment for lp:~3v1n0/unity/launcher-controller-ensure-new

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Looks good.

There are some merge markers in added file 'tests/test_launcher_controller.cpp'.

General comments:

 * If you want to avoid the friend declaration, you can do something like this:
   class TestLauncherExpectationsInterface
   {
       public:
           virtual void ExpectNumControllers (int num) = 0;
           ...
   }

   LauncherControllerPrivate::SetExpectations (TestLauncherExpectationsInterface *expect)
   {
       ...
       expect->ExpectNumControllers (whatever);
   }

   ::testing::NiceMock <MockLauncherExpectations> mock_launcher_expectations;

   EXPECT_CALL (mock_launcher_expectations, ExpectNumControllers (value));

   This method is pretty ugly though, and the friend might be better. Generally speaking, testing implementation details isn't really recommended, but in this case we want to do that.

 * Its a bit unclear as to how UScreen.SetupFakeMultimonitor () causes more launchers to be added to an lc it doesn't know about, unless some singleton knows about lc, and that's pretty confusing.

« Back to merge proposal