From 91a649ab62c9cf60ae9d02dd38c23fc23f98a1f3 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 21 May 2018 16:05:56 -0700 Subject: updater: Add ABORT command. This will be used for testing purpose only, replacing the previously used "fail", to intentionally abort an update. As we're separating the logic between commands parsing and execution, "abort" needs to be considered as a valid command during the parsing. Test: recovery_unit_test and recovery_component_test on marlin. Change-Id: I47c41c423e62c41cc8515fd92f3c5959be08da02 --- tests/component/updater_test.cpp | 13 ++++++++++--- tests/unit/commands_test.cpp | 24 ++++++++++++++++++++++++ updater/blockimg.cpp | 7 +++++++ updater/commands.cpp | 17 ++++++++++++++++- updater/include/private/commands.h | 12 ++++++++++++ 5 files changed, 69 insertions(+), 4 deletions(-) diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index fe4f45e15..e8abb5350 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -46,6 +46,7 @@ #include "otautil/paths.h" #include "otautil/print_sha1.h" #include "otautil/sysutil.h" +#include "private/commands.h" #include "updater/blockimg.h" #include "updater/install.h" #include "updater/updater.h" @@ -150,6 +151,9 @@ class UpdaterTest : public ::testing::Test { Paths::Get().set_last_command_file(temp_last_command_.path); Paths::Get().set_stash_directory_base(temp_stash_base_.path); + // Enable a special command "abort" to simulate interruption. + Command::abort_allowed_ = true; + last_command_file_ = temp_last_command_.path; image_file_ = image_temp_file_.path; } @@ -580,7 +584,7 @@ TEST_F(UpdaterTest, block_image_update_fail) { "2", "stash " + src_hash + " 2,0,2", "free " + src_hash, - "fail", + "abort", // clang-format on }; @@ -714,7 +718,7 @@ TEST_F(UpdaterTest, last_command_update) { "stash " + block1_hash + " 2,0,1", "move " + block1_hash + " 2,1,2 1 2,0,1", "stash " + block3_hash + " 2,2,3", - "fail", + "abort", // clang-format on }; @@ -859,6 +863,9 @@ class ResumableUpdaterTest : public testing::TestWithParam { Paths::Get().set_last_command_file(temp_last_command_.path); Paths::Get().set_stash_directory_base(temp_stash_base_.path); + // Enable a special command "abort" to simulate interruption. + Command::abort_allowed_ = true; + index_ = GetParam(); image_file_ = image_temp_file_.path; last_command_file_ = temp_last_command_.path; @@ -1030,7 +1037,7 @@ TEST_P(ResumableUpdaterTest, InterruptVerifyResume) { << g_transfer_list[kTransferListHeaderLines + index_] << ")"; std::vector transfer_list_copy{ g_transfer_list }; - transfer_list_copy[kTransferListHeaderLines + index_] = "fail"; + transfer_list_copy[kTransferListHeaderLines + index_] = "abort"; g_entries["transfer_list"] = android::base::Join(transfer_list_copy, '\n'); diff --git a/tests/unit/commands_test.cpp b/tests/unit/commands_test.cpp index cb2be9176..3daa58f33 100644 --- a/tests/unit/commands_test.cpp +++ b/tests/unit/commands_test.cpp @@ -159,6 +159,27 @@ TEST(CommandsTest, Parse_EmptyInput) { ASSERT_EQ("invalid type", err); } +TEST(CommandsTest, Parse_ABORT_Allowed) { + Command::abort_allowed_ = true; + + const std::string input{ "abort" }; + std::string err; + Command command = Command::Parse(input, 0, &err); + ASSERT_TRUE(command); + + ASSERT_EQ(TargetInfo(), command.target()); + ASSERT_EQ(SourceInfo(), command.source()); + ASSERT_EQ(StashInfo(), command.stash()); + ASSERT_EQ(PatchInfo(), command.patch()); +} + +TEST(CommandsTest, Parse_ABORT_NotAllowed) { + const std::string input{ "abort" }; + std::string err; + Command command = Command::Parse(input, 0, &err); + ASSERT_FALSE(command); +} + TEST(CommandsTest, Parse_BSDIFF) { const std::string input{ "bsdiff 0 148 " @@ -312,9 +333,12 @@ TEST(CommandsTest, Parse_ZERO) { } TEST(CommandsTest, Parse_InvalidNumberOfArgs) { + Command::abort_allowed_ = true; + // Note that the case of having excess args in BSDIFF, IMGDIFF and MOVE is covered by // ParseTargetInfoAndSourceInfo_InvalidInput. std::vector inputs{ + "abort foo", "bsdiff", "erase", "erase 4,3,5,10,12 hash1", diff --git a/updater/blockimg.cpp b/updater/blockimg.cpp index 937c5e14b..fc713859b 100644 --- a/updater/blockimg.cpp +++ b/updater/blockimg.cpp @@ -1489,6 +1489,11 @@ static int PerformCommandErase(CommandParameters& params) { return 0; } +static int PerformCommandAbort(CommandParameters&) { + LOG(INFO) << "Aborting as instructed"; + return -1; +} + using CommandFunction = std::function; using CommandMap = std::unordered_map; @@ -1888,6 +1893,7 @@ Value* BlockImageVerifyFn(const char* name, State* state, // Commands which are not allowed are set to nullptr to skip them completely. const CommandMap command_map{ // clang-format off + { Command::Type::ABORT, PerformCommandAbort }, { Command::Type::BSDIFF, PerformCommandDiff }, { Command::Type::ERASE, nullptr }, { Command::Type::FREE, PerformCommandFree }, @@ -1908,6 +1914,7 @@ Value* BlockImageUpdateFn(const char* name, State* state, const std::vector>& argv) { const CommandMap command_map{ // clang-format off + { Command::Type::ABORT, PerformCommandAbort }, { Command::Type::BSDIFF, PerformCommandDiff }, { Command::Type::ERASE, PerformCommandErase }, { Command::Type::FREE, PerformCommandFree }, diff --git a/updater/commands.cpp b/updater/commands.cpp index fb19ebc9a..e88149678 100644 --- a/updater/commands.cpp +++ b/updater/commands.cpp @@ -29,8 +29,16 @@ using namespace std::string_literals; +bool Command::abort_allowed_ = false; + Command::Type Command::ParseType(const std::string& type_str) { - if (type_str == "bsdiff") { + if (type_str == "abort") { + if (!abort_allowed_) { + LOG(ERROR) << "ABORT disallowed"; + return Type::LAST; + } + return Type::ABORT; + } else if (type_str == "bsdiff") { return Type::BSDIFF; } else if (type_str == "erase") { return Type::ERASE; @@ -237,6 +245,13 @@ Command Command::Parse(const std::string& line, size_t index, std::string* err) src_hash, &source_info, err)) { return {}; } + } else if (op == Type::ABORT) { + // No-op, other than sanity checking the input args. + if (pos != tokens.size()) { + *err = android::base::StringPrintf("invalid number of args: %zu (expected 0)", + tokens.size() - pos); + return {}; + } } else { *err = "invalid op"; return {}; diff --git a/updater/include/private/commands.h b/updater/include/private/commands.h index 784892fb5..087d7cfbf 100644 --- a/updater/include/private/commands.h +++ b/updater/include/private/commands.h @@ -213,9 +213,13 @@ class PatchInfo { // - Free the given stash data. // - Meaningful args: StashInfo // +// abort +// - Abort the current update. Allowed for testing code only. +// class Command { public: enum class Type { + ABORT, BSDIFF, ERASE, FREE, @@ -280,6 +284,11 @@ class Command { } private: + friend class ResumableUpdaterTest; + friend class UpdaterTest; + + FRIEND_TEST(CommandsTest, Parse_ABORT_Allowed); + FRIEND_TEST(CommandsTest, Parse_InvalidNumberOfArgs); FRIEND_TEST(CommandsTest, ParseTargetInfoAndSourceInfo_InvalidInput); FRIEND_TEST(CommandsTest, ParseTargetInfoAndSourceInfo_StashesOnly); FRIEND_TEST(CommandsTest, ParseTargetInfoAndSourceInfo_SourceBlocksAndStashes); @@ -293,6 +302,9 @@ class Command { const std::string& src_hash, SourceInfo* source, std::string* err); + // Allows parsing ABORT command, which should be used for testing purpose only. + static bool abort_allowed_; + // The type of the command. Type type_{ Type::LAST }; // The index of the Command object, which is specified by the caller. -- cgit v1.2.3