diff options
-rw-r--r-- | mock_file_writer.h | 3 | ||||
-rw-r--r-- | mock_service_observer.h | 38 | ||||
-rw-r--r-- | payload_consumer/download_action.cc | 18 | ||||
-rw-r--r-- | payload_consumer/download_action.h | 3 | ||||
-rw-r--r-- | payload_consumer/download_action_unittest.cc | 88 | ||||
-rw-r--r-- | update_attempter.h | 9 | ||||
-rw-r--r-- | update_attempter_unittest.cc | 75 |
7 files changed, 224 insertions, 10 deletions
diff --git a/mock_file_writer.h b/mock_file_writer.h index 72d6a863..26cd45d0 100644 --- a/mock_file_writer.h +++ b/mock_file_writer.h @@ -24,7 +24,8 @@ namespace chromeos_update_engine { class MockFileWriter : public FileWriter { public: - MOCK_METHOD2(Write, ssize_t(const void* bytes, size_t count)); + MOCK_METHOD2(Write, bool(const void* bytes, size_t count)); + MOCK_METHOD3(Write, bool(const void* bytes, size_t count, ErrorCode* error)); MOCK_METHOD0(Close, int()); }; diff --git a/mock_service_observer.h b/mock_service_observer.h new file mode 100644 index 00000000..032f55a0 --- /dev/null +++ b/mock_service_observer.h @@ -0,0 +1,38 @@ +// +// Copyright (C) 2017 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef UPDATE_ENGINE_MOCK_SERVICE_OBSERVER_H_ +#define UPDATE_ENGINE_MOCK_SERVICE_OBSERVER_H_ + +#include <gmock/gmock.h> +#include "update_engine/service_observer_interface.h" + +namespace chromeos_update_engine { + +class MockServiceObserver : public ServiceObserverInterface { + public: + MOCK_METHOD5(SendStatusUpdate, + void(int64_t last_checked_time, + double progress, + update_engine::UpdateStatus status, + const std::string& new_version, + int64_t new_size)); + MOCK_METHOD1(SendPayloadApplicationComplete, void(ErrorCode error_code)); +}; + +} // namespace chromeos_update_engine + +#endif // UPDATE_ENGINE_MOCK_SERVICE_OBSERVER_H_ diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc index c3a50161..cd25962b 100644 --- a/payload_consumer/download_action.cc +++ b/payload_consumer/download_action.cc @@ -173,6 +173,7 @@ void DownloadAction::PerformAction() { install_plan_.Dump(); bytes_received_ = 0; + bytes_received_previous_payloads_ = 0; bytes_total_ = 0; for (const auto& payload : install_plan_.payloads) bytes_total_ += payload.size; @@ -317,8 +318,10 @@ void DownloadAction::ReceivedBytes(HttpFetcher* fetcher, } bytes_received_ += length; + uint64_t bytes_downloaded_total = + bytes_received_previous_payloads_ + bytes_received_; if (delegate_ && download_active_) { - delegate_->BytesReceived(length, bytes_received_, bytes_total_); + delegate_->BytesReceived(length, bytes_downloaded_total, bytes_total_); } if (writer_ && !writer_->Write(bytes, length, &code_)) { if (code_ != ErrorCode::kSuccess) { @@ -349,13 +352,16 @@ void DownloadAction::ReceivedBytes(HttpFetcher* fetcher, void DownloadAction::TransferComplete(HttpFetcher* fetcher, bool successful) { if (writer_) { LOG_IF(WARNING, writer_->Close() != 0) << "Error closing the writer."; - writer_ = nullptr; + if (delta_performer_.get() == writer_) { + // no delta_performer_ in tests, so leave the test writer in place + writer_ = nullptr; + } } download_active_ = false; ErrorCode code = successful ? ErrorCode::kSuccess : ErrorCode::kDownloadTransferError; - if (code == ErrorCode::kSuccess && delta_performer_.get()) { - if (!payload_->already_applied) + if (code == ErrorCode::kSuccess) { + if (delta_performer_ && !payload_->already_applied) code = delta_performer_->VerifyPayload(payload_->hash, payload_->size); if (code != ErrorCode::kSuccess) { LOG(ERROR) << "Download of " << install_plan_.download_url @@ -365,10 +371,12 @@ void DownloadAction::TransferComplete(HttpFetcher* fetcher, bool successful) { CloseP2PSharingFd(true); } else if (payload_ < &install_plan_.payloads.back() && system_state_->payload_state()->NextPayload()) { + LOG(INFO) << "Incrementing to next payload"; // No need to reset if this payload was already applied. - if (!payload_->already_applied) + if (delta_performer_ && !payload_->already_applied) DeltaPerformer::ResetUpdateProgress(prefs_, false); // Start downloading next payload. + bytes_received_previous_payloads_ += payload_->size; payload_++; install_plan_.download_url = system_state_->payload_state()->GetCurrentUrl(); diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h index d0e60008..786db203 100644 --- a/payload_consumer/download_action.h +++ b/payload_consumer/download_action.h @@ -166,7 +166,8 @@ class DownloadAction : public InstallPlanAction, // For reporting status to outsiders DownloadActionDelegate* delegate_; - uint64_t bytes_received_{0}; + uint64_t bytes_received_{0}; // per file/range + uint64_t bytes_received_previous_payloads_{0}; uint64_t bytes_total_{0}; bool download_active_{false}; diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc index 7d3ac6c3..f42b1d8e 100644 --- a/payload_consumer/download_action_unittest.cc +++ b/payload_consumer/download_action_unittest.cc @@ -41,6 +41,7 @@ #include "update_engine/common/utils.h" #include "update_engine/fake_p2p_manager_configuration.h" #include "update_engine/fake_system_state.h" +#include "update_engine/mock_file_writer.h" #include "update_engine/payload_consumer/mock_download_action.h" #include "update_engine/update_manager/fake_update_manager.h" @@ -56,6 +57,7 @@ using test_utils::ScopedTempFile; using testing::AtLeast; using testing::InSequence; using testing::Return; +using testing::SetArgPointee; using testing::_; class DownloadActionTest : public ::testing::Test { }; @@ -242,6 +244,92 @@ TEST(DownloadActionTest, NoDownloadDelegateTest) { false); // use_download_delegate } +TEST(DownloadActionTest, MultiPayloadProgressTest) { + std::vector<brillo::Blob> payload_datas; + // the first payload must be the largest, as it's the actual payload used by + // the MockHttpFetcher for all downloaded data. + payload_datas.emplace_back(4 * kMockHttpFetcherChunkSize + 256); + payload_datas.emplace_back(2 * kMockHttpFetcherChunkSize); + brillo::FakeMessageLoop loop(nullptr); + loop.SetAsCurrent(); + FakeSystemState fake_system_state; + EXPECT_CALL(*fake_system_state.mock_payload_state(), NextPayload()) + .WillOnce(Return(true)); + + MockFileWriter mock_file_writer; + EXPECT_CALL(mock_file_writer, Close()).WillRepeatedly(Return(0)); + EXPECT_CALL(mock_file_writer, Write(_, _, _)) + .WillRepeatedly( + DoAll(SetArgPointee<2>(ErrorCode::kSuccess), Return(true))); + + InstallPlan install_plan; + uint64_t total_expected_download_size{0}; + for (const auto& data : payload_datas) { + uint64_t size = data.size(); + install_plan.payloads.push_back( + {.size = size, .type = InstallPayloadType::kFull}); + total_expected_download_size += size; + } + ObjectFeederAction<InstallPlan> feeder_action; + feeder_action.set_obj(install_plan); + MockPrefs prefs; + MockHttpFetcher* http_fetcher = new MockHttpFetcher( + payload_datas[0].data(), payload_datas[0].size(), nullptr); + // takes ownership of passed in HttpFetcher + DownloadAction download_action(&prefs, + fake_system_state.boot_control(), + fake_system_state.hardware(), + &fake_system_state, + http_fetcher); + download_action.SetTestFileWriter(&mock_file_writer); + BondActions(&feeder_action, &download_action); + MockDownloadActionDelegate download_delegate; + { + InSequence s; + download_action.set_delegate(&download_delegate); + // these are hand-computed based on the payloads specified above + EXPECT_CALL(download_delegate, + BytesReceived(kMockHttpFetcherChunkSize, + kMockHttpFetcherChunkSize, + total_expected_download_size)); + EXPECT_CALL(download_delegate, + BytesReceived(kMockHttpFetcherChunkSize, + kMockHttpFetcherChunkSize * 2, + total_expected_download_size)); + EXPECT_CALL(download_delegate, + BytesReceived(kMockHttpFetcherChunkSize, + kMockHttpFetcherChunkSize * 3, + total_expected_download_size)); + EXPECT_CALL(download_delegate, + BytesReceived(kMockHttpFetcherChunkSize, + kMockHttpFetcherChunkSize * 4, + total_expected_download_size)); + EXPECT_CALL(download_delegate, + BytesReceived(256, + kMockHttpFetcherChunkSize * 4 + 256, + total_expected_download_size)); + EXPECT_CALL(download_delegate, + BytesReceived(kMockHttpFetcherChunkSize, + kMockHttpFetcherChunkSize * 5 + 256, + total_expected_download_size)); + EXPECT_CALL(download_delegate, + BytesReceived(kMockHttpFetcherChunkSize, + total_expected_download_size, + total_expected_download_size)); + } + ActionProcessor processor; + processor.EnqueueAction(&feeder_action); + processor.EnqueueAction(&download_action); + + loop.PostTask( + FROM_HERE, + base::Bind( + [](ActionProcessor* processor) { processor->StartProcessing(); }, + base::Unretained(&processor))); + loop.Run(); + EXPECT_FALSE(loop.PendingTasks()); +} + namespace { class TerminateEarlyTestProcessorDelegate : public ActionProcessorDelegate { public: diff --git a/update_attempter.h b/update_attempter.h index b4e2f60b..9dd844dd 100644 --- a/update_attempter.h +++ b/update_attempter.h @@ -252,17 +252,20 @@ class UpdateAttempter : public ActionProcessorDelegate, FRIEND_TEST(UpdateAttempterTest, ActionCompletedDownloadTest); FRIEND_TEST(UpdateAttempterTest, ActionCompletedErrorTest); FRIEND_TEST(UpdateAttempterTest, ActionCompletedOmahaRequestTest); + FRIEND_TEST(UpdateAttempterTest, BootTimeInUpdateMarkerFile); + FRIEND_TEST(UpdateAttempterTest, BroadcastCompleteDownloadTest); + FRIEND_TEST(UpdateAttempterTest, ChangeToDownloadingOnReceivedBytesTest); FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventTest); FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventResumedTest); FRIEND_TEST(UpdateAttempterTest, DisableDeltaUpdateIfNeededTest); + FRIEND_TEST(UpdateAttempterTest, DownloadProgressAccumulationTest); FRIEND_TEST(UpdateAttempterTest, MarkDeltaUpdateFailureTest); FRIEND_TEST(UpdateAttempterTest, PingOmahaTest); + FRIEND_TEST(UpdateAttempterTest, ReportDailyMetrics); FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest); FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionTest); - FRIEND_TEST(UpdateAttempterTest, UpdateTest); - FRIEND_TEST(UpdateAttempterTest, ReportDailyMetrics); - FRIEND_TEST(UpdateAttempterTest, BootTimeInUpdateMarkerFile); FRIEND_TEST(UpdateAttempterTest, TargetVersionPrefixSetAndReset); + FRIEND_TEST(UpdateAttempterTest, UpdateTest); // CertificateChecker::Observer method. // Report metrics about the certificate being checked. diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc index 4ebc85c8..911f6640 100644 --- a/update_attempter_unittest.cc +++ b/update_attempter_unittest.cc @@ -48,6 +48,7 @@ #include "update_engine/fake_system_state.h" #include "update_engine/mock_p2p_manager.h" #include "update_engine/mock_payload_state.h" +#include "update_engine/mock_service_observer.h" #include "update_engine/payload_consumer/filesystem_verifier_action.h" #include "update_engine/payload_consumer/install_plan.h" #include "update_engine/payload_consumer/payload_constants.h" @@ -219,6 +220,7 @@ TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) { EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0); attempter_.ActionCompleted(nullptr, &action, ErrorCode::kSuccess); EXPECT_EQ(UpdateStatus::FINALIZING, attempter_.status()); + EXPECT_EQ(0.0, attempter_.download_progress_); ASSERT_EQ(nullptr, attempter_.error_event_.get()); } @@ -232,6 +234,79 @@ TEST_F(UpdateAttempterTest, ActionCompletedErrorTest) { ASSERT_NE(nullptr, attempter_.error_event_.get()); } +TEST_F(UpdateAttempterTest, DownloadProgressAccumulationTest) { + // Simple test case, where all the values match (nothing was skipped) + uint64_t bytes_progressed_1 = 1024 * 1024; // 1MB + uint64_t bytes_progressed_2 = 1024 * 1024; // 1MB + uint64_t bytes_received_1 = bytes_progressed_1; + uint64_t bytes_received_2 = bytes_received_1 + bytes_progressed_2; + uint64_t bytes_total = 20 * 1024 * 1024; // 20MB + + double progress_1 = + static_cast<double>(bytes_received_1) / static_cast<double>(bytes_total); + double progress_2 = + static_cast<double>(bytes_received_2) / static_cast<double>(bytes_total); + + EXPECT_EQ(0.0, attempter_.download_progress_); + // This is set via inspecting the InstallPlan payloads when the + // OmahaResponseAction is completed + attempter_.new_payload_size_ = bytes_total; + NiceMock<MockServiceObserver> observer; + EXPECT_CALL(observer, + SendStatusUpdate( + _, progress_1, UpdateStatus::DOWNLOADING, _, bytes_total)); + EXPECT_CALL(observer, + SendStatusUpdate( + _, progress_2, UpdateStatus::DOWNLOADING, _, bytes_total)); + attempter_.AddObserver(&observer); + attempter_.BytesReceived(bytes_progressed_1, bytes_received_1, bytes_total); + EXPECT_EQ(progress_1, attempter_.download_progress_); + // This iteration validates that a later set of updates to the variables are + // properly handled (so that |getStatus()| will return the same progress info + // as the callback is receiving. + attempter_.BytesReceived(bytes_progressed_2, bytes_received_2, bytes_total); + EXPECT_EQ(progress_2, attempter_.download_progress_); +} + +TEST_F(UpdateAttempterTest, ChangeToDownloadingOnReceivedBytesTest) { + // The transition into UpdateStatus::DOWNLOADING happens when the + // first bytes are received. + uint64_t bytes_progressed = 1024 * 1024; // 1MB + uint64_t bytes_received = 2 * 1024 * 1024; // 2MB + uint64_t bytes_total = 20 * 1024 * 1024; // 300MB + attempter_.status_ = UpdateStatus::CHECKING_FOR_UPDATE; + // This is set via inspecting the InstallPlan payloads when the + // OmahaResponseAction is completed + attempter_.new_payload_size_ = bytes_total; + EXPECT_EQ(0.0, attempter_.download_progress_); + NiceMock<MockServiceObserver> observer; + EXPECT_CALL( + observer, + SendStatusUpdate(_, _, UpdateStatus::DOWNLOADING, _, bytes_total)); + attempter_.AddObserver(&observer); + attempter_.BytesReceived(bytes_progressed, bytes_received, bytes_total); + EXPECT_EQ(UpdateStatus::DOWNLOADING, attempter_.status_); +} + +TEST_F(UpdateAttempterTest, BroadcastCompleteDownloadTest) { + // There is a special case to ensure that at 100% downloaded, + // download_progress_ is updated and that value broadcast. This test confirms + // that. + uint64_t bytes_progressed = 0; // ignored + uint64_t bytes_received = 5 * 1024 * 1024; // ignored + uint64_t bytes_total = 5 * 1024 * 1024; // 300MB + attempter_.status_ = UpdateStatus::DOWNLOADING; + attempter_.new_payload_size_ = bytes_total; + EXPECT_EQ(0.0, attempter_.download_progress_); + NiceMock<MockServiceObserver> observer; + EXPECT_CALL( + observer, + SendStatusUpdate(_, 1.0, UpdateStatus::DOWNLOADING, _, bytes_total)); + attempter_.AddObserver(&observer); + attempter_.BytesReceived(bytes_progressed, bytes_received, bytes_total); + EXPECT_EQ(1.0, attempter_.download_progress_); +} + TEST_F(UpdateAttempterTest, ActionCompletedOmahaRequestTest) { unique_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, nullptr)); fetcher->FailTransfer(500); // Sets the HTTP response code. |