Move IsBootDeviceRemovable() to the HardwareInterface.
The UpdateCheckScheduler class had a IsBootDeviceRemovable method to
mock out the value during testing of that class. Instead, this patch
uses the HardwareInterface to mock out that value.
BUG=chromium:358269
TEST=Unittests updated.
Change-Id: Ib20f70fa0468aaa4bc8bb1b674084bd9a61e5085
Reviewed-on: https://chromium-review.googlesource.com/197598
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
diff --git a/fake_hardware.h b/fake_hardware.h
index 06f1c30..e5d2e75 100644
--- a/fake_hardware.h
+++ b/fake_hardware.h
@@ -19,6 +19,7 @@
FakeHardware()
: kernel_device_("/dev/sdz4"),
boot_device_("/dev/sdz5"),
+ is_boot_device_removable_(false),
kernel_devices_({"/dev/sdz2", "/dev/sdz4"}),
is_official_build_(true),
is_normal_boot_mode_(true),
@@ -34,6 +35,10 @@
virtual std::string BootDevice() const override { return boot_device_; }
+ virtual bool IsBootDeviceRemovable() const override {
+ return is_boot_device_removable_;
+ }
+
virtual std::vector<std::string> GetKernelDevices() const override {
return kernel_devices_;
}
@@ -73,10 +78,14 @@
virtual std::string GetECVersion() const override { return ec_version_; }
// Setters
- void SetBootDevice(const std::string boot_device) {
+ void SetBootDevice(const std::string& boot_device) {
boot_device_ = boot_device;
}
+ void SetIsBootDeviceRemovable(bool is_boot_device_removable) {
+ is_boot_device_removable_ = is_boot_device_removable;
+ }
+
void SetIsOfficialBuild(bool is_official_build) {
is_official_build_ = is_official_build;
}
@@ -111,6 +120,7 @@
private:
std::string kernel_device_;
std::string boot_device_;
+ bool is_boot_device_removable_;
std::vector<std::string> kernel_devices_;
std::map<std::string, bool> is_bootable_;
bool is_official_build_;
diff --git a/hardware.cc b/hardware.cc
index 5d5492f..441e066 100644
--- a/hardware.cc
+++ b/hardware.cc
@@ -60,6 +60,10 @@
return boot_path;
}
+bool Hardware::IsBootDeviceRemovable() const {
+ return utils::IsRemovableDevice(utils::GetDiskName(BootDevice()));
+}
+
bool Hardware::IsKernelBootable(const std::string& kernel_device,
bool* bootable) const {
CgptAddParams params;
diff --git a/hardware.h b/hardware.h
index fdcdf13..99c0803 100644
--- a/hardware.h
+++ b/hardware.h
@@ -20,6 +20,7 @@
// HardwareInterface methods.
virtual std::string BootKernelDevice() const override;
virtual std::string BootDevice() const override;
+ virtual bool IsBootDeviceRemovable() const override;
virtual std::vector<std::string> GetKernelDevices() const override;
virtual bool IsKernelBootable(const std::string& kernel_device,
bool* bootable) const override;
diff --git a/hardware_interface.h b/hardware_interface.h
index bc7fee3..83db8a2 100644
--- a/hardware_interface.h
+++ b/hardware_interface.h
@@ -26,6 +26,9 @@
// Returns the currently booted rootfs partition. "/dev/sda3", for example.
virtual std::string BootDevice() const = 0;
+ // Return whether the BootDevice() is a removable device.
+ virtual bool IsBootDeviceRemovable() const = 0;
+
// Returns a list of all kernel partitions available (whether bootable or not)
virtual std::vector<std::string> GetKernelDevices() const = 0;
diff --git a/mock_hardware.h b/mock_hardware.h
index a996896..574f1f7 100644
--- a/mock_hardware.h
+++ b/mock_hardware.h
@@ -22,6 +22,9 @@
ON_CALL(*this, BootDevice())
.WillByDefault(testing::Invoke(&fake_,
&FakeHardware::BootDevice));
+ ON_CALL(*this, IsBootDeviceRemovable())
+ .WillByDefault(testing::Invoke(&fake_,
+ &FakeHardware::IsBootDeviceRemovable));
ON_CALL(*this, GetKernelDevices())
.WillByDefault(testing::Invoke(&fake_,
&FakeHardware::GetKernelDevices));
@@ -56,6 +59,7 @@
// Hardware overrides.
MOCK_CONST_METHOD0(BootKernelDevice, std::string());
MOCK_CONST_METHOD0(BootDevice, std::string());
+ MOCK_CONST_METHOD0(IsBootDeviceRemovable, bool());
MOCK_CONST_METHOD0(GetKernelDevices, std::vector<std::string>());
MOCK_CONST_METHOD2(IsKernelBootable,
bool(const std::string& kernel_device, bool* bootable));
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 45a28ae..0f3d960 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -41,7 +41,7 @@
LOG(WARNING) << "Non-official build: periodic update checks disabled.";
return;
}
- if (IsBootDeviceRemovable()) {
+ if (system_state_->hardware()->IsBootDeviceRemovable()) {
LOG(WARNING) << "Removable device boot: periodic update checks disabled.";
return;
}
@@ -58,11 +58,6 @@
ScheduleCheck(kTimeoutInitialInterval, kTimeoutRegularFuzz);
}
-bool UpdateCheckScheduler::IsBootDeviceRemovable() {
- return utils::IsRemovableDevice(utils::GetDiskName(
- system_state_->hardware()->BootDevice()));
-}
-
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 6e95add..a2dc41a 100644
--- a/update_check_scheduler.h
+++ b/update_check_scheduler.h
@@ -65,7 +65,6 @@
FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzPriorityTest);
FRIEND_TEST(UpdateCheckSchedulerTest, ComputeNextIntervalAndFuzzTest);
FRIEND_TEST(UpdateCheckSchedulerTest, GTimeoutAddSecondsTest);
- FRIEND_TEST(UpdateCheckSchedulerTest, IsBootDeviceRemovableTest);
FRIEND_TEST(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest);
FRIEND_TEST(UpdateCheckSchedulerTest, RunNonOfficialBuildTest);
FRIEND_TEST(UpdateCheckSchedulerTest, RunTest);
@@ -84,9 +83,6 @@
// Wraps GLib's g_timeout_add_seconds so that it can be mocked in tests.
virtual guint GTimeoutAddSeconds(guint interval, GSourceFunc function);
- // Wrappers for utils functions so that they can be mocked in tests.
- virtual bool IsBootDeviceRemovable();
-
// 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
// already scheduled.
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index 608b6a6..74daaf3 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -40,7 +40,6 @@
fake_system_state_(fake_system_state) {}
MOCK_METHOD2(GTimeoutAddSeconds, guint(guint seconds, GSourceFunc function));
- MOCK_METHOD0(IsBootDeviceRemovable, bool());
FakeSystemState* fake_system_state_;
};
@@ -164,17 +163,10 @@
g_main_loop_unref(loop_);
}
-TEST_F(UpdateCheckSchedulerTest, IsBootDeviceRemovableTest) {
- // Invokes the actual utils wrapper method rather than the subclass mock.
- EXPECT_FALSE(scheduler_.UpdateCheckScheduler::IsBootDeviceRemovable());
-}
-
TEST_F(UpdateCheckSchedulerTest, RunBootDeviceRemovableTest) {
scheduler_.enabled_ = true;
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- EXPECT_CALL(scheduler_, IsBootDeviceRemovable())
- .Times(1)
- .WillOnce(Return(true));
+ fake_system_state_.fake_hardware()->SetIsBootDeviceRemovable(true);
scheduler_.Run();
EXPECT_FALSE(scheduler_.enabled_);
EXPECT_EQ(NULL, attempter_.update_check_scheduler());
@@ -195,9 +187,7 @@
&interval_min,
&interval_max);
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- EXPECT_CALL(scheduler_, IsBootDeviceRemovable())
- .Times(1)
- .WillOnce(Return(false));
+ fake_system_state_.fake_hardware()->SetIsBootDeviceRemovable(false);
EXPECT_CALL(scheduler_,
GTimeoutAddSeconds(AllOf(Ge(interval_min), Le(interval_max)),
scheduler_.StaticCheck)).Times(1);