Merge lp:~3v1n0/unity/testing-friend into lp:unity

[Marco Trevisan (Treviño)]

TestingFriend: added testing friend, a tool that will (hopefully) help you in testing!

Added the unity::tests::Friend class definition and some utility macros that can be used to easily test private members of classes without great refactoring.

Basically it's only needed that the class(es) you're testing is friend of unity::tests::Friend and then on the testing side you can implement that Friend function with the proxy functions you need to access to private members.
I've added some macros that allow this definition easier. This is needed since in C++ the friendship is not transitive.

I know that the same could have been achieved just by adding a different friend class for each class that is then implemented as inheriting from ::testing::Test (as done in lp:~3v1n0/unity/launcher-controller-ensure-new for LauncherController), but I didn't like this for two reasons:
1) it would have allowed to access to all members from the tests, without any explicit control on it
2) it would have not allowed to test interactions between classes without defining friends around

[Tim Penhey (thumper)]

Don't you have to make sure that the unit::tests::Friend class is in an anonymous namespace in the test source file to make sure you don't get linking errors?

[Marco Trevisan (Treviño) (3v1n0)]

> Don't you have to make sure that the unit::tests::Friend class is in an
> anonymous namespace in the test source file to make sure you don't get linking
> errors?

If I add it to an anonymous namespace, it won't work since it won't be considered friend of the class I've added to.

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:2558
http://jenkins.qa.ubuntu.com/job/unity-ci/126/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=amd64/126/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-ci/./distribution=quantal,flavor=i386/126/console

review: Needs Fixing (continuous-integration)

[Martin Mrazik (mrazik)]

Sorry for the jenkins failure. I'm looking into this.

[Marco Trevisan (Treviño) (3v1n0)]

