Remove IsOfficialBuild() from other singleton interfaces.
IsOfficialBuild() is replicated on other singleton interfaces as a
shortcut for accessing the new HardwareInterface. These shortcuts
were used for testing when it wasn't possible to fake out this value
in a more standard way.
This patch removes the IsOfficialBuild() method from all the
singleton interfaces and uses HardwareInterface directly, which
can be faked via the default FakeHardware on MockSystemState.
This helps reduce the actual dependencies on the
UpdateCheckScheduler before migrate it to the PolicyManager.
Some minor linter issues are also solved on this patch.
BUG=chromium:358269
TEST=Unittests still pass.
Change-Id: I19d5add04b8cdc679e918cbc7fe27f688e8da64e
Reviewed-on: https://chromium-review.googlesource.com/192974
Reviewed-by: Chris Sosa <sosa@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
diff --git a/http_fetcher.h b/http_fetcher.h
index 8f0e453..07cebc9 100644
--- a/http_fetcher.h
+++ b/http_fetcher.h
@@ -119,9 +119,7 @@
ProxyResolver* proxy_resolver() const { return proxy_resolver_; }
- // These are used for testing:
- virtual void SetBuildType(bool is_official) {}
-
+ // Returns the global SystemState.
SystemState* GetSystemState() {
return system_state_;
}
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index f09bf52..9044129 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -199,7 +199,7 @@
mock_system_state_.set_connection_manager(&mock_connection_manager_);
}
- virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
+ virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
HttpFetcher* NewLargeFetcher() {
return NewLargeFetcher(1);
}
@@ -272,7 +272,7 @@
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
- ret->SetBuildType(false);
+ mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(false);
return ret;
}
@@ -323,7 +323,7 @@
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
- ret->SetBuildType(false);
+ mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(false);
return ret;
}
@@ -1057,7 +1057,8 @@
}
LOG(INFO) << "added range: " << tmp_str;
}
- multi_fetcher->SetBuildType(false);
+ dynamic_cast<MockSystemState*>(fetcher_in->GetSystemState())
+ ->get_fake_hardware()->SetIsOfficialBuild(false);
multi_fetcher->set_delegate(&delegate);
StartTransferArgs start_xfer_args = {multi_fetcher, url};
@@ -1244,7 +1245,9 @@
bool is_official_build = (i == 1);
LOG(INFO) << "is_update_allowed_over_connection: " << is_allowed;
LOG(INFO) << "is_official_build: " << is_official_build;
- fetcher->SetBuildType(is_official_build);
+ // NewLargeFetcher creates the HttpFetcher* with a MockSystemState.
+ dynamic_cast<MockSystemState*>(fetcher->GetSystemState())
+ ->get_fake_hardware()->SetIsOfficialBuild(is_official_build);
fetcher->set_delegate(&delegate);
StartTransferArgs start_xfer_args =
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 57f0595..886a446 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -56,11 +56,6 @@
return is_allowed;
}
-bool LibcurlHttpFetcher::IsOfficialBuild() const {
- return force_build_type_ ? forced_official_build_
- : system_state_->hardware()->IsOfficialBuild();
-}
-
bool LibcurlHttpFetcher::GetProxyType(const std::string& proxy,
curl_proxytype* out_type) {
if (utils::StringHasPrefix(proxy, "socks5://") ||
@@ -199,7 +194,7 @@
// If we are running in test mode or using a dev/test build, then lock down
// the appropriate curl options for HTTP or HTTPS depending on the url.
- if (!is_test_mode_ && IsOfficialBuild()) {
+ if (!is_test_mode_ && GetSystemState()->hardware()->IsOfficialBuild()) {
if (StartsWithASCII(url_to_use, "http://", false))
SetCurlOptionsForHttp();
else
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index ff0275a..cc8b7a6 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -15,6 +15,7 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/connection_manager.h"
+#include "update_engine/hardware_interface.h"
#include "update_engine/http_fetcher.h"
#include "update_engine/system_state.h"
@@ -45,8 +46,6 @@
no_network_retry_count_(0),
no_network_max_retries_(0),
idle_seconds_(1),
- force_build_type_(false),
- forced_official_build_(false),
in_write_callback_(false),
sent_byte_(false),
terminate_requested_(false),
@@ -57,7 +56,7 @@
connect_timeout_seconds_(kDownloadConnectTimeoutSeconds) {
// Dev users want a longer timeout (180 seconds) because they may
// be waiting on the dev server to build an image.
- if (!IsOfficialBuild())
+ if (!system_state->hardware()->IsOfficialBuild())
low_speed_time_seconds_ = kDownloadDevModeLowSpeedTimeSeconds;
base::Time time_oobe_complete;
if (!system_state_->IsOOBEComplete(&time_oobe_complete))
@@ -103,11 +102,6 @@
no_network_max_retries_ = retries;
}
- void SetBuildType(bool is_official) {
- force_build_type_ = true;
- forced_official_build_ = is_official;
- }
-
void set_check_certificate(
CertificateChecker::ServerToCheck check_certificate) {
check_certificate_ = check_certificate;
@@ -208,9 +202,6 @@
// False otherwise.
bool IsUpdateAllowedOverCurrentConnection() const;
- // Returns whether or not the current build is official.
- bool IsOfficialBuild() const;
-
// Sets the curl options for HTTP URL.
void SetCurlOptionsForHttp();
@@ -270,11 +261,6 @@
// Seconds to wait before asking libcurl to "perform".
int idle_seconds_;
- // If true, assume the build is official or not, according to
- // forced_official_build_. Useful for testing.
- bool force_build_type_;
- bool forced_official_build_;
-
// If true, we are currently performing a write callback on the delegate.
bool in_write_callback_;
diff --git a/multi_range_http_fetcher.h b/multi_range_http_fetcher.h
index 029dcba..8b5ace8 100644
--- a/multi_range_http_fetcher.h
+++ b/multi_range_http_fetcher.h
@@ -79,9 +79,6 @@
virtual void set_retry_seconds(int seconds) {
base_fetcher_->set_retry_seconds(seconds);
}
- virtual void SetBuildType(bool is_official) {
- base_fetcher_->SetBuildType(is_official);
- }
virtual void SetProxies(const std::deque<std::string>& proxies) {
base_fetcher_->SetProxies(proxies);
}
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 682bae7..096387b 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -5,9 +5,9 @@
#include "update_engine/update_check_scheduler.h"
#include "update_engine/certificate_checker.h"
+#include "update_engine/gpio_handler.h"
#include "update_engine/hardware_interface.h"
#include "update_engine/http_common.h"
-#include "update_engine/gpio_handler.h"
#include "update_engine/system_state.h"
#include "update_engine/utils.h"
@@ -37,7 +37,7 @@
enabled_ = false;
update_attempter_->set_update_check_scheduler(NULL);
- if (!IsOfficialBuild()) {
+ if (!system_state_->hardware()->IsOfficialBuild()) {
LOG(WARNING) << "Non-official build: periodic update checks disabled.";
return;
}
@@ -63,10 +63,6 @@
system_state_->hardware()->BootDevice()));
}
-bool UpdateCheckScheduler::IsOfficialBuild() {
- return system_state_->hardware()->IsOfficialBuild();
-}
-
guint UpdateCheckScheduler::GTimeoutAddSeconds(guint interval,
GSourceFunc function) {
return g_timeout_add_seconds(interval, function, this);
diff --git a/update_check_scheduler.h b/update_check_scheduler.h
index c7b64c6..6e95add 100644
--- a/update_check_scheduler.h
+++ b/update_check_scheduler.h
@@ -9,8 +9,8 @@
#include <glib.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
-#include "update_engine/update_attempter.h"
#include "update_engine/system_state.h"
+#include "update_engine/update_attempter.h"
namespace chromeos_update_engine {
@@ -66,7 +66,6 @@
FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzTest);
FRIEND_TEST(UpdateCheckSchedulerTest, GTimeoutAddSecondsTest);
FRIEND_TEST(UpdateCheckSchedulerTest, IsBootDeviceRemovableTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, IsOfficialBuildTest);
FRIEND_TEST(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest);
FRIEND_TEST(UpdateCheckSchedulerTest, RunNonOfficialBuildTest);
FRIEND_TEST(UpdateCheckSchedulerTest, RunTest);
@@ -87,7 +86,6 @@
// Wrappers for utils functions so that they can be mocked in tests.
virtual bool IsBootDeviceRemovable();
- virtual bool IsOfficialBuild();
// Returns true if an update check can be scheduled. An update check should
// not be scheduled if periodic update checks are disabled or if one is
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index b3497c4..62eb876 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -2,16 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "update_engine/update_check_scheduler.h"
+
#include <gtest/gtest.h>
#include "update_engine/certificate_checker.h"
#include "update_engine/certificate_checker_mock.h"
#include "update_engine/mock_system_state.h"
#include "update_engine/update_attempter_mock.h"
-#include "update_engine/update_check_scheduler.h"
using base::Time;
-using std::string;
using testing::_;
using testing::AllOf;
using testing::Assign;
@@ -28,7 +28,7 @@
*interval_min = interval - fuzz / 2;
*interval_max = interval + fuzz - fuzz / 2;
}
-} // namespace {}
+} // namespace
// Test a subclass rather than the main class directly so that we can mock out
// GLib and utils in tests. There're explicit unit test for the wrapper methods.
@@ -41,7 +41,6 @@
MOCK_METHOD2(GTimeoutAddSeconds, guint(guint seconds, GSourceFunc function));
MOCK_METHOD0(IsBootDeviceRemovable, bool());
- MOCK_METHOD0(IsOfficialBuild, bool());
MockSystemState* mock_system_state_;
};
@@ -170,14 +169,9 @@
EXPECT_FALSE(scheduler_.UpdateCheckScheduler::IsBootDeviceRemovable());
}
-TEST_F(UpdateCheckSchedulerTest, IsOfficialBuildTest) {
- // Invokes the actual utils wrapper method rather than the subclass mock.
- EXPECT_TRUE(scheduler_.UpdateCheckScheduler::IsOfficialBuild());
-}
-
TEST_F(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest) {
scheduler_.enabled_ = true;
- EXPECT_CALL(scheduler_, IsOfficialBuild()).Times(1).WillOnce(Return(true));
+ mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(true);
EXPECT_CALL(scheduler_, IsBootDeviceRemovable())
.Times(1)
.WillOnce(Return(true));
@@ -188,7 +182,7 @@
TEST_F(UpdateCheckSchedulerTest, RunNonOfficialBuildTest) {
scheduler_.enabled_ = true;
- EXPECT_CALL(scheduler_, IsOfficialBuild()).Times(1).WillOnce(Return(false));
+ mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(false);
scheduler_.Run();
EXPECT_FALSE(scheduler_.enabled_);
EXPECT_EQ(NULL, attempter_.update_check_scheduler());
@@ -200,7 +194,7 @@
UpdateCheckScheduler::kTimeoutRegularFuzz,
&interval_min,
&interval_max);
- EXPECT_CALL(scheduler_, IsOfficialBuild()).Times(1).WillOnce(Return(true));
+ mock_system_state_.get_fake_hardware()->SetIsOfficialBuild(true);
EXPECT_CALL(scheduler_, IsBootDeviceRemovable())
.Times(1)
.WillOnce(Return(false));