Split GetECVersion to allow saner unit testing.
Previously, GetECVersion() accepted a (const char *) parameter that
was used solely as a flag meaning "unit test this function with
the specified data in place of mosys output". This change splits
the function into GetECVersion() and ParseECVersion(), and only unit
tests the second part.
BUG=None
TEST=unit tests
Change-Id: Ic48d18c02bd1924f49a0d8f0034ccb1ae8b5231e
Reviewed-on: https://chromium-review.googlesource.com/174883
Commit-Queue: Richard Barnette <jrbarnette@chromium.org>
Tested-by: Richard Barnette <jrbarnette@chromium.org>
Reviewed-by: Chris Sosa <sosa@chromium.org>
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index 9a729f0..ea25c74 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -81,7 +81,7 @@
hwid_ = utils::GetHardwareClass();
if (CollectECFWVersions()) {
fw_version_ = utils::GetFirmwareVersion();
- ec_version_ = utils::GetECVersion(NULL);
+ ec_version_ = utils::GetECVersion();
}
if (current_channel_ == target_channel_) {
diff --git a/utils.cc b/utils.cc
index c5d3e05..7a304ee 100644
--- a/utils.cc
+++ b/utils.cc
@@ -109,30 +109,30 @@
return ReadValueFromCrosSystem("fwid");
}
-string GetECVersion(const char* input_line) {
- string line;
- if(input_line == NULL) {
- int exit_code = 0;
- vector<string> cmd(1, "/usr/sbin/mosys");
- cmd.push_back("-k");
- cmd.push_back("ec");
- cmd.push_back("info");
+string GetECVersion() {
+ int exit_code = 0;
+ vector<string> cmd(1, "/usr/sbin/mosys");
+ cmd.push_back("-k");
+ cmd.push_back("ec");
+ cmd.push_back("info");
- bool success = Subprocess::SynchronousExec(cmd, &exit_code, &line);
- if (!success || exit_code) {
- LOG(ERROR) << "Unable to read ec info from mosys (" << exit_code << ")";
- return "";
- }
- } else {
- line = input_line;
+ string input_line;
+ bool success = Subprocess::SynchronousExec(cmd, &exit_code, &input_line);
+ if (!success || exit_code) {
+ LOG(ERROR) << "Unable to read ec info from mosys (" << exit_code << ")";
+ return "";
}
- TrimWhitespaceASCII(line, TRIM_ALL, &line);
+ return ParseECVersion(input_line);
+}
+
+string ParseECVersion(string input_line) {
+ TrimWhitespaceASCII(input_line, TRIM_ALL, &input_line);
// At this point we want to conver the format key=value pair from mosys to
// a vector of key value pairs.
vector<pair<string, string> > kv_pairs;
- if (base::SplitStringIntoKeyValuePairs(line, '=', ' ', &kv_pairs)) {
+ if (base::SplitStringIntoKeyValuePairs(input_line, '=', ' ', &kv_pairs)) {
for (vector<pair<string, string> >::iterator it = kv_pairs.begin();
it != kv_pairs.end(); ++it) {
// Finally match against the fw_verion which may have quotes.
diff --git a/utils.h b/utils.h
index 6f6539f..30c2ccd 100644
--- a/utils.h
+++ b/utils.h
@@ -61,11 +61,14 @@
// chrome os firmware.
std::string GetFirmwareVersion();
-// Returns the ec version or an empty string if the system is not running a
-// custom chrome os ec. If input_line is not NULL, reads from this line,
-// otherwise polls the system for the input line. input_line should contain
-// fw_version=value.
-std::string GetECVersion(const char* input_line);
+// Parse the firmware version from one line of output from the
+// "mosys" command.
+std::string ParseECVersion(std::string input_line);
+
+// Reads and parses the ec version from the "mosys" command. Returns
+// the version found, or an empty string if the system is not running a
+// custom chrome os ec.
+std::string GetECVersion();
// Writes the data passed to path. The file at path will be overwritten if it
// exists. Returns true on success, false otherwise.
diff --git a/utils_unittest.cc b/utils_unittest.cc
index ae7343d..c9afbe0 100644
--- a/utils_unittest.cc
+++ b/utils_unittest.cc
@@ -38,18 +38,16 @@
}
TEST(UtilsTest, CanParseECVersion) {
- // Chroot won't have an ec version.
- EXPECT_EQ("", utils::GetECVersion(NULL));
-
// Should be able to parse and valid key value line.
- EXPECT_EQ("12345", utils::GetECVersion("fw_version=12345"));
- EXPECT_EQ("123456", utils::GetECVersion("b=1231a fw_version=123456 a=fasd2"));
- EXPECT_EQ("12345", utils::GetECVersion("fw_version=12345"));
- EXPECT_EQ("00VFA616", utils::GetECVersion(
+ EXPECT_EQ("12345", utils::ParseECVersion("fw_version=12345"));
+ EXPECT_EQ("123456", utils::ParseECVersion(
+ "b=1231a fw_version=123456 a=fasd2"));
+ EXPECT_EQ("12345", utils::ParseECVersion("fw_version=12345"));
+ EXPECT_EQ("00VFA616", utils::ParseECVersion(
"vendor=\"sam\" fw_version=\"00VFA616\""));
// For invalid entries, should return the empty string.
- EXPECT_EQ("", utils::GetECVersion("b=1231a fw_version a=fasd2"));
+ EXPECT_EQ("", utils::ParseECVersion("b=1231a fw_version a=fasd2"));
}
TEST(UtilsTest, NormalizePathTest) {