commit 4afba20439a10cd7e30831ac8ba664ca468a0bd6
author Han Shen <shenhan@google.com> Mon Dec 17 08:48:35 2012 -0800
committer Gerrit <chrome-bot@google.com> Mon Dec 17 15:18:40 2012 -0800
tree 3b14e3127d83d63fcef1e7f14c19c03b92f17bea
parent 618af2b8d701e1033c178857012de58f23aaa252

Fix shill unittest bugs - singleton instances must be handled properly across different unittests.

Unittests for shill is a single executable, which run each unittest
one by one, so for each singleton, we must setup up and clear
properly, we must not make any assumption that this singleton is used
only for a particular test.

For instance in config80211_unittest.cc, singleton member
'Config80211::sock_' is assigned a value which invalidates right after
Config80211Test finishes, when later ShillDaemonTest::Start comes to
this non-NULL value, thinking that this is properly initialized and
makes a call on it, the program crashes.

TEST=all shill tests passed after fixing (was crashing)
BUG=None

Change-Id: Ifd17bfca319e4f5d93cb82bdde010d9a5db36dcd
Reviewed-on: https://gerrit.chromium.org/gerrit/39771
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Commit-Ready: Han Shen <shenhan@chromium.org>
Tested-by: Han Shen <shenhan@chromium.org>

config80211_unittest.cc

diff --git a/config80211_unittest.cc b/config80211_unittest.cc
index d9e93c8..3db52e3 100644
--- a/config80211_unittest.cc
+++ b/config80211_unittest.cc
@@ -360,6 +360,12 @@
 class Config80211Test : public Test {
  public:
   Config80211Test() : config80211_(Config80211::GetInstance()) {}
+  ~Config80211Test() {
+    // Config80211 is a singleton, the sock_ field *MUST* be cleared
+    // before "Config80211Test::socket_" gets invalidated, otherwise
+    // later tests will refer to a corrupted memory.
+    config80211_->sock_ = NULL;
+  }
   void SetupConfig80211Object() {
     EXPECT_NE(config80211_, reinterpret_cast<Config80211 *>(NULL));
     config80211_->sock_ = &socket_;

dhcp_provider_unittest.cc

diff --git a/dhcp_provider_unittest.cc b/dhcp_provider_unittest.cc
index 042cea5..7107b48 100644
--- a/dhcp_provider_unittest.cc
+++ b/dhcp_provider_unittest.cc
@@ -24,6 +24,10 @@
   DHCPProviderTest() : provider_(DHCPProvider::GetInstance()) {
     provider_->glib_ = &glib_;
     provider_->control_interface_ = &control_;
+    // DHCPProvider is a singleton, there is no guarentee that it is
+    // not setup/used elsewhere, so reset its state before running our
+    // tests.
+    provider_->configs_.clear();
   }
 
  protected:

rtnl_handler.h

diff --git a/rtnl_handler.h b/rtnl_handler.h
index d6fe967..3973d13 100644
--- a/rtnl_handler.h
+++ b/rtnl_handler.h
@@ -105,6 +105,7 @@
   friend class DeviceInfoTest;
   friend class ModemTest;
   friend class RTNLHandlerTest;
+  friend class RTNLListenerTest;
   friend class RoutingTableTest;
 
   FRIEND_TEST(RTNLListenerTest, NoRun);

rtnl_listener_unittest.cc

diff --git a/rtnl_listener_unittest.cc b/rtnl_listener_unittest.cc
index aed4137..faf2913 100644
--- a/rtnl_listener_unittest.cc
+++ b/rtnl_listener_unittest.cc
@@ -31,6 +31,13 @@
 
   MOCK_METHOD1(ListenerCallback, void(const RTNLMessage &));
 
+  virtual void SetUp() {
+    // RTNLHandler is a singleton, there's no guarentee that it is not
+    // setup/used by other unittests. Clear "listeners_" field before we run
+    // tests.
+    RTNLHandler::GetInstance()->listeners_.clear();
+  }
+
  protected:
   Callback<void(const RTNLMessage &)> callback_;
 };