shill: Factors some common unittest code into own methods.
A code review on a previous CL suggested refactoring the StartScan and
AttemptConnection code from the unittests into their own methods. This
does that.
Found and fixed a tiny buglet where transitioning from
scanning(progressive) to scanning(full) emitted an extra dbus
signal(scanning=true).
Also made VerifyScanState 'const'.
BUG=None.
TEST=unittest.
Change-Id: Ia4d993dfd0ee425cba1a38fed2561e213f42e699
Reviewed-on: https://gerrit.chromium.org/gerrit/63712
Commit-Queue: Wade Guthrie <wdg@chromium.org>
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/wifi.cc b/wifi.cc
index 08f9084..4ea90ec 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -1946,11 +1946,11 @@
}
int log_level = 6;
- bool state_changed = true;
+ bool state_or_method_changed = true;
bool is_terminal_state = false;
if (new_state == scan_state_ && new_method == scan_method_) {
log_level = 7;
- state_changed = false;
+ state_or_method_changed = false;
} else if (new_state == kScanConnected || new_state == kScanFoundNothing) {
// These 'terminal' states are slightly more interesting than the
// intermediate states.
@@ -1975,7 +1975,7 @@
<< " -> " << ScanStateString(new_state, new_method)
<< " @ " << elapsed_time.InMillisecondsF()
<< " ms into scan.";
- if (!state_changed)
+ if (!state_or_method_changed)
return;
// Actually change the state.
@@ -1983,7 +1983,8 @@
ScanMethod old_method = scan_method_;
scan_state_ = new_state;
scan_method_ = new_method;
- if (new_state == kScanScanning || old_state == kScanScanning) {
+ if (new_state != old_state &&
+ (new_state == kScanScanning || old_state == kScanScanning)) {
Error error;
adaptor()->EmitBoolChanged(flimflam::kScanningProperty,
GetScanPending(&error));
diff --git a/wifi.h b/wifi.h
index 2e307f5..c19f3af 100644
--- a/wifi.h
+++ b/wifi.h
@@ -217,6 +217,7 @@
friend class WiFiObjectTest; // access to supplicant_*_proxy_, link_up_
friend class WiFiTimerTest; // kNumFastScanAttempts, kFastScanIntervalSeconds
+ friend class WiFiMainTest; // ScanState, ScanMethod
FRIEND_TEST(WiFiMainTest, AppendBgscan);
FRIEND_TEST(WiFiMainTest, ConnectToServiceNotPending); // ScanState
FRIEND_TEST(WiFiMainTest, ConnectToWithError); // ScanState
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 042690e..30c6c21 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -364,7 +364,7 @@
wifi_->SetScanState(new_state, new_method, reason);
}
- void VerifyScanState(WiFi::ScanState state, WiFi::ScanMethod method) {
+ void VerifyScanState(WiFi::ScanState state, WiFi::ScanMethod method) const {
EXPECT_EQ(state, wifi_->scan_state_);
EXPECT_EQ(method, wifi_->scan_method_);
}
@@ -907,6 +907,46 @@
WiFiMainTest() : WiFiObjectTest(&dispatcher_) {}
protected:
+ void StartScan(WiFi::ScanMethod method) {
+ if (method == WiFi::kScanMethodFull) {
+ EnableFullScan();
+ } else {
+ EXPECT_CALL(*scan_session_, InitiateScan());
+ }
+ VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
+ EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*metrics(), NotifyDeviceScanStarted(_));
+ StartWiFi();
+ EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
+ dispatcher_.DispatchPendingEvents();
+ VerifyScanState(WiFi::kScanScanning, method);
+ }
+
+ MockWiFiServiceRefPtr AttemptConnection(WiFi::ScanMethod method,
+ WiFiEndpointRefPtr *endpoint,
+ ::DBus::Path *bss_path) {
+ WiFiEndpointRefPtr dummy_endpoint;
+ if (!endpoint) {
+ endpoint = &dummy_endpoint; // If caller doesn't care about endpoint.
+ }
+
+ ::DBus::Path dummy_bss_path;
+ if (!bss_path) {
+ bss_path = &dummy_bss_path; // If caller doesn't care about bss_path.
+ }
+
+ EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, false));
+ EXPECT_CALL(*metrics(), NotifyDeviceScanFinished(_));
+ EXPECT_CALL(*metrics(), NotifyDeviceConnectStarted(_, _));
+ MockWiFiServiceRefPtr service =
+ SetupConnectingService(DBus::Path(), endpoint, bss_path);
+ ReportScanDoneKeepScanSession();
+ dispatcher_.DispatchPendingEvents();
+ VerifyScanState(WiFi::kScanConnecting, method);
+
+ return service;
+ }
+
EventDispatcher dispatcher_;
};
@@ -1487,29 +1527,11 @@
ScopeLogger::GetInstance()->EnableScopesByName("wifi");
ScopeLogger::GetInstance()->set_verbose_level(10);
- // Scan.
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- EXPECT_CALL(*metrics(), NotifyDeviceScanStarted(_));
- StartWiFi();
- EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
- dispatcher_.DispatchPendingEvents();
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
-
+ StartScan(WiFi::kScanMethodProgressive);
const base::CancelableClosure &pending_timeout = GetPendingTimeout();
EXPECT_TRUE(pending_timeout.IsCancelled());
-
- // Initiate a connection.
- WiFiEndpointRefPtr endpoint;
- ::DBus::Path bss_path;
- EXPECT_CALL(*metrics(), NotifyDeviceScanFinished(_));
- EXPECT_CALL(*metrics(), NotifyDeviceConnectStarted(_, _));
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, false));
- MockWiFiServiceRefPtr service =
- SetupConnectingService(DBus::Path(), &endpoint, &bss_path);
- ReportScanDoneKeepScanSession();
- dispatcher_.DispatchPendingEvents();
- VerifyScanState(WiFi::kScanConnecting, WiFi::kScanMethodProgressive);
+ MockWiFiServiceRefPtr service = AttemptConnection(
+ WiFi::kScanMethodProgressive, nullptr, nullptr);
// Timeout the connection attempt.
EXPECT_FALSE(pending_timeout.IsCancelled());
@@ -1736,13 +1758,7 @@
SetScanSize(1, 1);
// Do the first scan (finds nothing).
- EXPECT_CALL(*scan_session_, InitiateScan());
- EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- StartWiFi();
- dispatcher_.DispatchPendingEvents();
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
+ StartScan(WiFi::kScanMethodProgressive);
ReportScanDoneKeepScanSession();
// Do the second scan (connects afterwards).
@@ -1778,12 +1794,7 @@
EXPECT_CALL(*metrics(), NotifyDeviceConnectFinished(_)).Times(0);
// Do the first scan (finds nothing).
- EXPECT_CALL(*scan_session_, InitiateScan());
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- EXPECT_CALL(*metrics(), NotifyDeviceScanStarted(_));
- StartWiFi();
- dispatcher_.DispatchPendingEvents(); // Launch ProgressiveScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
+ StartScan(WiFi::kScanMethodProgressive);
ReportScanDoneKeepScanSession();
// Do the second scan (finds nothing).
@@ -2770,16 +2781,9 @@
ScopeLogger::GetInstance()->EnableScopesByName("wifi");
ScopeLogger::GetInstance()->set_verbose_level(10);
- EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- EnableFullScan();
- StartWiFi();
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
- EXPECT_CALL(*GetSupplicantInterfaceProxy(), Scan(_));
- dispatcher_.DispatchPendingEvents();
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodFull);
-
+ StartScan(WiFi::kScanMethodFull);
ReportScanDone();
+
EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
EXPECT_CALL(log, Log(_, _, HasSubstr("FULL_NOCONNECTION ->")));
EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, false));
@@ -2795,24 +2799,12 @@
ScopeLogger::GetInstance()->EnableScopesByName("wifi");
ScopeLogger::GetInstance()->set_verbose_level(10);
- EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- EnableFullScan();
- StartWiFi();
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
- dispatcher_.DispatchPendingEvents(); // Launch ScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodFull);
-
- // Setup a service and ask to connect to it.
+ StartScan(WiFi::kScanMethodFull);
WiFiEndpointRefPtr endpoint;
::DBus::Path bss_path;
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, false));
- MockWiFiServiceRefPtr service =
- SetupConnectingService(DBus::Path(), &endpoint, &bss_path);
-
- ReportScanDoneKeepScanSession();
- dispatcher_.DispatchPendingEvents(); // Launch UpdateScanStateAfterScanDone
- VerifyScanState(WiFi::kScanConnecting, WiFi::kScanMethodFull);
+ MockWiFiServiceRefPtr service = AttemptConnection(WiFi::kScanMethodFull,
+ &endpoint,
+ &bss_path);
// Complete the connection.
EXPECT_CALL(*service, NotifyCurrentEndpoint(EndpointMatch(endpoint)));
@@ -2826,30 +2818,15 @@
}
TEST_F(WiFiMainTest, ProgressiveScanConnectingToConnected) {
- ScopedMockLog log;
+ NiceScopedMockLog log;
ScopeLogger::GetInstance()->EnableScopesByName("wifi");
ScopeLogger::GetInstance()->set_verbose_level(10);
- EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
- EXPECT_CALL(*metrics(), NotifyDeviceScanStarted(_));
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- StartWiFi();
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
- dispatcher_.DispatchPendingEvents(); // Runs ProgressiveScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
-
- // Setup a service and ask to connect to it.
+ StartScan(WiFi::kScanMethodProgressive);
WiFiEndpointRefPtr endpoint;
::DBus::Path bss_path;
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, false));
- EXPECT_CALL(*metrics(), NotifyDeviceScanFinished(_));
- EXPECT_CALL(*metrics(), NotifyDeviceConnectStarted(_, _));
- MockWiFiServiceRefPtr service =
- SetupConnectingService(DBus::Path(), &endpoint, &bss_path);
-
- ReportScanDoneKeepScanSession();
- dispatcher_.DispatchPendingEvents(); // Launch ProgressiveScanTask
- VerifyScanState(WiFi::kScanConnecting, WiFi::kScanMethodProgressive);
+ MockWiFiServiceRefPtr service = AttemptConnection(
+ WiFi::kScanMethodProgressive, &endpoint, &bss_path);
// Complete the connection.
EXPECT_CALL(*service, NotifyCurrentEndpoint(EndpointMatch(endpoint)));
@@ -2864,30 +2841,13 @@
}
TEST_F(WiFiMainTest, ProgressiveScanConnectingToNotFound) {
- NiceScopedMockLog log;
-
- EXPECT_CALL(*adaptor_, EmitBoolChanged(_, _)).Times(AnyNumber());
- EXPECT_CALL(*metrics(), NotifyDeviceScanStarted(_));
- VerifyScanState(WiFi::kScanIdle, WiFi::kScanMethodNone);
- StartWiFi();
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, true));
- dispatcher_.DispatchPendingEvents(); // Runs ProgressiveScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
-
- // Setup a service and ask to connect to it.
+ StartScan(WiFi::kScanMethodProgressive);
WiFiEndpointRefPtr endpoint;
- ::DBus::Path bss_path;
- EXPECT_CALL(*adaptor_, EmitBoolChanged(flimflam::kScanningProperty, false));
- EXPECT_CALL(*metrics(), NotifyDeviceScanFinished(_));
- EXPECT_CALL(*metrics(), NotifyDeviceConnectStarted(_, _));
- MockWiFiServiceRefPtr service =
- SetupConnectingService(DBus::Path(), &endpoint, &bss_path);
-
- ReportScanDoneKeepScanSession();
- dispatcher_.DispatchPendingEvents(); // Launch ProgressiveScanTask
- VerifyScanState(WiFi::kScanConnecting, WiFi::kScanMethodProgressive);
+ MockWiFiServiceRefPtr service = AttemptConnection(
+ WiFi::kScanMethodProgressive, &endpoint, nullptr);
// Simulate connection timeout.
+ NiceScopedMockLog log;
ScopeLogger::GetInstance()->EnableScopesByName("wifi");
ScopeLogger::GetInstance()->set_verbose_level(10);
EXPECT_CALL(*service,
@@ -2933,12 +2893,7 @@
TEST_F(WiFiMainTest, ConnectToServiceNotPending) {
// Test for SetPendingService(NULL), condition a)
// |ConnectTo|->|DisconnectFrom|.
- NiceScopedMockLog log;
-
- // Start scanning.
- StartWiFi();
- dispatcher_.DispatchPendingEvents(); // Runs ProgressiveScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
+ StartScan(WiFi::kScanMethodProgressive);
// Setup pending service.
MockWiFiServiceRefPtr service_pending(
@@ -2946,6 +2901,7 @@
EXPECT_EQ(service_pending.get(), GetPendingService().get());
// ConnectTo a different service than the pending one.
+ NiceScopedMockLog log;
ScopeLogger::GetInstance()->EnableScopesByName("wifi");
ScopeLogger::GetInstance()->set_verbose_level(10);
EXPECT_CALL(*GetSupplicantInterfaceProxy(), Disconnect());
@@ -2962,9 +2918,7 @@
}
TEST_F(WiFiMainTest, ConnectToWithError) {
- StartWiFi();
- dispatcher_.DispatchPendingEvents(); // Runs ProgressiveScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
+ StartScan(WiFi::kScanMethodProgressive);
EXPECT_CALL(*GetSupplicantInterfaceProxy(), AddNetwork(_)).
WillOnce(Throw(
@@ -2983,9 +2937,7 @@
TEST_F(WiFiMainTest, ScanStateHandleDisconnect) {
// Test for SetPendingService(NULL), condition d) Disconnect while scanning.
// Start scanning.
- StartWiFi();
- dispatcher_.DispatchPendingEvents(); // Runs ProgressiveScanTask
- VerifyScanState(WiFi::kScanScanning, WiFi::kScanMethodProgressive);
+ StartScan(WiFi::kScanMethodProgressive);
// Set the pending service.
ReportScanDoneKeepScanSession();