Revert "Improve testing of WiFi bootstrapping condition" Testing BUG: integration. This reverts commit 38d582f519f7d6a642c5a013e43d24dea1fd7185. Change-Id: I627610eb48e5077eda2db62574b653fc969e0e2a
diff --git a/libweave/include/weave/test/mock_task_runner.h b/libweave/include/weave/test/mock_task_runner.h index 0aa3186..5fee077 100644 --- a/libweave/include/weave/test/mock_task_runner.h +++ b/libweave/include/weave/test/mock_task_runner.h
@@ -30,7 +30,6 @@ bool RunOnce(); void Run(); - void Break(); base::Clock* GetClock(); private: @@ -46,7 +45,6 @@ } }; - 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 cce7994..c7d8c24 100644 --- a/libweave/src/privet/cloud_delegate.cc +++ b/libweave/src/privet/cloud_delegate.cc
@@ -76,7 +76,6 @@ 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 6dd044d..5a32419 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.cc +++ b/libweave/src/privet/wifi_bootstrap_manager.cc
@@ -16,6 +16,12 @@ 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, @@ -75,7 +81,7 @@ task_runner_->PostDelayedTask( FROM_HERE, base::Bind(&WifiBootstrapManager::OnBootstrapTimeout, tasks_weak_factory_.GetWeakPtr()), - base::TimeDelta::FromMinutes(10)); + base::TimeDelta::FromMinutes(kBootstrapTimeoutMinutes)); } // TODO(vitalybuka): Add SSID probing. privet_ssid_ = GenerateSsid(); @@ -98,7 +104,7 @@ task_runner_->PostDelayedTask( FROM_HERE, base::Bind(&WifiBootstrapManager::OnConnectTimeout, tasks_weak_factory_.GetWeakPtr()), - base::TimeDelta::FromMinutes(3)); + base::TimeDelta::FromMinutes(kConnectTimeoutMinutes)); network_->ConnectToService(ssid, passphrase, base::Bind(&WifiBootstrapManager::OnConnectSuccess, tasks_weak_factory_.GetWeakPtr(), ssid), @@ -113,19 +119,6 @@ // 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() { @@ -241,9 +234,22 @@ VLOG(3) << "ConnectivityChanged: " << is_connected; UpdateConnectionState(); - if (state_ == State::kMonitoring || // Reset monitoring timeout. - (state_ == State::kDisabled && is_connected)) { + if (state_ == State::kBootstrapping && 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 c0052a0..f622c77 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.h +++ b/libweave/src/privet/wifi_bootstrap_manager.h
@@ -101,7 +101,6 @@ 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 a78b4e1..da9246a 100644 --- a/libweave/src/test/mock_task_runner.cc +++ b/libweave/src/test/mock_task_runner.cc
@@ -40,15 +40,10 @@ } void MockTaskRunner::Run() { - break_ = false; - while (!break_ && RunOnce()) { + while (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 0629dac..7b3bbdb 100644 --- a/libweave/src/weave_unittest.cc +++ b/libweave/src/weave_unittest.cc
@@ -221,18 +221,11 @@ void InitNetwork() { EXPECT_CALL(network_, AddOnConnectionChangedCallback(_)) - .WillRepeatedly( - Invoke([this](const Network::OnConnectionChangedCallback& cb) { - network_callbacks_.push_back(cb); - })); + .WillRepeatedly(Return()); EXPECT_CALL(network_, GetConnectionState()) .WillRepeatedly(Return(NetworkState::kOffline)); - } - - void IgnoreMdns() { - EXPECT_CALL(mdns_, GetId()).WillRepeatedly(Return("TEST_ID")); - EXPECT_CALL(mdns_, PublishService(_, _, _)).WillRepeatedly(Return()); - EXPECT_CALL(mdns_, StopPublishing("_privet._tcp")).WillOnce(Return()); + EXPECT_CALL(network_, EnableAccessPoint(MatchesRegex("DEVICE_NAME.*prv"))) + .WillOnce(Return()); } void InitMdns() { @@ -279,16 +272,12 @@ })); } - void InitDefaultExpectations() { + void StartDevice() { InitConfigStore(); InitNetwork(); - EXPECT_CALL(network_, EnableAccessPoint(MatchesRegex("DEVICE_NAME.*prv"))) - .WillOnce(Return()); InitHttpServer(); InitMdns(); - } - void StartDevice() { weave::Device::Options options; options.xmpp_enabled = false; @@ -322,8 +311,6 @@ 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_; @@ -344,19 +331,11 @@ &network_, nullptr, nullptr); } -class WeaveBasicTest : public WeaveTest { - public: - void SetUp() override { - WeaveTest::SetUp(); - InitDefaultExpectations(); - } -}; - -TEST_F(WeaveBasicTest, Start) { +TEST_F(WeaveTest, Start) { StartDevice(); } -TEST_F(WeaveBasicTest, Register) { +TEST_F(WeaveTest, Register) { StartDevice(); auto draft = CreateDictionaryValue(kDeviceResource); @@ -384,94 +363,4 @@ 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