From ffed57a7a3e5e82969b5998f759d09fbb6edd4e2 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Mon, 23 Apr 2018 17:51:45 -0700 Subject: Dump debug information for apply_patch unit tests The apply patch test should have a deterministic way to append patch data. Add debug logs to dump the length and SHA1 of each step to further track down the flakiness. Also redirect the debug logging to stdout in case the logcat becomes too chatty. Bug: 67849209 Test: Run recovery_component_test Change-Id: I42bafef2d9dee599719ae57840b3d8c00d243ebd --- tests/component/applypatch_test.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/component/applypatch_test.cpp b/tests/component/applypatch_test.cpp index f19f28c60..292d76e43 100644 --- a/tests/component/applypatch_test.cpp +++ b/tests/component/applypatch_test.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -46,7 +47,7 @@ using namespace std::string_literals; static void sha1sum(const std::string& fname, std::string* sha1, size_t* fsize = nullptr) { - ASSERT_NE(nullptr, sha1); + ASSERT_TRUE(sha1 != nullptr); std::string data; ASSERT_TRUE(android::base::ReadFileToString(fname, &data)); @@ -68,6 +69,14 @@ static void mangle_file(const std::string& fname) { ASSERT_TRUE(android::base::WriteStringToFile(content, fname)); } +static void test_logger(android::base::LogId /* id */, android::base::LogSeverity severity, + const char* /* tag */, const char* /* file */, unsigned int /* line */, + const char* message) { + if (severity >= android::base::GetMinimumLogSeverity()) { + fprintf(stdout, "%s\n", message); + } +} + class ApplyPatchTest : public ::testing::Test { public: virtual void SetUp() override { @@ -109,6 +118,8 @@ class ApplyPatchModesTest : public ::testing::Test { protected: void SetUp() override { CacheLocation::location().set_cache_temp_source(cache_source.path); + android::base::InitLogging(nullptr, &test_logger); + android::base::SetMinimumLogSeverity(android::base::LogSeverity::DEBUG); } TemporaryFile cache_source; -- cgit v1.2.3 From 9e1ccd47b49199b728afb06b28aa7987b5d65960 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 25 Apr 2018 17:19:20 -0700 Subject: Dump the uncompressed data's SHA1 to debug flaky tests Dump the SHA1 of the uncompressed data in applypatch to confirm if we are at least doing the bspatch part correctly. (I expect so since the actual length of the uncompressed data matches the expected length). Also try to decompress the deflate chunk inside the recovery image for these two flacky tests. In theory, there shouldn't be randomness in zlib; so we would know if we process the data wrongly if the deflate fails to decompress. Bug: 67849209 Test: recovery_component_test Change-Id: Id947522153b1eeb0d10d161298a96fb045f92018 --- tests/component/applypatch_test.cpp | 54 +++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/component/applypatch_test.cpp b/tests/component/applypatch_test.cpp index 292d76e43..04055b9fe 100644 --- a/tests/component/applypatch_test.cpp +++ b/tests/component/applypatch_test.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include "applypatch/applypatch.h" #include "applypatch/applypatch_modes.h" @@ -46,6 +47,47 @@ using namespace std::string_literals; +// TODO(b/67849209) Remove after debug the flakiness. +static void DecompressAndDumpRecoveryImage(const std::string& image_path) { + // Expected recovery_image structure + // chunk normal: 45066 bytes + // chunk deflate: 479442 bytes + // chunk normal: 5199 bytes + std::string recovery_content; + ASSERT_TRUE(android::base::ReadFileToString(image_path, &recovery_content)); + ASSERT_GT(recovery_content.size(), 45066 + 5199); + + z_stream strm = {}; + strm.avail_in = recovery_content.size() - 45066 - 5199; + strm.next_in = + const_cast(reinterpret_cast(recovery_content.data())) + 45066; + + ASSERT_EQ(Z_OK, inflateInit2(&strm, -15)); + + constexpr unsigned int BUFFER_SIZE = 32768; + std::vector uncompressed_data(BUFFER_SIZE); + size_t uncompressed_length = 0; + SHA_CTX ctx; + SHA1_Init(&ctx); + int ret; + do { + strm.avail_out = BUFFER_SIZE; + strm.next_out = uncompressed_data.data(); + + ret = inflate(&strm, Z_NO_FLUSH); + ASSERT_GE(ret, 0); + + SHA1_Update(&ctx, uncompressed_data.data(), BUFFER_SIZE - strm.avail_out); + uncompressed_length += BUFFER_SIZE - strm.avail_out; + } while (ret != Z_STREAM_END); + inflateEnd(&strm); + + uint8_t digest[SHA_DIGEST_LENGTH]; + SHA1_Final(digest, &ctx); + GTEST_LOG_(INFO) << "uncompressed length " << uncompressed_length + << " sha1: " << short_sha1(digest); +} + static void sha1sum(const std::string& fname, std::string* sha1, size_t* fsize = nullptr) { ASSERT_TRUE(sha1 != nullptr); @@ -317,7 +359,11 @@ TEST_F(ApplyPatchModesTest, PatchModeEmmcTargetWithoutBonusFile) { recovery_img_sha1.c_str(), recovery_img_size_arg.c_str(), patch_arg.c_str() }; - ASSERT_EQ(0, applypatch_modes(args.size(), args.data())); + + if (applypatch_modes(args.size(), args.data()) != 0) { + DecompressAndDumpRecoveryImage(tgt_file.path); + FAIL(); + } } TEST_F(ApplyPatchModesTest, PatchModeEmmcTargetWithMultiplePatches) { @@ -360,7 +406,11 @@ TEST_F(ApplyPatchModesTest, PatchModeEmmcTargetWithMultiplePatches) { for (const auto& arg : args) { printf(" %s\n", arg); } - ASSERT_EQ(0, applypatch_modes(args.size(), args.data())); + + if (applypatch_modes(args.size(), args.data()) != 0) { + DecompressAndDumpRecoveryImage(tgt_file.path); + FAIL(); + } } // Ensures that applypatch works with a bsdiff based recovery-from-boot.p. -- cgit v1.2.3 From 641fa97def3e6ef52471e47ad25d0658ef4d943e Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 25 Apr 2018 18:59:40 -0700 Subject: Rename CacheLocation to Paths. We have a general need for overriding more paths (e.g. "/tmp"), mostly for testing purpose. Rename CacheLocation to Paths, and use that to manage TEMPORARY_{INSTALL,LOG}_FILE. Test: mmma -j bootable/recovery Test: recovery_component_test Change-Id: Ia8ce8e5695df37ca434f13ac4d3206de1e8e9396 --- tests/component/applypatch_test.cpp | 10 +++++----- tests/component/updater_test.cpp | 15 +++++++-------- 2 files changed, 12 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/tests/component/applypatch_test.cpp b/tests/component/applypatch_test.cpp index 292d76e43..158ea6359 100644 --- a/tests/component/applypatch_test.cpp +++ b/tests/component/applypatch_test.cpp @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -36,12 +35,13 @@ #include #include #include +#include #include #include "applypatch/applypatch.h" #include "applypatch/applypatch_modes.h" #include "common/test_constants.h" -#include "otautil/cache_location.h" +#include "otautil/paths.h" #include "otautil/print_sha1.h" using namespace std::string_literals; @@ -110,14 +110,14 @@ class ApplyPatchCacheTest : public ApplyPatchTest { protected: void SetUp() override { ApplyPatchTest::SetUp(); - CacheLocation::location().set_cache_temp_source(old_file); + Paths::Get().set_cache_temp_source(old_file); } }; class ApplyPatchModesTest : public ::testing::Test { protected: void SetUp() override { - CacheLocation::location().set_cache_temp_source(cache_source.path); + Paths::Get().set_cache_temp_source(cache_source.path); android::base::InitLogging(nullptr, &test_logger); android::base::SetMinimumLogSeverity(android::base::LogSeverity::DEBUG); } @@ -157,7 +157,7 @@ class FreeCacheTest : public ::testing::Test { } void SetUp() override { - CacheLocation::location().set_cache_log_directory(mock_log_dir.path); + Paths::Get().set_cache_log_directory(mock_log_dir.path); } // A mock method to calculate the free space. It assumes the partition has a total size of 40960 diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index 5bfd7cb40..5d3b2d996 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -41,8 +41,8 @@ #include "common/test_constants.h" #include "edify/expr.h" #include "otautil/SysUtil.h" -#include "otautil/cache_location.h" #include "otautil/error_code.h" +#include "otautil/paths.h" #include "otautil/print_sha1.h" #include "updater/blockimg.h" #include "updater/install.h" @@ -106,10 +106,9 @@ class UpdaterTest : public ::testing::Test { RegisterInstallFunctions(); RegisterBlockImageFunctions(); - // Mock the location of last_command_file. - CacheLocation::location().set_cache_temp_source(temp_saved_source_.path); - CacheLocation::location().set_last_command_file(temp_last_command_.path); - CacheLocation::location().set_stash_directory_base(temp_stash_base_.path); + Paths::Get().set_cache_temp_source(temp_saved_source_.path); + Paths::Get().set_last_command_file(temp_last_command_.path); + Paths::Get().set_stash_directory_base(temp_stash_base_.path); } TemporaryFile temp_saved_source_; @@ -719,7 +718,7 @@ TEST_F(UpdaterTest, brotli_new_data) { } TEST_F(UpdaterTest, last_command_update) { - std::string last_command_file = CacheLocation::location().last_command_file(); + std::string last_command_file = Paths::Get().last_command_file(); std::string block1 = std::string(4096, '1'); std::string block2 = std::string(4096, '2'); @@ -806,7 +805,7 @@ TEST_F(UpdaterTest, last_command_update) { } TEST_F(UpdaterTest, last_command_update_unresumable) { - std::string last_command_file = CacheLocation::location().last_command_file(); + std::string last_command_file = Paths::Get().last_command_file(); std::string block1 = std::string(4096, '1'); std::string block2 = std::string(4096, '2'); @@ -861,7 +860,7 @@ TEST_F(UpdaterTest, last_command_update_unresumable) { } TEST_F(UpdaterTest, last_command_verify) { - std::string last_command_file = CacheLocation::location().last_command_file(); + std::string last_command_file = Paths::Get().last_command_file(); std::string block1 = std::string(4096, '1'); std::string block2 = std::string(4096, '2'); -- cgit v1.2.3