Ah, and Tim... I see no link errors here even when defining it multiple times (actually it's defined twice). Is that maybe because the definition is actually inline?

[Marco Trevisan (Treviño) (3v1n0)]

> Ah, and Tim... I see no link errors here even when defining it multiple times
> (actually it's defined twice). Is that maybe because the definition is
> actually inline?

Looking at the standard, it also seems caused by the fact that these tests::Friend classes are defined in different translation units, and this will be always true (until you don't play too much with tests), so we should be and we actually are safe from linking errors.

[Francis Ginther (fginther)]

Review was claimed by accident, please ignore.

review: Abstain

[Brandon Schaefer (brandontschaefer)]

In file included from /home/bschaefer/src/unity/tests/test_launcher_controller.cpp:24:0:
/home/bschaefer/src/unity/launcher/LauncherController.h: In member function ‘unity::launcher::LauncherModel::Ptr unity::launcher::TestLauncherController::GetLauncherModel()’:
/home/bschaefer/src/unity/launcher/LauncherController.h:91:25: error: ‘std::unique_ptr<unity::launcher::Controller::Impl> unity::launcher::Controller::pimpl’ is private
/home/bschaefer/src/unity/tests/test_launcher_controller.cpp:96:15: error: within this context

Also should merge with trunk as it still has the old dependency for libutouch-geis

review: Needs Fixing

[Andrea Azzarone (azzar1)]

review: Disapprove

Unmerged revisions

2558. By Marco Trevisan (Treviño) on 2012-07-25

Merging with lp:~3v1n0/unity/launcher-controller-ensure-new (and trunk)

2557. By Marco Trevisan (Treviño) on 2012-07-25

test_edge_barrier_controller: use tests::Friend

2556. By Marco Trevisan (Treviño) on 2012-07-25

EdgeBarrierController: move the definition of ::Impl in EdgeBarrierControllerPrivate

For testing purposes...

2555. By Marco Trevisan (Treviño) on 2012-07-25

LauncherController: use unity::tests::Friend to access to pimpl!

2554. By Marco Trevisan (Treviño) on 2012-07-25

TestingFriend: added testing friend, a tool that will help you in testing!

=== modified file 'launcher/EdgeBarrierController.cpp'
--- launcher/EdgeBarrierController.cpp	2012-07-25 23:45:28 +0000
+++ launcher/EdgeBarrierController.cpp	2012-07-25 23:45:28 +0000
@@ -18,47 +18,12 @@
  *              Marco Trevisan <[e-mail]>
  */
 
-#include "EdgeBarrierController.h"
-#include "Decaymulator.h"
+#include "EdgeBarrierControllerPrivate.h"
 #include "unity-shared/UScreen.h"
-#include "UnityCore/GLibSource.h"
 
 namespace unity {
 namespace ui {
 
-struct EdgeBarrierController::Impl
-{
-  Impl(EdgeBarrierController *parent);
-
-  void ResizeBarrierList(std::vector<nux::Geometry> const& layout);
-  void SetupBarriers(std::vector<nux::Geometry> const& layout);
-
-  void OnPointerBarrierEvent(PointerBarrierWrapper* owner, BarrierEvent::Ptr event);
-  void BarrierRelease(PointerBarrierWrapper* owner, int event);
-
-  bool StickyEdgeSetter(bool const& new_val)
-  {
-    if (parent_->options() && new_val != parent_->options()->edge_resist())
-    {
-      parent_->options()->edge_resist = new_val;
-      return true;
-    }
-    return false;
-  }
-
-  bool StickyEdgeGetter()
-  {
-    return parent_->options() ? parent_->options()->edge_resist() : false;
-  }
-
-  std::vector<PointerBarrierWrapper::Ptr> barriers_;
-  std::vector<EdgeBarrierSubscriber*> subscribers_;
-  Decaymulator decaymulator_;
-  glib::Source::UniquePtr release_timeout_;
-  float edge_overcome_pressure_;
-  EdgeBarrierController* parent_;
-};
-
 EdgeBarrierController::Impl::Impl(EdgeBarrierController *parent)
   : edge_overcome_pressure_(0)
   , parent_(parent)
@@ -235,10 +200,5 @@
   return pimpl->subscribers_[monitor];
 }
 
-void EdgeBarrierController::ProcessBarrierEvent(PointerBarrierWrapper* owner, BarrierEvent::Ptr event)
-{
-  pimpl->OnPointerBarrierEvent(owner, event);
-}
-
 }
 }
 
=== modified file 'launcher/EdgeBarrierController.h'
--- launcher/EdgeBarrierController.h	2012-07-25 23:45:28 +0000
+++ launcher/EdgeBarrierController.h	2012-07-25 23:45:28 +0000
@@ -22,6 +22,7 @@
 
 #include "PointerBarrier.h"
 #include "LauncherOptions.h"
+#include "unity-shared/TestingFriend.h"
 
 namespace unity {
 namespace ui {
@@ -45,12 +46,11 @@
   void Unsubscribe(EdgeBarrierSubscriber* subscriber, unsigned int monitor);
   EdgeBarrierSubscriber* GetSubscriber(unsigned int monitor);
 
-protected:
-  void ProcessBarrierEvent(PointerBarrierWrapper* owner, BarrierEvent::Ptr event);
-
 private:
   struct Impl;
   std::unique_ptr<Impl> pimpl;
+
+  friend class tests::Friend;
 };
 
 }
 
=== added file 'launcher/EdgeBarrierControllerPrivate.h'
--- launcher/EdgeBarrierControllerPrivate.h	1970-01-01 00:00:00 +0000
+++ launcher/EdgeBarrierControllerPrivate.h	2012-07-25 23:45:28 +0000
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ *              Jason Smith <[e-mail]>
+ *
+ */
+
+#ifndef UNITY_EDGE_BARRIER_CONTROLLER_PRIVATE_H
+#define UNITY_EDGE_BARRIER_CONTROLLER_PRIVATE_H
+
+#include "EdgeBarrierController.h"
+#include "Decaymulator.h"
+#include "UnityCore/GLibSource.h"
+
+namespace unity {
+namespace ui {
+
+struct EdgeBarrierController::Impl
+{
+  Impl(EdgeBarrierController *parent);
+
+  void ResizeBarrierList(std::vector<nux::Geometry> const& layout);
+  void SetupBarriers(std::vector<nux::Geometry> const& layout);
+
+  void OnPointerBarrierEvent(PointerBarrierWrapper* owner, BarrierEvent::Ptr event);
+  void BarrierRelease(PointerBarrierWrapper* owner, int event);
+
+  bool StickyEdgeSetter(bool const& new_val)
+  {
+    if (parent_->options() && new_val != parent_->options()->edge_resist())
+    {
+      parent_->options()->edge_resist = new_val;
+      return true;
+    }
+    return false;
+  }
+
+  bool StickyEdgeGetter()
+  {
+    return parent_->options() ? parent_->options()->edge_resist() : false;
+  }
+
+  std::vector<PointerBarrierWrapper::Ptr> barriers_;
+  std::vector<EdgeBarrierSubscriber*> subscribers_;
+  Decaymulator decaymulator_;
+  glib::Source::UniquePtr release_timeout_;
+  float edge_overcome_pressure_;
+  EdgeBarrierController* parent_;
+};
+
+} // namespace ui
+} // namespace unity
+
+#endif
 
=== modified file 'launcher/LauncherController.h'
--- launcher/LauncherController.h	2012-07-25 23:45:28 +0000
+++ launcher/LauncherController.h	2012-07-25 23:45:28 +0000
@@ -27,6 +27,7 @@
 
 #include "LauncherOptions.h"
 #include "SoftwareCenterLauncherIcon.h"
+#include "unity-shared/TestingFriend.h"
 
 namespace unity
 {
@@ -35,7 +36,6 @@
 class AbstractLauncherIcon;
 class Launcher;
 class LauncherModel;
-class TestLauncherController;
 
 class Controller : public unity::debug::Introspectable, public sigc::trackable
 {
@@ -87,9 +87,9 @@
   void AddProperties(GVariantBuilder* builder);
 
 private:
-  friend class TestLauncherController;
   class Impl;
   std::unique_ptr<Impl> pimpl;
+  friend class tests::Friend;
 };
 
 }
 
=== modified file 'tests/test_edge_barrier_controller.cpp'
--- tests/test_edge_barrier_controller.cpp	2012-07-25 23:45:28 +0000
+++ tests/test_edge_barrier_controller.cpp	2012-07-25 23:45:28 +0000
@@ -22,7 +22,7 @@
 #include "test_utils.h"
 #include "test_uscreen_mock.h"
 
-#include "EdgeBarrierController.h"
+#include "EdgeBarrierControllerPrivate.h"
 
 using namespace unity;
 using namespace unity::ui;
@@ -46,20 +46,6 @@
   MOCK_METHOD1(ReleaseBarrier, void(int));
 };
 
-class MockEdgeBarrierController : public EdgeBarrierController
-{
-public:
-  void ProcessBarrierEvent(PointerBarrierWrapper* owner, BarrierEvent::Ptr event)
-  {
-    EdgeBarrierController::ProcessBarrierEvent(owner, event);
-  }
-
-  EdgeBarrierSubscriber* GetSubscriber(unsigned int monitor)
-  {
-    return EdgeBarrierController::GetSubscriber(monitor);
-  }
-};
-
 class TestBarrierSubscriber : public EdgeBarrierSubscriber
 {
 public:
@@ -75,7 +61,21 @@
   bool handles_;
 };
 
-class TestEdgeBarrierController : public Test
+}
+
+namespace unity
+{
+namespace tests
+{
+// Implementing unity::tests::Friend, see TestingFriend.h
+class Friend : public ::testing::Test
+{
+public:
+  PRIVATE_PIMPL_METHOD_CALLER(pimpl, OnPointerBarrierEvent);
+  PRIVATE_PIMPL_MEMBER_GETTER(pimpl, subscribers_);
+};
+
+class TestEdgeBarrierController : public Friend
 {
 public:
   virtual void SetUp()
@@ -93,6 +93,11 @@
     }
   }
 
+  void ProcessBarrierEvent(PointerBarrierWrapper* owner, BarrierEvent::Ptr event)
+  {
+    CallPrivatepimplOnPointerBarrierEvent<EdgeBarrierController, void, PointerBarrierWrapper*, BarrierEvent::Ptr>(bc, owner, event);
+  }
+
   BarrierEvent::Ptr MakeBarrierEvent(int id, bool breaker)
   {
     int velocity = breaker ? std::numeric_limits<int>::max() : bc.options()->edge_overcome_pressure() - 1;
@@ -101,7 +106,7 @@
 
   TestBarrierSubscriber subscribers_[max_num_monitors];
   MockUScreen uscreen;
-  MockEdgeBarrierController bc;
+  EdgeBarrierController bc;
 };
 
 TEST_F(TestEdgeBarrierController, Construction)
@@ -145,7 +150,7 @@
   auto breaking_barrier_event = MakeBarrierEvent(0, true);
 
   EXPECT_CALL(owner, ReleaseBarrier(_)).Times(0);
-  bc.ProcessBarrierEvent(&owner, breaking_barrier_event);
+  ProcessBarrierEvent(&owner, breaking_barrier_event);
 }
 
 TEST_F(TestEdgeBarrierController, ProcessHandledEventOnReleasedBarrier)
