AU/unittest: allow use of a parametric update completed marker
This should allow unit tests to be run in parallel. We introduce
a private constructor available only to specific test classes of
UpdateAttempter, which allows us to set a different marker.
Note that, in all unit tests that do not specifically make use of this
marker, we actually want UpdateAttempter to ignore it entirely. I'm not
entirely positive whether not ignoring would lead to interferences
between concurrently run tests, but I think it's better to err on the
safe side. (The marker is never ignored when used with any code other
than UpdateAttempter unit testing code.)
BUG=chromium:236465
TEST=Unit tests pass incl ignore semantics when not needed
Change-Id: I30fbed2ae2c21368d79127ed44811007e2a66e77
Reviewed-on: https://gerrit.chromium.org/gerrit/63840
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_attempter.cc b/update_attempter.cc
index 5755fb4..9ed321d 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -53,11 +53,11 @@
const char* const UpdateAttempter::kTestUpdateUrl(
"http://garnold.mtv.corp.google.com:8080/update");
-const char* kUpdateCompletedMarker =
- "/var/run/update_engine_autoupdate_completed";
-
namespace {
const int kMaxConsecutiveObeyProxyRequests = 20;
+
+const char* kUpdateCompletedMarker =
+ "/var/run/update_engine_autoupdate_completed";
} // namespace {}
const char* UpdateStatusToString(UpdateStatus status) {
@@ -109,34 +109,52 @@
UpdateAttempter::UpdateAttempter(SystemState* system_state,
DbusGlibInterface* dbus_iface)
- : processor_(new ActionProcessor()),
- system_state_(system_state),
- dbus_service_(NULL),
- update_check_scheduler_(NULL),
- fake_update_success_(false),
- http_response_code_(0),
- shares_(utils::kCpuSharesNormal),
- manage_shares_source_(NULL),
- download_active_(false),
- status_(UPDATE_STATUS_IDLE),
- download_progress_(0.0),
- last_checked_time_(0),
- new_version_("0.0.0.0"),
- new_payload_size_(0),
- proxy_manual_checks_(0),
- obeying_proxies_(true),
- chrome_proxy_resolver_(dbus_iface),
- updated_boot_flags_(false),
- update_boot_flags_running_(false),
- start_action_processor_(false),
- policy_provider_(NULL),
- is_using_test_url_(false),
- is_test_mode_(false),
- is_test_update_attempted_(false) {
+ : chrome_proxy_resolver_(dbus_iface) {
+ Init(system_state, kUpdateCompletedMarker);
+}
+
+UpdateAttempter::UpdateAttempter(SystemState* system_state,
+ DbusGlibInterface* dbus_iface,
+ const std::string& update_completed_marker)
+ : chrome_proxy_resolver_(dbus_iface) {
+ Init(system_state, update_completed_marker);
+}
+
+
+void UpdateAttempter::Init(SystemState* system_state,
+ const std::string& update_completed_marker) {
+ // Initialite data members.
+ processor_.reset(new ActionProcessor());
+ system_state_ = system_state;
+ dbus_service_ = NULL;
+ update_check_scheduler_ = NULL;
+ fake_update_success_ = false;
+ http_response_code_ = 0;
+ shares_ = utils::kCpuSharesNormal;
+ manage_shares_source_ = NULL;
+ download_active_ = false;
+ download_progress_ = 0.0;
+ last_checked_time_ = 0;
+ new_version_ = "0.0.0.0";
+ new_payload_size_ = 0;
+ proxy_manual_checks_ = 0;
+ obeying_proxies_ = true;
+ updated_boot_flags_ = false;
+ update_boot_flags_running_ = false;
+ start_action_processor_ = false;
+ is_using_test_url_ = false;
+ is_test_mode_ = false;
+ is_test_update_attempted_ = false;
+ update_completed_marker_ = update_completed_marker;
+
prefs_ = system_state->prefs();
omaha_request_params_ = system_state->request_params();
- if (utils::FileExists(kUpdateCompletedMarker))
+
+ if (!update_completed_marker_.empty() &&
+ utils::FileExists(update_completed_marker_.c_str()))
status_ = UPDATE_STATUS_UPDATED_NEED_REBOOT;
+ else
+ status_ = UPDATE_STATUS_IDLE;
}
UpdateAttempter::~UpdateAttempter() {
@@ -681,7 +699,8 @@
}
if (code == kErrorCodeSuccess) {
- utils::WriteFile(kUpdateCompletedMarker, "", 0);
+ if (!update_completed_marker_.empty())
+ utils::WriteFile(update_completed_marker_.c_str(), "", 0);
prefs_->SetInt64(kPrefsDeltaUpdateFailures, 0);
prefs_->SetString(kPrefsPreviousVersion,
omaha_request_params_->app_version());
@@ -853,9 +872,11 @@
// Remove the reboot marker so that if the machine is rebooted
// after resetting to idle state, it doesn't go back to
// UPDATE_STATUS_UPDATED_NEED_REBOOT state.
- const FilePath kUpdateCompletedMarkerPath(kUpdateCompletedMarker);
- if (!file_util::Delete(kUpdateCompletedMarkerPath, false))
- ret_value = false;
+ if (!update_completed_marker_.empty()) {
+ const FilePath update_completed_marker_path(update_completed_marker_);
+ if (!file_util::Delete(update_completed_marker_path, false))
+ ret_value = false;
+ }
// Notify the PayloadState that the successful payload was canceled.
system_state_->payload_state()->ResetUpdateStatus();
diff --git a/update_attempter.h b/update_attempter.h
index 423fb88..dea9bc9 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -35,8 +35,6 @@
class UpdateCheckScheduler;
-extern const char* kUpdateCompletedMarker;
-
enum UpdateStatus {
UPDATE_STATUS_IDLE = 0,
UPDATE_STATUS_CHECKING_FOR_UPDATE,
@@ -185,6 +183,12 @@
// Update server URL for automated lab test.
static const char* const kTestUpdateUrl;
+ // Special ctor + friend declarations for testing purposes.
+ UpdateAttempter(SystemState* system_state,
+ DbusGlibInterface* dbus_iface,
+ const std::string& update_completed_marker);
+
+ friend class UpdateAttempterUnderTest;
friend class UpdateAttempterTest;
FRIEND_TEST(UpdateAttempterTest, ActionCompletedDownloadTest);
FRIEND_TEST(UpdateAttempterTest, ActionCompletedErrorTest);
@@ -199,6 +203,10 @@
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionTest);
FRIEND_TEST(UpdateAttempterTest, UpdateTest);
+ // Ctor helper method.
+ void Init(SystemState* system_state,
+ const std::string& update_completed_marker);
+
// Sets the status to the given status and notifies a status update over dbus.
// Also accepts a supplement notice, which is delegated to the scheduler and
// used for making better informed scheduling decisions (e.g. retry timeout).
@@ -393,6 +401,10 @@
// The current scatter factor as found in the policy setting.
base::TimeDelta scatter_factor_;
+ // Update completed marker file. An empty string means this marker is being
+ // ignored (nor is it being written), which is useful for testing situations.
+ std::string update_completed_marker_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 91c6dfe..1a52095 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -40,9 +40,17 @@
// methods.
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
- explicit UpdateAttempterUnderTest(MockSystemState* mock_system_state,
- MockDbusGlib* dbus)
- : UpdateAttempter(mock_system_state, dbus) {}
+ // 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,
+ MockDbusGlib* dbus)
+ : UpdateAttempter(mock_system_state, dbus, "") {}
+
+ UpdateAttempterUnderTest(MockSystemState* mock_system_state,
+ MockDbusGlib* dbus,
+ const string& update_completed_marker)
+ : UpdateAttempter(mock_system_state, dbus, update_completed_marker) {}
};
class UpdateAttempterTest : public ::testing::Test {
@@ -170,12 +178,16 @@
}
TEST_F(UpdateAttempterTest, RunAsRootConstructWithUpdatedMarkerTest) {
- extern const char* kUpdateCompletedMarker;
- const FilePath kMarker(kUpdateCompletedMarker);
- EXPECT_EQ(0, file_util::WriteFile(kMarker, "", 0));
- UpdateAttempterUnderTest attempter(&mock_system_state_, &dbus_);
+ string test_update_completed_marker;
+ CHECK(utils::MakeTempFile(
+ "/tmp/update_attempter_unittest-update_completed_marker-XXXXXX",
+ &test_update_completed_marker, NULL));
+ ScopedPathUnlinker completed_marker_unlinker(test_update_completed_marker);
+ const FilePath marker(test_update_completed_marker);
+ EXPECT_EQ(0, file_util::WriteFile(marker, "", 0));
+ UpdateAttempterUnderTest attempter(&mock_system_state_, &dbus_,
+ test_update_completed_marker);
EXPECT_EQ(UPDATE_STATUS_UPDATED_NEED_REBOOT, attempter.status());
- EXPECT_TRUE(file_util::Delete(kMarker, false));
}
TEST_F(UpdateAttempterTest, GetErrorCodeForActionTest) {