Revise the SystemState hierarchy.
* Removed all #includes from SystemState; added includes in .cc files
that use the various objects (MetricsLibrary, DevicePolicy, etc).
* MockSystemState:
- Regulated the set of getters/setters: foo() returns the current Foo
object interface; this object can be overridden by set_foo();
mock_foo() or fake_foo() returns the default (internal) mock/fake
equivalent, and fails if it is different from foo() (safety).
- Make member declaration order consistent with that of API.
- Removed MOCK_METHOD declarations for two methods and replaced them
with fake getter/setter. This means that MockSystemState is now
reduced to a fake, and can be renamed (separate CL). This also means
that a few tests have a slightly different semantics now.
* All virtual overrides are qualified as such. However, removed the
'const' method qualified from all getters: it made little sense,
especially when considering that getters are handing addresses of
internal mock members.
* Made the UpdateAttempter a contained member of both
{Real,Mock}SystemState, resolving initialization dependencies. In
general, the invariant is that all members of the SystemState that
rely on it being fully populated by the time of their initialization,
need to export a separate Init() method, that will be called (by the
SystemState implementation constructor or Init() method) only after
all members are set.
* Made the mock GPIO handler and connection manager contained members of
MockSystemState; the destructor could safely be moved.
* Cleanup in UpdateAttempter (part of resolving dependencies):
- Ordinary member initialization done via default initializers
(constants) or initializer list in the constructor (parameters).
- Init() method only does work that cannot be done during
construction, with appropriate comment documenting the need for it.
- Better reuse via constructor delegation.
BUG=chromium:358278
TEST=Unit tests.
Change-Id: I96ff6fc7e7400b0a9feb6cc8d4ffe97a51000f91
Reviewed-on: https://chromium-review.googlesource.com/193587
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 2148c06..82e96dd 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -47,14 +47,14 @@
// We always feed an explicit update completed marker name; however, unless
// explicitly specified, we feed an empty string, which causes the
// UpdateAttempter class to ignore / not write the marker file.
- UpdateAttempterUnderTest(MockSystemState* mock_system_state,
- MockDBusWrapper* dbus)
- : UpdateAttempter(mock_system_state, dbus, "") {}
+ UpdateAttempterUnderTest(SystemState* system_state,
+ DBusWrapperInterface* dbus_iface)
+ : UpdateAttempterUnderTest(system_state, dbus_iface, "") {}
- UpdateAttempterUnderTest(MockSystemState* mock_system_state,
- MockDBusWrapper* dbus,
- const string& update_completed_marker)
- : UpdateAttempter(mock_system_state, dbus, update_completed_marker) {}
+ UpdateAttempterUnderTest(SystemState* system_state,
+ DBusWrapperInterface* dbus_iface,
+ const std::string& update_completed_marker)
+ : UpdateAttempter(system_state, dbus_iface, update_completed_marker) {}
};
class UpdateAttempterTest : public ::testing::Test {
@@ -63,7 +63,12 @@
: attempter_(&mock_system_state_, &dbus_),
mock_connection_manager(&mock_system_state_),
loop_(NULL) {
+ // Override system state members.
mock_system_state_.set_connection_manager(&mock_connection_manager);
+ mock_system_state_.set_update_attempter(&attempter_);
+
+ // Finish initializing the attempter.
+ attempter_.Init();
}
virtual void SetUp() {
@@ -145,7 +150,7 @@
void P2PEnabledHousekeepingFailsStart();
static gboolean StaticP2PEnabledHousekeepingFails(gpointer data);
- NiceMock<MockSystemState> mock_system_state_;
+ MockSystemState mock_system_state_;
NiceMock<MockDBusWrapper> dbus_;
UpdateAttempterUnderTest attempter_;
NiceMock<ActionProcessorMock>* processor_;
@@ -464,14 +469,13 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
if (!valid_slot) {
// References bootable kernels in fake_hardware.h
string rollback_kernel = "/dev/sdz2";
LOG(INFO) << "Test Mark Unbootable: " << rollback_kernel;
- mock_system_state_.get_fake_hardware()->MarkKernelUnbootable(
+ mock_system_state_.fake_hardware()->MarkKernelUnbootable(
rollback_kernel);
}
@@ -629,8 +633,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetReleaseChannelDelegated(_)).WillRepeatedly(
DoAll(SetArgumentPointee<0>(bool(false)),
@@ -665,8 +668,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetUpdateDisabled(_))
.WillRepeatedly(DoAll(
@@ -858,8 +860,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetTargetVersionPrefix(_))
.WillRepeatedly(DoAll(
@@ -891,8 +892,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
.WillRepeatedly(DoAll(
@@ -920,7 +920,7 @@
Prefs prefs;
attempter_.prefs_ = &prefs;
- mock_system_state_.get_fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
+ mock_system_state_.fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
string prefs_dir;
EXPECT_TRUE(utils::MakeTempDirectory("ue_ut_prefs.XXXXXX",
@@ -937,8 +937,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
.WillRepeatedly(DoAll(
@@ -984,7 +983,7 @@
Prefs prefs;
attempter_.prefs_ = &prefs;
- mock_system_state_.get_fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
+ mock_system_state_.fake_hardware()->SetIsOOBEComplete(Time::UnixEpoch());
string prefs_dir;
EXPECT_TRUE(utils::MakeTempDirectory("ue_ut_prefs.XXXXXX",
@@ -1004,8 +1003,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
- Return(device_policy));
+ mock_system_state_.set_device_policy(device_policy);
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
.WillRepeatedly(DoAll(