@@ -159,7 +164,7 @@
   auto breaking_barrier_event = MakeBarrierEvent(0, true);
 
   EXPECT_CALL(owner, ReleaseBarrier(_)).Times(0);
-  bc.ProcessBarrierEvent(&owner, breaking_barrier_event);
+  ProcessBarrierEvent(&owner, breaking_barrier_event);
 }
 
 TEST_F(TestEdgeBarrierController, ProcessUnHandledEventBreakingBarrier)
@@ -171,7 +176,7 @@
   auto breaking_barrier_event = MakeBarrierEvent(breaking_id, true);
 
   EXPECT_CALL(owner, ReleaseBarrier(breaking_id));
-  bc.ProcessBarrierEvent(&owner, breaking_barrier_event);
+  ProcessBarrierEvent(&owner, breaking_barrier_event);
 }
 
 TEST_F(TestEdgeBarrierController, ProcessUnHandledEventBreakingBarrierOnMaxMonitor)
@@ -185,7 +190,7 @@
   // you can reproduce this repeating this test multiple times using the
   // --gtest_repeat=X command line
   EXPECT_CALL(owner, ReleaseBarrier(_));
-  bc.ProcessBarrierEvent(&owner, breaking_barrier_event);
+  ProcessBarrierEvent(&owner, breaking_barrier_event);
 }
 
 TEST_F(TestEdgeBarrierController, ProcessUnHandledEventNotBreakingBarrier)
