AU/unittest: fix to OmahaResponseHandlerAction unit tests
The said class uses a fixed named file for communicating data (in this
case, a deadline marker) to Chrome, which causes unit tests to fail when
run in parallel. This change makes this file parametric for testing
purposes, allowing the tests to be run in parallel.
Note that we only override the file name for tests that actually examine
the file. For other invocations there may still be race conditions when
writing to the marker file, but they should not affect the outcome of
the tests.
BUG=chromium:236465
TEST=Tests successful
Change-Id: Ieb2a3ef1b4d9c5d5d2e06cf8e281eb832b2a9ff8
Reviewed-on: https://gerrit.chromium.org/gerrit/62825
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 21832c5..923c62d 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -26,7 +26,15 @@
SystemState* system_state)
: system_state_(system_state),
got_no_update_response_(false),
- key_path_(DeltaPerformer::kUpdatePayloadPublicKeyPath) {}
+ key_path_(DeltaPerformer::kUpdatePayloadPublicKeyPath),
+ deadline_file_(kDeadlineFile) {}
+
+OmahaResponseHandlerAction::OmahaResponseHandlerAction(
+ SystemState* system_state, const string& deadline_file)
+ : system_state_(system_state),
+ got_no_update_response_(false),
+ key_path_(DeltaPerformer::kUpdatePayloadPublicKeyPath),
+ deadline_file_(deadline_file) {}
void OmahaResponseHandlerAction::PerformAction() {
CHECK(HasInputObject());
@@ -104,10 +112,10 @@
// file. Ideally, we would include this information in D-Bus's GetStatus
// method and UpdateStatus signal. A potential issue is that update_engine may
// be unresponsive during an update download.
- utils::WriteFile(kDeadlineFile,
+ utils::WriteFile(deadline_file_.c_str(),
response.deadline.data(),
response.deadline.size());
- chmod(kDeadlineFile, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+ chmod(deadline_file_.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
completer.set_code(kErrorCodeSuccess);
}
diff --git a/omaha_response_handler_action.h b/omaha_response_handler_action.h
index 82e99bf..db72c91 100644
--- a/omaha_response_handler_action.h
+++ b/omaha_response_handler_action.h
@@ -32,7 +32,8 @@
public:
static const char kDeadlineFile[];
- OmahaResponseHandlerAction(SystemState* system_state);
+ explicit OmahaResponseHandlerAction(SystemState* system_state);
+
typedef ActionTraits<OmahaResponseHandlerAction>::InputObjectType
InputObjectType;
typedef ActionTraits<OmahaResponseHandlerAction>::OutputObjectType
@@ -57,8 +58,6 @@
void set_key_path(const std::string& path) { key_path_ = path; }
private:
- FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventResumedTest);
-
// Returns true if payload hash checks are mandatory based on the state
// of the system and the contents of the Omaha response. False otherwise.
bool AreHashChecksMandatory(const OmahaResponse& response);
@@ -78,6 +77,17 @@
// Public key path to use for payload verification.
std::string key_path_;
+ // File used for communication deadline to Chrome.
+ const std::string deadline_file_;
+
+ // Special ctor + friend declarations for testing purposes.
+ OmahaResponseHandlerAction(SystemState* system_state,
+ const std::string& deadline_file);
+
+ friend class OmahaResponseHandlerActionTest;
+
+ FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventResumedTest);
+
DISALLOW_COPY_AND_ASSIGN(OmahaResponseHandlerAction);
};
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index f3b4b99..06d1570 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -25,9 +25,11 @@
bool DoTestCommon(MockSystemState* mock_system_state,
const OmahaResponse& in,
const string& boot_dev,
+ const string& deadline_file,
InstallPlan* out);
bool DoTest(const OmahaResponse& in,
const string& boot_dev,
+ const string& deadline_file,
InstallPlan* out);
};
@@ -66,6 +68,7 @@
MockSystemState* mock_system_state,
const OmahaResponse& in,
const string& boot_dev,
+ const string& test_deadline_file,
InstallPlan* out) {
ActionProcessor processor;
OmahaResponseHandlerActionProcessorDelegate delegate;
@@ -85,7 +88,10 @@
EXPECT_CALL(*(mock_system_state->mock_payload_state()), GetRollbackVersion())
.WillRepeatedly(Return(kBadVersion));
- OmahaResponseHandlerAction response_handler_action(mock_system_state);
+ OmahaResponseHandlerAction response_handler_action(
+ mock_system_state,
+ (test_deadline_file.empty() ?
+ OmahaResponseHandlerAction::kDeadlineFile : test_deadline_file));
response_handler_action.set_boot_device(boot_dev);
BondActions(&feeder_action, &response_handler_action);
ObjectCollectorAction<InstallPlan> collector_action;
@@ -104,14 +110,18 @@
bool OmahaResponseHandlerActionTest::DoTest(const OmahaResponse& in,
const string& boot_dev,
+ const string& deadline_file,
InstallPlan* out) {
MockSystemState mock_system_state;
- return DoTestCommon(&mock_system_state, in, boot_dev, out);
+ return DoTestCommon(&mock_system_state, in, boot_dev, deadline_file, out);
}
TEST_F(OmahaResponseHandlerActionTest, SimpleTest) {
- ScopedPathUnlinker deadline_unlinker(
- OmahaResponseHandlerAction::kDeadlineFile);
+ string test_deadline_file;
+ CHECK(utils::MakeTempFile(
+ "/tmp/omaha_response_handler_action_unittest-XXXXXX",
+ &test_deadline_file, NULL));
+ ScopedPathUnlinker deadline_unlinker(test_deadline_file);
{
OmahaResponse in;
in.update_exists = true;
@@ -123,18 +133,15 @@
in.prompt = false;
in.deadline = "20101020";
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", test_deadline_file, &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_EQ("/dev/sda5", install_plan.install_path);
string deadline;
- EXPECT_TRUE(utils::ReadFile(
- OmahaResponseHandlerAction::kDeadlineFile,
- &deadline));
+ EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline));
EXPECT_EQ("20101020", deadline);
struct stat deadline_stat;
- EXPECT_EQ(0, stat(OmahaResponseHandlerAction::kDeadlineFile,
- &deadline_stat));
+ EXPECT_EQ(0, stat(test_deadline_file.c_str(), &deadline_stat));
EXPECT_EQ(S_IFREG | S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH,
deadline_stat.st_mode);
}
@@ -148,14 +155,13 @@
in.size = 12;
in.prompt = true;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", test_deadline_file, &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_EQ("/dev/sda3", install_plan.install_path);
string deadline;
- EXPECT_TRUE(utils::ReadFile(
- OmahaResponseHandlerAction::kDeadlineFile,
- &deadline) && deadline.empty());
+ EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline) &&
+ deadline.empty());
}
{
OmahaResponse in;
@@ -168,14 +174,12 @@
in.prompt = true;
in.deadline = "some-deadline";
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda3", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda3", test_deadline_file, &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_EQ("/dev/sda5", install_plan.install_path);
string deadline;
- EXPECT_TRUE(utils::ReadFile(
- OmahaResponseHandlerAction::kDeadlineFile,
- &deadline));
+ EXPECT_TRUE(utils::ReadFile(test_deadline_file, &deadline));
EXPECT_EQ("some-deadline", deadline);
}
}
@@ -184,7 +188,7 @@
OmahaResponse in;
in.update_exists = false;
InstallPlan install_plan;
- EXPECT_FALSE(DoTest(in, "/dev/sda1", &install_plan));
+ EXPECT_FALSE(DoTest(in, "/dev/sda1", "", &install_plan));
EXPECT_EQ("", install_plan.download_url);
EXPECT_EQ("", install_plan.payload_hash);
EXPECT_EQ("", install_plan.install_path);
@@ -204,14 +208,14 @@
in.prompt = true;
// Version is blacklisted for first call so no update.
- EXPECT_FALSE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_FALSE(DoTest(in, "/dev/sda5", "", &install_plan));
EXPECT_EQ("", install_plan.download_url);
EXPECT_EQ("", install_plan.payload_hash);
EXPECT_EQ("", install_plan.install_path);
// Version isn't blacklisted.
in.version = version_ok;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
}
@@ -224,7 +228,7 @@
in.hash = "HASHj+";
in.size = 12;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_TRUE(install_plan.hash_checks_mandatory);
@@ -239,7 +243,7 @@
in.hash = "HASHj+";
in.size = 12;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_FALSE(install_plan.hash_checks_mandatory);
@@ -255,7 +259,7 @@
in.hash = "HASHj+";
in.size = 12;
InstallPlan install_plan;
- EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", "", &install_plan));
EXPECT_EQ(in.payload_urls[0], install_plan.download_url);
EXPECT_EQ(in.hash, install_plan.payload_hash);
EXPECT_TRUE(install_plan.hash_checks_mandatory);
@@ -294,7 +298,8 @@
mock_system_state.set_request_params(¶ms);
InstallPlan install_plan;
- EXPECT_TRUE(DoTestCommon(&mock_system_state, in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTestCommon(&mock_system_state, in, "/dev/sda5", "",
+ &install_plan));
EXPECT_TRUE(install_plan.powerwash_required);
}
@@ -331,7 +336,8 @@
mock_system_state.set_request_params(¶ms);
InstallPlan install_plan;
- EXPECT_TRUE(DoTestCommon(&mock_system_state, in, "/dev/sda5", &install_plan));
+ EXPECT_TRUE(DoTestCommon(&mock_system_state, in, "/dev/sda5", "",
+ &install_plan));
EXPECT_FALSE(install_plan.powerwash_required);
}