shill: Start a mockable GLib interface.
Use the interface to test DHCPConfig::Start.
BUG=chromium-os:16319,chromium-os:16013
TEST=unit test, tested on device
Change-Id: Ice6c65b0cc2ac2dd738cb6aacb426703cce48a3f
Reviewed-on: http://gerrit.chromium.org/gerrit/2356
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Chris Masone <cmasone@chromium.org>
diff --git a/Makefile b/Makefile
index 4263d05..65dc8b7 100644
--- a/Makefile
+++ b/Makefile
@@ -50,6 +50,7 @@
dhcpcd_proxy.o \
ethernet.o \
ethernet_service.o \
+ glib.o \
glib_io_handler.o \
ipconfig.o \
manager.o \
diff --git a/dhcp_config.cc b/dhcp_config.cc
index 9ca6eed..13d9325 100644
--- a/dhcp_config.cc
+++ b/dhcp_config.cc
@@ -7,10 +7,10 @@
#include <arpa/inet.h>
#include <base/logging.h>
-#include <glib.h>
#include "shill/dhcpcd_proxy.h"
#include "shill/dhcp_provider.h"
+#include "shill/glib_interface.h"
using std::string;
using std::vector;
@@ -28,10 +28,13 @@
const char DHCPConfig::kDHCPCDPath[] = "/sbin/dhcpcd";
-DHCPConfig::DHCPConfig(DHCPProvider *provider, DeviceConstRefPtr device)
+DHCPConfig::DHCPConfig(DHCPProvider *provider,
+ DeviceConstRefPtr device,
+ GLibInterface *glib)
: IPConfig(device),
provider_(provider),
- pid_(0) {
+ pid_(0),
+ glib_(glib) {
VLOG(2) << __func__ << ": " << GetDeviceName();
}
@@ -91,20 +94,20 @@
envp[0] = NULL;
GPid pid = 0;
- if (!g_spawn_async(NULL,
- argv,
- envp,
- G_SPAWN_DO_NOT_REAP_CHILD,
- NULL,
- NULL,
- &pid,
- NULL)) {
+ if (!glib_->SpawnAsync(NULL,
+ argv,
+ envp,
+ G_SPAWN_DO_NOT_REAP_CHILD,
+ NULL,
+ NULL,
+ &pid,
+ NULL)) {
LOG(ERROR) << "Unable to spawn " << kDHCPCDPath;
return false;
}
pid_ = pid;
LOG(INFO) << "Spawned " << kDHCPCDPath << " with pid: " << pid_;
- provider_->BindPID(pid_, DHCPConfigRefPtr(this));
+ provider_->BindPID(pid_, this);
// TODO(petkov): Add an exit watch to cleanup w/ g_spawn_close_pid.
return true;
}
@@ -140,9 +143,7 @@
return false;
}
} else if (key == kConfigurationKeyRouters) {
- vector<unsigned int> routers;
- DBus::MessageIter reader = value.reader();
- reader >> routers;
+ vector<unsigned int> routers = value.operator vector<unsigned int>();
if (routers.empty()) {
LOG(ERROR) << "No routers provided.";
return false;
@@ -152,10 +153,7 @@
return false;
}
} else if (key == kConfigurationKeyDNS) {
- vector<unsigned int> servers;
- DBus::MessageIter reader = value.reader();
- reader >> servers;
- vector<string> dns_servers;
+ vector<unsigned int> servers = value.operator vector<unsigned int>();
for (vector<unsigned int>::const_iterator it = servers.begin();
it != servers.end(); ++it) {
string server = GetIPv4AddressString(*it);
@@ -167,8 +165,7 @@
} else if (key == kConfigurationKeyDomainName) {
properties->domain_name = value.reader().get_string();
} else if (key == kConfigurationKeyDomainSearch) {
- DBus::MessageIter reader = value.reader();
- reader >> properties->domain_search;
+ properties->domain_search = value.operator vector<string>();
} else if (key == kConfigurationKeyMTU) {
int mtu = value.reader().get_uint16();
if (mtu >= 576) {
diff --git a/dhcp_config.h b/dhcp_config.h
index a70d634..0e25873 100644
--- a/dhcp_config.h
+++ b/dhcp_config.h
@@ -17,6 +17,7 @@
class DHCPConfig;
class DHCPProvider;
class DHCPProxyInterface;
+class GLibInterface;
typedef scoped_refptr<DHCPConfig> DHCPConfigRefPtr;
@@ -33,7 +34,9 @@
static const char kConfigurationKeyRouters[];
static const char kConfigurationKeySubnetCIDR[];
- DHCPConfig(DHCPProvider *provider, DeviceConstRefPtr device);
+ DHCPConfig(DHCPProvider *provider,
+ DeviceConstRefPtr device,
+ GLibInterface *glib);
virtual ~DHCPConfig();
// Inherited from IPConfig.
@@ -51,6 +54,7 @@
private:
FRIEND_TEST(DHCPConfigTest, GetIPv4AddressString);
FRIEND_TEST(DHCPConfigTest, ParseConfiguration);
+ FRIEND_TEST(DHCPConfigTest, Start);
static const char kDHCPCDPath[];
@@ -75,6 +79,8 @@
// The proxy for communicating with the DHCP client.
scoped_ptr<DHCPProxyInterface> proxy_;
+ GLibInterface *glib_;
+
DISALLOW_COPY_AND_ASSIGN(DHCPConfig);
};
diff --git a/dhcp_config_unittest.cc b/dhcp_config_unittest.cc
index 117f23c..1c2b498 100644
--- a/dhcp_config_unittest.cc
+++ b/dhcp_config_unittest.cc
@@ -3,30 +3,37 @@
// found in the LICENSE file.
#include "shill/dhcp_config.h"
+#include "shill/dhcp_provider.h"
#include "shill/mock_control.h"
#include "shill/mock_device.h"
+#include "shill/mock_glib.h"
using std::string;
using std::vector;
+using testing::_;
+using testing::Return;
+using testing::SetArgumentPointee;
using testing::Test;
namespace shill {
class DHCPConfigTest : public Test {
public:
- DHCPConfigTest() :
- device_(new MockDevice(&control_interface_, NULL, NULL, "testname", 0)) {}
+ DHCPConfigTest()
+ : device_(new MockDevice(&control_interface_, NULL, NULL, "testname", 0)),
+ config_(new DHCPConfig(DHCPProvider::GetInstance(), device_, &glib_)) {}
protected:
+ MockGLib glib_;
MockControl control_interface_;
scoped_refptr<MockDevice> device_;
+ DHCPConfigRefPtr config_;
};
TEST_F(DHCPConfigTest, GetIPv4AddressString) {
- DHCPConfigRefPtr config(new DHCPConfig(NULL, device_));
- EXPECT_EQ("255.255.255.255", config->GetIPv4AddressString(0xffffffff));
- EXPECT_EQ("0.0.0.0", config->GetIPv4AddressString(0));
- EXPECT_EQ("1.2.3.4", config->GetIPv4AddressString(0x04030201));
+ EXPECT_EQ("255.255.255.255", config_->GetIPv4AddressString(0xffffffff));
+ EXPECT_EQ("0.0.0.0", config_->GetIPv4AddressString(0));
+ EXPECT_EQ("1.2.3.4", config_->GetIPv4AddressString(0x04030201));
}
TEST_F(DHCPConfigTest, ParseConfiguration) {
@@ -38,40 +45,35 @@
conf[DHCPConfig::kConfigurationKeyBroadcastAddress].writer().append_uint32(
0x10203040);
{
- DBus::Variant var;
vector<unsigned int> routers;
routers.push_back(0x02040608);
routers.push_back(0x03050709);
- DBus::MessageIter writer = var.writer();
+ DBus::MessageIter writer =
+ conf[DHCPConfig::kConfigurationKeyRouters].writer();
writer << routers;
- conf[DHCPConfig::kConfigurationKeyRouters] = var;
}
{
- DBus::Variant var;
vector<unsigned int> dns;
dns.push_back(0x09070503);
dns.push_back(0x08060402);
- DBus::MessageIter writer = var.writer();
+ DBus::MessageIter writer = conf[DHCPConfig::kConfigurationKeyDNS].writer();
writer << dns;
- conf[DHCPConfig::kConfigurationKeyDNS] = var;
}
conf[DHCPConfig::kConfigurationKeyDomainName].writer().append_string(
"domain-name");
{
- DBus::Variant var;
vector<string> search;
search.push_back("foo.com");
search.push_back("bar.com");
- DBus::MessageIter writer = var.writer();
+ DBus::MessageIter writer =
+ conf[DHCPConfig::kConfigurationKeyDomainSearch].writer();
writer << search;
- conf[DHCPConfig::kConfigurationKeyDomainSearch] = var;
}
conf[DHCPConfig::kConfigurationKeyMTU].writer().append_uint16(600);
conf["UnknownKey"] = DBus::Variant();
- DHCPConfigRefPtr config(new DHCPConfig(NULL, device_));
IPConfig::Properties properties;
- ASSERT_TRUE(config->ParseConfiguration(conf, &properties));
+ ASSERT_TRUE(config_->ParseConfiguration(conf, &properties));
EXPECT_EQ("4.3.2.1", properties.address);
EXPECT_EQ(16, properties.subnet_cidr);
EXPECT_EQ("64.48.32.16", properties.broadcast_address);
@@ -86,4 +88,20 @@
EXPECT_EQ(600, properties.mtu);
}
+TEST_F(DHCPConfigTest, Start) {
+ EXPECT_CALL(glib_, SpawnAsync(_, _, _, _, _, _, _, _))
+ .WillOnce(Return(false));
+ EXPECT_FALSE(config_->Start());
+ EXPECT_EQ(0, config_->pid_);
+
+ const unsigned int kPID = 1234;
+ EXPECT_CALL(glib_, SpawnAsync(_, _, _, _, _, _, _, _))
+ .WillOnce(DoAll(SetArgumentPointee<6>(kPID), Return(true)));
+ EXPECT_TRUE(config_->Start());
+ EXPECT_EQ(kPID, config_->pid_);
+ DHCPProvider *provider = DHCPProvider::GetInstance();
+ ASSERT_TRUE(provider->configs_.find(kPID) != provider->configs_.end());
+ EXPECT_EQ(config_.get(), provider->configs_.find(1234)->second.get());
+}
+
} // namespace shill
diff --git a/dhcp_provider.cc b/dhcp_provider.cc
index e082d5e..c506a77 100644
--- a/dhcp_provider.cc
+++ b/dhcp_provider.cc
@@ -10,7 +10,7 @@
namespace shill {
-DHCPProvider::DHCPProvider() {
+DHCPProvider::DHCPProvider() : glib_(NULL) {
VLOG(2) << __func__;
}
@@ -22,14 +22,16 @@
return Singleton<DHCPProvider>::get();
}
-void DHCPProvider::Init(DBus::Connection *connection) {
+void DHCPProvider::Init(DBus::Connection *connection,
+ GLibInterface *glib) {
VLOG(2) << __func__;
listener_.reset(new DHCPCDListener(this, connection));
+ glib_ = glib;
}
DHCPConfigRefPtr DHCPProvider::CreateConfig(DeviceConstRefPtr device) {
VLOG(2) << __func__;
- return DHCPConfigRefPtr(new DHCPConfig(this, device));
+ return DHCPConfigRefPtr(new DHCPConfig(this, device, glib_));
}
DHCPConfigRefPtr DHCPProvider::GetConfig(unsigned int pid) {
diff --git a/dhcp_provider.h b/dhcp_provider.h
index 06680fd..28e07e4 100644
--- a/dhcp_provider.h
+++ b/dhcp_provider.h
@@ -10,6 +10,7 @@
#include <base/memory/scoped_ptr.h>
#include <base/memory/singleton.h>
#include <dbus-c++/connection.h>
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/device.h"
#include "shill/dhcp_config.h"
@@ -33,7 +34,7 @@
// Initializes the provider singleton. This method hooks up a D-Bus signal
// listener to |connection| that catches signals from spawned DHCP clients and
// dispatches them to the appropriate DHCP configuration instance.
- void Init(DBus::Connection *connection);
+ void Init(DBus::Connection *connection, GLibInterface *glib);
// Creates a new DHCPConfig for |device|. The DHCP configuration for the
// device can then be initiated through DHCPConfig::Request and
@@ -55,6 +56,8 @@
private:
friend struct DefaultSingletonTraits<DHCPProvider>;
+ FRIEND_TEST(DHCPConfigTest, Start);
+
typedef std::map<unsigned int, DHCPConfigRefPtr> PIDConfigMap;
// Private to ensure that this behaves as a singleton.
@@ -68,6 +71,8 @@
// A map that binds PIDs to DHCP configuration instances.
PIDConfigMap configs_;
+ GLibInterface *glib_;
+
DISALLOW_COPY_AND_ASSIGN(DHCPProvider);
};
diff --git a/glib.cc b/glib.cc
new file mode 100644
index 0000000..dc33470
--- /dev/null
+++ b/glib.cc
@@ -0,0 +1,27 @@
+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "shill/glib.h"
+
+namespace shill {
+
+gboolean GLib::SpawnAsync(const gchar *working_directory,
+ gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ GError **error) {
+ return g_spawn_async(working_directory,
+ argv,
+ envp,
+ flags,
+ child_setup,
+ user_data,
+ child_pid,
+ error);
+}
+
+} // namespace shill
diff --git a/glib.h b/glib.h
new file mode 100644
index 0000000..274d3a2
--- /dev/null
+++ b/glib.h
@@ -0,0 +1,27 @@
+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SHILL_GLIB_H_
+#define SHILL_GLIB_H_
+
+#include "shill/glib_interface.h"
+
+namespace shill {
+
+// A concrete implementation of the GLib interface.
+class GLib : public GLibInterface {
+ public:
+ virtual gboolean SpawnAsync(const gchar *working_directory,
+ gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ GError **error);
+};
+
+} // namespace shill
+
+#endif // SHILL_GLIB_H_
diff --git a/glib_interface.h b/glib_interface.h
new file mode 100644
index 0000000..37b3405
--- /dev/null
+++ b/glib_interface.h
@@ -0,0 +1,30 @@
+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SHILL_GLIB_INTERFACE_H_
+#define SHILL_GLIB_INTERFACE_H_
+
+#include <glib.h>
+
+namespace shill {
+
+// A virtual GLib interface allowing GLib mocking in tests.
+class GLibInterface {
+ public:
+ virtual ~GLibInterface() {}
+
+ // g_spawn_async
+ virtual gboolean SpawnAsync(const gchar *working_directory,
+ gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ GError **error) = 0;
+};
+
+} // namespace shill
+
+#endif // SHILL_GLIB_INTERFACE_H_
diff --git a/mock_glib.h b/mock_glib.h
new file mode 100644
index 0000000..073217f
--- /dev/null
+++ b/mock_glib.h
@@ -0,0 +1,28 @@
+// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SHILL_MOCK_GLIB_H_
+#define SHILL_MOCK_GLIB_H_
+
+#include <gmock/gmock.h>
+
+#include "shill/glib_interface.h"
+
+namespace shill {
+
+class MockGLib : public GLibInterface {
+ public:
+ MOCK_METHOD8(SpawnAsync, gboolean(const gchar *working_directory,
+ gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ GError **error));
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_GLIB_H_
diff --git a/shill_main.cc b/shill_main.cc
index 27ead0e..0039e09 100644
--- a/shill_main.cc
+++ b/shill_main.cc
@@ -14,6 +14,7 @@
#include "shill/dbus_control.h"
#include "shill/dhcp_provider.h"
+#include "shill/glib.h"
#include "shill/shill_daemon.h"
using std::string;
@@ -94,7 +95,8 @@
// TODO(pstew): This should be chosen based on config
shill::DBusControl *dbus_control = new shill::DBusControl();
dbus_control->Init();
- shill::DHCPProvider::GetInstance()->Init(dbus_control->connection());
+ shill::GLib glib;
+ shill::DHCPProvider::GetInstance()->Init(dbus_control->connection(), &glib);
shill::Daemon daemon(&config, dbus_control);
daemon.Run();