@@ -197,7 +202,7 @@
   auto not_breaking_barrier_event = MakeBarrierEvent(not_breaking_id, false);
 
   EXPECT_CALL(owner, ReleaseBarrier(not_breaking_id)).Times(0);
-  bc.ProcessBarrierEvent(&owner, not_breaking_barrier_event);
+  ProcessBarrierEvent(&owner, not_breaking_barrier_event);
 }
 
 TEST_F(TestEdgeBarrierController, ProcessUnHandledEventOnReleasedBarrier)
@@ -209,7 +214,7 @@
   auto not_breaking_barrier_event = MakeBarrierEvent(not_breaking_id, false);
 
   EXPECT_CALL(owner, ReleaseBarrier(not_breaking_id));
-  bc.ProcessBarrierEvent(&owner, not_breaking_barrier_event);
+  ProcessBarrierEvent(&owner, not_breaking_barrier_event);
 }
 
 TEST_F(TestEdgeBarrierController, BreakingEdgeTemporaryReleasesBarrier)
@@ -217,7 +222,7 @@
   MockPointerBarrier owner;
 
   EXPECT_CALL(owner, ReleaseBarrier(1));
-  bc.ProcessBarrierEvent(&owner, MakeBarrierEvent(1, true));
+  ProcessBarrierEvent(&owner, MakeBarrierEvent(1, true));
   ASSERT_TRUE(owner.released());
 
   Utils::WaitForTimeoutMSec(bc.options()->edge_passed_disabled_ms);
