Improve testing of WiFi bootstrapping condition
BUG=23755250,24054100
Change-Id: Ieefbc9f5b23ba4e29a89e43c62fefebd76646bc3
diff --git a/libweave/include/weave/test/mock_task_runner.h b/libweave/include/weave/test/mock_task_runner.h
index 5fee077..0aa3186 100644
--- a/libweave/include/weave/test/mock_task_runner.h
+++ b/libweave/include/weave/test/mock_task_runner.h
@@ -30,6 +30,7 @@
bool RunOnce();
void Run();
+ void Break();
base::Clock* GetClock();
private:
@@ -45,6 +46,7 @@
}
};
+ bool break_{false};
size_t counter_{0}; // Keeps order of tasks with the same time.
class TestClock;
diff --git a/libweave/src/privet/cloud_delegate.cc b/libweave/src/privet/cloud_delegate.cc
index c7d8c24..cce7994 100644
--- a/libweave/src/privet/cloud_delegate.cc
+++ b/libweave/src/privet/cloud_delegate.cc
@@ -76,6 +76,7 @@
return true;
}
+ // TODO(vitalybuka): Remove error.
bool GetName(std::string* name, ErrorPtr* error) const override {
*name = device_->GetConfig().name();
return true;
diff --git a/libweave/src/privet/wifi_bootstrap_manager.cc b/libweave/src/privet/wifi_bootstrap_manager.cc
index 5a32419..6dd044d 100644
--- a/libweave/src/privet/wifi_bootstrap_manager.cc
+++ b/libweave/src/privet/wifi_bootstrap_manager.cc
@@ -16,12 +16,6 @@
namespace weave {
namespace privet {
-namespace {
-const int kConnectTimeoutMinutes = 3;
-const int kBootstrapTimeoutMinutes = 10;
-const int kMonitorTimeoutMinutes = 2;
-}
-
WifiBootstrapManager::WifiBootstrapManager(
const std::string& last_configured_ssid,
const std::string& test_privet_ssid,
@@ -81,7 +75,7 @@
task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&WifiBootstrapManager::OnBootstrapTimeout,
tasks_weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromMinutes(kBootstrapTimeoutMinutes));
+ base::TimeDelta::FromMinutes(10));
}
// TODO(vitalybuka): Add SSID probing.
privet_ssid_ = GenerateSsid();
@@ -104,7 +98,7 @@
task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&WifiBootstrapManager::OnConnectTimeout,
tasks_weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromMinutes(kConnectTimeoutMinutes));
+ base::TimeDelta::FromMinutes(3));
network_->ConnectToService(ssid, passphrase,
base::Bind(&WifiBootstrapManager::OnConnectSuccess,
tasks_weak_factory_.GetWeakPtr(), ssid),
@@ -119,6 +113,19 @@
// We already have a callback in place with |network_| to update our
// connectivity state. See OnConnectivityChange().
UpdateState(State::kMonitoring);
+
+ if (network_->GetConnectionState() == NetworkState::kConnected) {
+ monitor_until_ = base::Time::Max();
+ } else {
+ if (monitor_until_ == base::Time::Max())
+ monitor_until_ = base::Time::Now() + base::TimeDelta::FromMinutes(2);
+
+ // Schedule timeout timer taking into account already offline time.
+ task_runner_->PostDelayedTask(
+ FROM_HERE, base::Bind(&WifiBootstrapManager::OnMonitorTimeout,
+ tasks_weak_factory_.GetWeakPtr()),
+ monitor_until_ - base::Time::Now());
+ }
}
void WifiBootstrapManager::EndMonitoring() {
@@ -234,22 +241,9 @@
VLOG(3) << "ConnectivityChanged: " << is_connected;
UpdateConnectionState();
- if (state_ == State::kBootstrapping && is_connected) {
+ if (state_ == State::kMonitoring || // Reset monitoring timeout.
+ (state_ == State::kDisabled && is_connected)) {
StartMonitoring();
- return;
- }
- if (state_ == State::kMonitoring) {
- if (is_connected) {
- tasks_weak_factory_.InvalidateWeakPtrs();
- } else {
- // Tasks queue may have more than one OnMonitorTimeout enqueued. The
- // first one could be executed as it would change the state and abort the
- // rest.
- task_runner_->PostDelayedTask(
- FROM_HERE, base::Bind(&WifiBootstrapManager::OnMonitorTimeout,
- tasks_weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromMinutes(kMonitorTimeoutMinutes));
- }
}
}
diff --git a/libweave/src/privet/wifi_bootstrap_manager.h b/libweave/src/privet/wifi_bootstrap_manager.h
index f622c77..c0052a0 100644
--- a/libweave/src/privet/wifi_bootstrap_manager.h
+++ b/libweave/src/privet/wifi_bootstrap_manager.h
@@ -101,6 +101,7 @@
TaskRunner* task_runner_{nullptr};
Network* network_{nullptr};
WifiSsidGenerator ssid_generator_;
+ base::Time monitor_until_;
std::vector<StateListener> state_listeners_;
bool currently_online_{false};
diff --git a/libweave/src/test/mock_task_runner.cc b/libweave/src/test/mock_task_runner.cc
index da9246a..a78b4e1 100644
--- a/libweave/src/test/mock_task_runner.cc
+++ b/libweave/src/test/mock_task_runner.cc
@@ -40,10 +40,15 @@
}
void MockTaskRunner::Run() {
- while (RunOnce()) {
+ break_ = false;
+ while (!break_ && RunOnce()) {
}
}
+void MockTaskRunner::Break() {
+ break_ = true;
+}
+
base::Clock* MockTaskRunner::GetClock() {
return test_clock_.get();
}
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc
index 7b3bbdb..0629dac 100644
--- a/libweave/src/weave_unittest.cc
+++ b/libweave/src/weave_unittest.cc
@@ -221,11 +221,18 @@
void InitNetwork() {
EXPECT_CALL(network_, AddOnConnectionChangedCallback(_))
- .WillRepeatedly(Return());
+ .WillRepeatedly(
+ Invoke([this](const Network::OnConnectionChangedCallback& cb) {
+ network_callbacks_.push_back(cb);
+ }));
EXPECT_CALL(network_, GetConnectionState())
.WillRepeatedly(Return(NetworkState::kOffline));
- EXPECT_CALL(network_, EnableAccessPoint(MatchesRegex("DEVICE_NAME.*prv")))
- .WillOnce(Return());
+ }
+
+ void IgnoreMdns() {
+ EXPECT_CALL(mdns_, GetId()).WillRepeatedly(Return("TEST_ID"));
+ EXPECT_CALL(mdns_, PublishService(_, _, _)).WillRepeatedly(Return());
+ EXPECT_CALL(mdns_, StopPublishing("_privet._tcp")).WillOnce(Return());
}
void InitMdns() {
@@ -272,12 +279,16 @@
}));
}
- void StartDevice() {
+ void InitDefaultExpectations() {
InitConfigStore();
InitNetwork();
+ EXPECT_CALL(network_, EnableAccessPoint(MatchesRegex("DEVICE_NAME.*prv")))
+ .WillOnce(Return());
InitHttpServer();
InitMdns();
+ }
+ void StartDevice() {
weave::Device::Options options;
options.xmpp_enabled = false;
@@ -311,6 +322,8 @@
StrictMock<test::MockMdns> mdns_;
StrictMock<test::MockHttpServer> http_server_;
+ std::vector<Network::OnConnectionChangedCallback> network_callbacks_;
+
weave::Cloud* cloud_{nullptr};
std::unique_ptr<weave::Device> device_;
@@ -331,11 +344,19 @@
&network_, nullptr, nullptr);
}
-TEST_F(WeaveTest, Start) {
+class WeaveBasicTest : public WeaveTest {
+ public:
+ void SetUp() override {
+ WeaveTest::SetUp();
+ InitDefaultExpectations();
+ }
+};
+
+TEST_F(WeaveBasicTest, Start) {
StartDevice();
}
-TEST_F(WeaveTest, Register) {
+TEST_F(WeaveBasicTest, Register) {
StartDevice();
auto draft = CreateDictionaryValue(kDeviceResource);
@@ -363,4 +384,94 @@
EXPECT_EQ("DEVICE_ID", cloud_->RegisterDevice("TEST_ID", nullptr));
}
+class WeaveWiFiSetupTest : public WeaveTest {
+ public:
+ void SetUp() override {
+ WeaveTest::SetUp();
+
+ InitConfigStore();
+ InitHttpServer();
+ InitNetwork();
+ IgnoreMdns();
+
+ EXPECT_CALL(network_, GetConnectionState())
+ .WillRepeatedly(Return(NetworkState::kConnected));
+ }
+};
+
+TEST_F(WeaveWiFiSetupTest, StartOnlineNoPrevSsid) {
+ StartDevice();
+
+ // Short disconnect.
+ EXPECT_CALL(network_, GetConnectionState())
+ .WillRepeatedly(Return(NetworkState::kOffline));
+ for (const auto& cb : network_callbacks_) {
+ task_runner_.PostDelayedTask(FROM_HERE, base::Bind(cb, false), {});
+ }
+ EXPECT_CALL(network_, GetConnectionState())
+ .WillRepeatedly(Return(NetworkState::kConnected));
+ for (const auto& cb : network_callbacks_) {
+ task_runner_.PostDelayedTask(FROM_HERE, base::Bind(cb, true),
+ base::TimeDelta::FromSeconds(10));
+ }
+ task_runner_.Run();
+
+ // Long disconnect.
+ EXPECT_CALL(network_, GetConnectionState())
+ .WillRepeatedly(Return(NetworkState::kOffline));
+ for (const auto& cb : network_callbacks_) {
+ task_runner_.PostDelayedTask(FROM_HERE, base::Bind(cb, false), {});
+ }
+ auto offline_from = task_runner_.GetClock()->Now();
+ EXPECT_CALL(network_, EnableAccessPoint(MatchesRegex("DEVICE_NAME.*prv")))
+ .WillOnce(InvokeWithoutArgs([this, offline_from]() {
+ EXPECT_GT(task_runner_.GetClock()->Now() - offline_from,
+ base::TimeDelta::FromMinutes(1));
+ }));
+ task_runner_.Run();
+}
+
+// If device has previously configured WiFi it will run AP for limited time
+// after which it will try to re-connect.
+TEST_F(WeaveWiFiSetupTest, StartOnlineWithPrevSsid) {
+ EXPECT_CALL(config_store_, LoadSettings())
+ .WillRepeatedly(Return(R"({"last_configured_ssid": "TEST_ssid"})"));
+ StartDevice();
+
+ // Long disconnect.
+ EXPECT_CALL(network_, GetConnectionState())
+ .WillRepeatedly(Return(NetworkState::kOffline));
+ for (const auto& cb : network_callbacks_) {
+ task_runner_.PostDelayedTask(FROM_HERE, base::Bind(cb, false), {});
+ }
+
+ for (int i = 0; i < 5; ++i) {
+ auto offline_from = task_runner_.GetClock()->Now();
+ // Temporarily offline mode.
+ EXPECT_CALL(network_, EnableAccessPoint(MatchesRegex("DEVICE_NAME.*prv")))
+ .WillOnce(InvokeWithoutArgs([this, &offline_from]() {
+ EXPECT_GT(task_runner_.GetClock()->Now() - offline_from,
+ base::TimeDelta::FromMinutes(1));
+ task_runner_.Break();
+ }));
+ task_runner_.Run();
+
+ // Try to reconnect again.
+ offline_from = task_runner_.GetClock()->Now();
+ EXPECT_CALL(network_, DisableAccessPoint())
+ .WillOnce(InvokeWithoutArgs([this, offline_from]() {
+ EXPECT_GT(task_runner_.GetClock()->Now() - offline_from,
+ base::TimeDelta::FromMinutes(5));
+ task_runner_.Break();
+ }));
+ task_runner_.Run();
+ }
+
+ EXPECT_CALL(network_, GetConnectionState())
+ .WillRepeatedly(Return(NetworkState::kConnected));
+ for (const auto& cb : network_callbacks_)
+ task_runner_.PostDelayedTask(FROM_HERE, base::Bind(cb, true), {});
+ task_runner_.Run();
+}
+
} // namespace weave
