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