@@ -231,12 +236,12 @@
   subscribers_[monitor].handles_ = false;
 
   EXPECT_CALL(owner, ReleaseBarrier(5));
-  bc.ProcessBarrierEvent(&owner, MakeBarrierEvent(5, true));
+  ProcessBarrierEvent(&owner, MakeBarrierEvent(5, true));
   ASSERT_TRUE(owner.released());
 
   subscribers_[monitor].handles_ = false;
   EXPECT_CALL(owner, ReleaseBarrier(6));
-  bc.ProcessBarrierEvent(&owner, MakeBarrierEvent(6, false));
+  ProcessBarrierEvent(&owner, MakeBarrierEvent(6, false));
 }
 
 TEST_F(TestEdgeBarrierController, BreakingEdgeDontReleasesBarrierForHandledEvents)
@@ -246,12 +251,12 @@
   subscribers_[monitor].handles_ = false;
 
   EXPECT_CALL(owner, ReleaseBarrier(5));
-  bc.ProcessBarrierEvent(&owner, MakeBarrierEvent(5, true));
+  ProcessBarrierEvent(&owner, MakeBarrierEvent(5, true));
   ASSERT_TRUE(owner.released());
 
   subscribers_[monitor].handles_ = true;
   EXPECT_CALL(owner, ReleaseBarrier(_)).Times(0);
-  bc.ProcessBarrierEvent(&owner, MakeBarrierEvent(6, true));
+  ProcessBarrierEvent(&owner, MakeBarrierEvent(6, true));
 }
 
 TEST_F(TestEdgeBarrierController, StickyEdgePropertyProxiesLauncherOption)
@@ -270,3 +275,4 @@
 }
 
 }
+}
 
