shill: L2TPIPSecDriver: add test for another Notify case
If we already have a |device_| when L2TPIPsecDriver::Notify
is called, we don't create a new PPPDevice. This case didn't
have test coverage before, so this CL adds it.
While there: refactor some code, to reduce duplication
across test cases.
BUG=chromium:246826
TEST=unit tests (old+new)
Change-Id: Ic7325cabfd28f9db4efd90c0f7b765c1f48d4901
Reviewed-on: https://gerrit.chromium.org/gerrit/64263
Reviewed-by: Ben Chan <benchan@chromium.org>
Commit-Queue: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>
diff --git a/l2tp_ipsec_driver.cc b/l2tp_ipsec_driver.cc
index bcbc0dc..8f6950d 100644
--- a/l2tp_ipsec_driver.cc
+++ b/l2tp_ipsec_driver.cc
@@ -37,6 +37,7 @@
#include "shill/manager.h"
#include "shill/nss.h"
#include "shill/ppp_device.h"
+#include "shill/ppp_device_factory.h"
#include "shill/vpn_service.h"
using base::Bind;
@@ -101,6 +102,7 @@
device_info_(device_info),
glib_(glib),
nss_(NSS::GetInstance()),
+ ppp_device_factory_(PPPDeviceFactory::GetInstance()),
certificate_file_(new CertificateFile()),
weak_ptr_factory_(this) {}
@@ -411,8 +413,9 @@
bool blackhole_ipv6 = true;
if (!device_) {
- device_ = new PPPDevice(control_, dispatcher(), metrics_, manager(),
- interface_name, interface_index);
+ device_ = ppp_device_factory_->CreatePPPDevice(
+ control_, dispatcher(), metrics_, manager(), interface_name,
+ interface_index);
}
device_->SetEnabled(true);
device_->SelectService(service_);
diff --git a/l2tp_ipsec_driver.h b/l2tp_ipsec_driver.h
index a6833ad..2830e6a 100644
--- a/l2tp_ipsec_driver.h
+++ b/l2tp_ipsec_driver.h
@@ -27,6 +27,7 @@
class GLib;
class Metrics;
class NSS;
+class PPPDeviceFactory;
class L2TPIPSecDriver : public VPNDriver,
public RPCTaskDelegate {
@@ -64,6 +65,8 @@
FRIEND_TEST(L2TPIPSecDriverTest, InitOptions);
FRIEND_TEST(L2TPIPSecDriverTest, InitOptionsNoHost);
FRIEND_TEST(L2TPIPSecDriverTest, InitPSKOptions);
+ FRIEND_TEST(L2TPIPSecDriverTest, Notify);
+ FRIEND_TEST(L2TPIPSecDriverTest, NotifyWithExistingDevice);
FRIEND_TEST(L2TPIPSecDriverTest, NotifyDisconnected);
FRIEND_TEST(L2TPIPSecDriverTest, OnConnectionDisconnected);
FRIEND_TEST(L2TPIPSecDriverTest, OnL2TPIPSecVPNDied);
@@ -129,6 +132,7 @@
DeviceInfo *device_info_;
GLib *glib_;
NSS *nss_;
+ PPPDeviceFactory *ppp_device_factory_;
VPNServiceRefPtr service_;
scoped_ptr<ExternalTask> external_task_;
diff --git a/l2tp_ipsec_driver_unittest.cc b/l2tp_ipsec_driver_unittest.cc
index 2f9a8b0..12831f4 100644
--- a/l2tp_ipsec_driver_unittest.cc
+++ b/l2tp_ipsec_driver_unittest.cc
@@ -22,6 +22,7 @@
#include "shill/mock_metrics.h"
#include "shill/mock_nss.h"
#include "shill/mock_ppp_device.h"
+#include "shill/mock_ppp_device_factory.h"
#include "shill/mock_vpn_service.h"
using base::FilePath;
@@ -115,6 +116,10 @@
return driver_->IsConnectTimeoutStarted();
}
+ bool IsPSKFileCleared(const FilePath &psk_file_path) {
+ return !file_util::PathExists(psk_file_path) && GetPSKFile().empty();
+ }
+
// Used to assert that a flag appears in the options.
void ExpectInFlags(const vector<string> &options, const string &flag,
const string &value);
@@ -127,6 +132,41 @@
driver_->Notify(reason, dict);
}
+ FilePath FakeUpConnect() {
+ FilePath new_psk_file = SetupPSKFile();
+ SetService(service_);
+ StartConnectTimeout(0);
+ return new_psk_file;
+ }
+
+ void ExpectDeviceConnected(const map<string, string> &ppp_config) {
+ EXPECT_CALL(*device_, SetEnabled(true));
+ EXPECT_CALL(*device_, SelectService(static_cast<ServiceRefPtr>(service_)));
+ EXPECT_CALL(*device_, UpdateIPConfigFromPPP(ppp_config, _));
+ }
+
+ void ExpectMetricsReported() {
+ Error unused_error;
+ PropertyStore store;
+ driver_->InitPropertyStore(&store);
+ store.SetStringProperty(flimflam::kL2tpIpsecPskProperty, "x",
+ &unused_error);
+ store.SetStringProperty(flimflam::kL2tpIpsecPasswordProperty, "y",
+ &unused_error);
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ Metrics::kMetricVpnDriver,
+ Metrics::kVpnDriverL2tpIpsec,
+ Metrics::kMetricVpnDriverMax));
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ Metrics::kMetricVpnRemoteAuthenticationType,
+ Metrics::kVpnRemoteAuthenticationTypeL2tpIpsecPsk,
+ Metrics::kVpnRemoteAuthenticationTypeMax));
+ EXPECT_CALL(metrics_, SendEnumToUMA(
+ Metrics::kMetricVpnUserAuthenticationType,
+ Metrics::kVpnUserAuthenticationTypeL2tpIpsecUsernamePassword,
+ Metrics::kVpnUserAuthenticationTypeMax));
+ }
+
// Inherited from RPCTaskDelegate.
virtual void GetLogin(string *user, string *password);
virtual void Notify(const string &reason, const map<string, string> &dict);
@@ -182,8 +222,8 @@
TEST_F(L2TPIPSecDriverTest, Cleanup) {
driver_->IdleService(); // Ensure no crash.
+ FilePath psk_file = FakeUpConnect();
driver_->device_ = device_;
- driver_->service_ = service_;
driver_->external_task_.reset(
new MockExternalTask(&control_,
&glib_,
@@ -192,11 +232,8 @@
EXPECT_CALL(*device_, DropConnection());
EXPECT_CALL(*device_, SetEnabled(false));
EXPECT_CALL(*service_, SetFailure(Service::kFailureBadPassphrase));
- FilePath psk_file = SetupPSKFile();
- StartConnectTimeout(0);
driver_->FailService(Service::kFailureBadPassphrase); // Trigger Cleanup.
- EXPECT_FALSE(file_util::PathExists(psk_file));
- EXPECT_TRUE(driver_->psk_file_.empty());
+ EXPECT_TRUE(IsPSKFileCleared(psk_file));
EXPECT_FALSE(driver_->device_);
EXPECT_FALSE(driver_->service_);
EXPECT_FALSE(driver_->IsConnectTimeoutStarted());
@@ -211,8 +248,7 @@
TEST_F(L2TPIPSecDriverTest, DeletePSKFile) {
FilePath psk_file = SetupPSKFile();
driver_->DeletePSKFile();
- EXPECT_FALSE(file_util::PathExists(psk_file));
- EXPECT_TRUE(driver_->psk_file_.empty());
+ EXPECT_TRUE(IsPSKFileCleared(psk_file));
}
TEST_F(L2TPIPSecDriverTest, InitOptionsNoHost) {
@@ -426,7 +462,9 @@
EXPECT_CALL(glib_, ChildWatchAdd(_, _, _));
EXPECT_FALSE(driver_->SpawnL2TPIPSecVPN(&error));
+ EXPECT_FALSE(driver_->external_task_);
EXPECT_TRUE(driver_->SpawnL2TPIPSecVPN(&error));
+ EXPECT_TRUE(driver_->external_task_);
}
TEST_F(L2TPIPSecDriverTest, Connect) {
@@ -524,41 +562,39 @@
} // namespace
TEST_F(L2TPIPSecDriverTest, Notify) {
- map<string, string> config;
- config[kPPPInterfaceName] = kInterfaceName;
+ map<string, string> config{{kPPPInterfaceName, kInterfaceName}};
+ MockPPPDeviceFactory *mock_ppp_device_factory =
+ MockPPPDeviceFactory::GetInstance();
+ FilePath psk_file = FakeUpConnect();
+ driver_->ppp_device_factory_ = mock_ppp_device_factory;
EXPECT_CALL(device_info_, GetIndex(kInterfaceName))
.WillOnce(Return(kInterfaceIndex));
- EXPECT_CALL(*device_, SetEnabled(true));
- EXPECT_CALL(*device_, SelectService(static_cast<ServiceRefPtr>(service_)));
- EXPECT_CALL(*device_, UpdateIPConfigFromPPP(config, _));
- SetDevice(device_);
- SetService(service_);
- FilePath psk_file = SetupPSKFile();
- StartConnectTimeout(0);
-
- EXPECT_CALL(metrics_, SendEnumToUMA(
- Metrics::kMetricVpnDriver,
- Metrics::kVpnDriverL2tpIpsec,
- Metrics::kMetricVpnDriverMax));
- EXPECT_CALL(metrics_, SendEnumToUMA(
- Metrics::kMetricVpnRemoteAuthenticationType,
- Metrics::kVpnRemoteAuthenticationTypeL2tpIpsecPsk,
- Metrics::kVpnRemoteAuthenticationTypeMax));
- EXPECT_CALL(metrics_, SendEnumToUMA(
- Metrics::kMetricVpnUserAuthenticationType,
- Metrics::kVpnUserAuthenticationTypeL2tpIpsecUsernamePassword,
- Metrics::kVpnUserAuthenticationTypeMax));
-
- Error unused_error;
- PropertyStore store;
- driver_->InitPropertyStore(&store);
- store.SetStringProperty(flimflam::kL2tpIpsecPskProperty, "x", &unused_error);
- store.SetStringProperty(flimflam::kL2tpIpsecPasswordProperty, "y",
- &unused_error);
-
+ EXPECT_CALL(*mock_ppp_device_factory,
+ CreatePPPDevice(_, _, _, _, kInterfaceName, kInterfaceIndex))
+ .WillOnce(Return(device_));
+ ExpectDeviceConnected(config);
+ ExpectMetricsReported();
InvokeNotify(kPPPReasonConnect, config);
- EXPECT_FALSE(file_util::PathExists(psk_file));
- EXPECT_TRUE(GetPSKFile().empty());
+ EXPECT_TRUE(IsPSKFileCleared(psk_file));
+ EXPECT_FALSE(IsConnectTimeoutStarted());
+}
+
+
+TEST_F(L2TPIPSecDriverTest, NotifyWithExistingDevice) {
+ map<string, string> config{{kPPPInterfaceName, kInterfaceName}};
+ MockPPPDeviceFactory *mock_ppp_device_factory =
+ MockPPPDeviceFactory::GetInstance();
+ FilePath psk_file = FakeUpConnect();
+ driver_->ppp_device_factory_ = mock_ppp_device_factory;
+ SetDevice(device_);
+ EXPECT_CALL(device_info_, GetIndex(kInterfaceName))
+ .WillOnce(Return(kInterfaceIndex));
+ EXPECT_CALL(*mock_ppp_device_factory,
+ CreatePPPDevice(_, _, _, _, _, _)).Times(0);
+ ExpectDeviceConnected(config);
+ ExpectMetricsReported();
+ InvokeNotify(kPPPReasonConnect, config);
+ EXPECT_TRUE(IsPSKFileCleared(psk_file));
EXPECT_FALSE(IsConnectTimeoutStarted());
}