=== modified file 'tests/test_launcher_controller.cpp'
--- tests/test_launcher_controller.cpp	2012-07-25 23:45:28 +0000
+++ tests/test_launcher_controller.cpp	2012-07-25 23:45:28 +0000
@@ -32,6 +32,16 @@
 namespace unity
 {
 
+// Implementing unity::tests::Friend, see TestingFriend.h
+namespace tests
+{
+  class Friend : public ::testing::Test
+  {
+  public:
+    PRIVATE_PIMPL_MEMBER_GETTER(pimpl, edge_barriers_)
+  };
+}
+
 class MockFavoriteStore : public FavoriteStore
 {
 public:
@@ -51,7 +61,7 @@
 
 namespace launcher
 {
-class TestLauncherController : public testing::Test
+class TestLauncherController : public tests::Friend
 {
 public:
   TestLauncherController()
@@ -67,7 +77,7 @@
 protected:
   ui::EdgeBarrierController &GetBarrierController()
   {
-    return lc.pimpl->edge_barriers_;
+    return *get_private_pimpl_edge_barriers_<launcher::Controller, ui::EdgeBarrierController>(lc);
   }
 
   MockUScreen uscreen;
 
=== added file 'unity-shared/TestingFriend.h'
--- unity-shared/TestingFriend.h	1970-01-01 00:00:00 +0000
+++ unity-shared/TestingFriend.h	2012-07-25 23:45:28 +0000
@@ -0,0 +1,155 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Marco Trevisan (Treviño) <[e-mail]>
+ */
+
+#ifndef TESTING_FRIEND_H
+#define TESTING_FRIEND_H
+
+namespace unity
+{
+namespace tests
+{
+/*
+* This is a class without definition that is meant to be used as a friend
+* in the classes that we want to test accessing to private members.
+* The definition of this must be done in the testing file.
+*/
+class Friend;
+
+#define PRIVATE_MEMBER_GETTER(member) \
+  template <typename T, typename R> R* get_private_##member(T& cl) { return &(cl.member); }
+#define PRIVATE_PIMPL_MEMBER_GETTER(pimpl, sub) \
+  template <typename T, typename R> R* get_private_##pimpl##_##sub(T& cl) { return &(cl.pimpl->sub); }
+#define PRIVATE_METHOD_CALLER(Method) \
+  template <typename T, typename R, typename... Ts> R CallPrivate##Method(T& cl, Ts... vars) { return cl.Method(vars...); }
+#define PRIVATE_PIMPL_METHOD_CALLER(pimpl, Method) \
+  template <typename T, typename R, typename... Ts> R CallPrivate##pimpl##Method(T& cl, Ts... vars) { return cl.pimpl->Method(vars...); }
+
+/*
+ * Example:
+
+// This should go in FooTestMe.h
+
+#include "TestingFriend.h"
+
+class FooTestMe
+{
+public:
+  //...
+protected:
+  void virtual VirtualMethod() {}
+
+private:
+  void Method() { VirtualMethod(); }
+  double MethodWithValues(int a, bool b) { priv_value_ = b ? a+1 : a-1; return priv_value_;}
+
+  double priv_value_;
+
+  friend class unity::tests::Friend;
+};
+
+ * At this point in each test file you want to use this feature, you need to
+ * define the Friend class, inheriting from Google tests' testing::Test
+ * now, this class can access to all the private members of all the unity
+ * classes that have been defined as friend of unity::tests::Friend, but this
+ * is not enough to give the tests access to the private data (since we want to
+ * inherit from it on tests).
+ *
+ * So, to achieve this you can both write proxy functions and use the macros
+ * defined above to generate template functions that allow the access to the
+ * private data or methods.
+
+// This should go in test_foo_test_me.cpp
+
+#include <gtest/gtest.h>
+#include "FooTestMe.h"
+
+namespace unity
+{
+namespace tests
+{
+  class Friend : public ::testing::Test
+  {
+  public:
+    // Generates void CallPrivateMethod(T const& cl)
+    PRIVATE_METHOD_CALLER(Method)
+
+    // Generates int CallPrivateMethodWithValues(T const& cl, int a, bool b)
+    PRIVATE_METHOD_CALLER(MethodWithValues)
+
+    // Generates int* get_private_priv_value_(T const& cl)
+    PRIVATE_MEMBER_GETTER(priv_value_)
+
+    // Otherwise you can still do
+    double CallMethodWithValues(FooTestMe& foo_test_me, int a, int b)
+    {
+      return foo_test_me.MethodWithValues(a, b);
+    }
+  };
+}
+}
+
+ * It' also possible to mock the function to test, if needed.
+
+// Optional Mocking
+class FooTestMeMock : public FooTestMe
+{
+public:
+  MOCK_METHOD0(VirtualMethod, void());
+};
+
+ * This is the actual class used by gtest as testing class
+
+namespace
+{
+class TestFooTestMe : public unity::tests::Friend
+{
+public:
+  virtual void SetUp()
+  {}
+
+  // Just use FooTestMe if you don't need mocking
+  FooTestMeMock test_me;
+};
+
+ * In the test functions you can access the private members, just using the
+ * functions that have been generated into unity::tests::Friend to get access
+ * to the tested class.
+
+TEST_F(TestFooTestMe, Methods)
+{
+  EXPECT_CALL(test_me, VirtualMethod());
+  CallPrivateMethod<FooTestMe, void>(test_me);
+
+  double val = CallPrivateMethodWithValues<FooTestMe, double, int, bool>(test_me, 9, true);
+  EXPECT_EQ(val, 10.0);
+
+  double* priv_value = get_private_priv_value_<FooTestMe, double>(test_me);
+  EXPECT_EQ(*priv_value, 10.0);
+
+  EXPECT_EQ(11.0, CallMethodWithValues(test_me, 10, true));
+}
+}
+
+*
+*/
+
+} // namespace testing
+} // namespace unity
+
+#endif