From a36b472ad9c95758b9839d7cc753ef54a34baa90 Mon Sep 17 00:00:00 2001 From: Alex Pankhurst Date: Fri, 1 Nov 2019 19:47:04 +0000 Subject: [PATCH] [crash] added new Inspect metrics * Added two new metrics for reports: 1) Number of upload attempts 2) The final state of the report * The final state of the report is handled by the database. * More metrics will be coming soon! TESTED=`fx run-test crashpad_agent_tests` Change-Id: Iee140487df481ab5d06f80e7d77d5ea280308f2e --- .../feedback/crashpad_agent/database.cc | 16 ++- .../feedback/crashpad_agent/database.h | 8 +- .../crashpad_agent/inspect_manager.cc | 44 ++++++ .../feedback/crashpad_agent/inspect_manager.h | 17 +++ .../feedback/crashpad_agent/queue.cc | 5 +- .../feedback/crashpad_agent/tests/BUILD.gn | 2 + .../tests/crashpad_agent_unittest.cc | 6 +- .../crashpad_agent/tests/database_unittest.cc | 128 +++++++++++++++++- .../tests/inspect_manager_unittest.cc | 80 ++++++++++- .../crashpad_agent/tests/queue_unittest.cc | 80 ++++++----- 10 files changed, 338 insertions(+), 48 deletions(-) diff --git a/src/developer/feedback/crashpad_agent/database.cc b/src/developer/feedback/crashpad_agent/database.cc index 0742ccec8816..d440b0ed3469 100644 --- a/src/developer/feedback/crashpad_agent/database.cc +++ b/src/developer/feedback/crashpad_agent/database.cc @@ -23,7 +23,8 @@ using OperationStatus = crashpad::CrashReportDatabase::OperationStatus; constexpr char kCrashpadDatabasePath[] = "/tmp/crashes"; -std::unique_ptr Database::TryCreate(CrashpadDatabaseConfig config) { +std::unique_ptr Database::TryCreate(CrashpadDatabaseConfig config, + InspectManager* inspect_manager) { if (!files::IsDirectory(kCrashpadDatabasePath)) { files::CreateDirectory(kCrashpadDatabasePath); } @@ -36,12 +37,14 @@ std::unique_ptr Database::TryCreate(CrashpadDatabaseConfig config) { return nullptr; } - return std::unique_ptr(new Database(config, std::move(crashpad_database))); + return std::unique_ptr( + new Database(config, std::move(crashpad_database), inspect_manager)); } Database::Database(CrashpadDatabaseConfig config, - std::unique_ptr database) - : config_(config), database_(std::move(database)) { + std::unique_ptr database, + InspectManager* inspect_manager) + : config_(config), database_(std::move(database)), inspect_manager_(inspect_manager) { FXL_DCHECK(database_); } @@ -110,6 +113,8 @@ bool Database::MarkAsUploaded(std::unique_ptr upload_report, } const UUID local_report_id = upload_report->GetUUID(); + + inspect_manager_->MarkReportAsUploaded(local_report_id.ToString(), server_report_id); if (const auto status = database_->RecordUploadComplete(upload_report->TransferUploadReport(), server_report_id); status != OperationStatus::kNoError) { @@ -127,6 +132,8 @@ bool Database::Archive(const crashpad::UUID& local_report_id) { FX_LOGS(INFO) << fxl::StringPrintf("Archiving local crash report, ID %s, under %s", local_report_id.ToString().c_str(), kCrashpadDatabasePath); + inspect_manager_->MarkReportAsArchived(local_report_id.ToString()); + if (const auto status = database_->SkipReportUpload(local_report_id, CrashSkippedReason::kUploadFailed); status != OperationStatus::kNoError) { @@ -173,6 +180,7 @@ size_t Database::GarbageCollect() { } for (const auto& uuid : clean_up) { + inspect_manager_->MarkReportAsGarbageCollected(uuid.ToString()); CleanUp(uuid); } } diff --git a/src/developer/feedback/crashpad_agent/database.h b/src/developer/feedback/crashpad_agent/database.h index 973018c67ce5..059c8a8dd978 100644 --- a/src/developer/feedback/crashpad_agent/database.h +++ b/src/developer/feedback/crashpad_agent/database.h @@ -12,6 +12,7 @@ #include #include "src/developer/feedback/crashpad_agent/config.h" +#include "src/developer/feedback/crashpad_agent/inspect_manager.h" #include "src/developer/feedback/crashpad_agent/upload_report.h" #include "src/lib/fxl/macros.h" #include "third_party/crashpad/client/crash_report_database.h" @@ -22,7 +23,8 @@ namespace feedback { // Wrapper around the Crashpad database that also stores annotations. class Database { public: - static std::unique_ptr TryCreate(CrashpadDatabaseConfig config); + static std::unique_ptr TryCreate(CrashpadDatabaseConfig config, + InspectManager* inspect_manager); // Make a new report in |database_|. // @@ -72,13 +74,15 @@ class Database { bool has_minidump; }; - Database(CrashpadDatabaseConfig config, std::unique_ptr database); + Database(CrashpadDatabaseConfig config, std::unique_ptr database, + InspectManager* inspect_manager); // Removes |local_report_id| from |additional_data_|. void CleanUp(const crashpad::UUID& local_report_id); const CrashpadDatabaseConfig config_; std::unique_ptr database_; + InspectManager* inspect_manager_; std::unordered_map additional_data_; FXL_DISALLOW_COPY_AND_ASSIGN(Database); diff --git a/src/developer/feedback/crashpad_agent/inspect_manager.cc b/src/developer/feedback/crashpad_agent/inspect_manager.cc index 3cfc168b598b..4b9e14b6166a 100644 --- a/src/developer/feedback/crashpad_agent/inspect_manager.cc +++ b/src/developer/feedback/crashpad_agent/inspect_manager.cc @@ -51,6 +51,23 @@ bool InspectManager::AddReport(const std::string& program_name, return true; } +bool InspectManager::IncrementUploadAttempt(const std::string& local_report_id) { + if (!Contains(local_report_id)) { + FX_LOGS(ERROR) << "Failed to find local crash report, ID " << local_report_id; + return false; + } + + Report& report = reports_.at(local_report_id); + + if (!report.upload_attempts_) { + report.upload_attempts_ = node_manager_.Get(report.Path())->CreateUint("upload_attempts", 1u); + } else { + report.upload_attempts_.Add(1u); + } + + return true; +} + bool InspectManager::MarkReportAsUploaded(const std::string& local_report_id, const std::string& server_properties_report_id) { if (!Contains(local_report_id)) { @@ -59,6 +76,8 @@ bool InspectManager::MarkReportAsUploaded(const std::string& local_report_id, } Report& report = reports_.at(local_report_id); + report.final_state_ = node_manager_.Get(report.Path())->CreateString("final_state", "uploaded"); + const std::string server_path = JoinPath(report.Path(), "crash_server"); inspect::Node* server = node_manager_.Get(server_path); @@ -69,6 +88,31 @@ bool InspectManager::MarkReportAsUploaded(const std::string& local_report_id, return true; } +bool InspectManager::MarkReportAsArchived(const std::string& local_report_id) { + if (!Contains(local_report_id)) { + FX_LOGS(ERROR) << "Failed to find local crash report, ID " << local_report_id; + return false; + } + + Report& report = reports_.at(local_report_id); + report.final_state_ = node_manager_.Get(report.Path())->CreateString("final_state", "archived"); + + return true; +} + +bool InspectManager::MarkReportAsGarbageCollected(const std::string& local_report_id) { + if (!Contains(local_report_id)) { + FX_LOGS(ERROR) << "Failed to find local crash report, ID " << local_report_id; + return false; + } + + Report& report = reports_.at(local_report_id); + report.final_state_ = + node_manager_.Get(report.Path())->CreateString("final_state", "garbage_collected"); + + return true; +} + void InspectManager::ExposeConfig(const feedback::Config& config) { auto* crashpad_database = &config_.crashpad_database; auto* crash_server = &config_.crash_server; diff --git a/src/developer/feedback/crashpad_agent/inspect_manager.h b/src/developer/feedback/crashpad_agent/inspect_manager.h index 67c738e8d081..dcebf9cdac61 100644 --- a/src/developer/feedback/crashpad_agent/inspect_manager.h +++ b/src/developer/feedback/crashpad_agent/inspect_manager.h @@ -36,12 +36,27 @@ class InspectManager { // or another). bool AddReport(const std::string& program_name, const std::string& local_report_id); + // Increments the number of upload attempts for an existing report. + // + // Returns false if there are no reports with |local_report_id| as ID. + bool IncrementUploadAttempt(const std::string& local_report_id); + // Marks an existing report as uploaded, storing its server report ID. // // Returns false if there are no reports with |local_report_id| as ID. bool MarkReportAsUploaded(const std::string& local_report_id, const std::string& server_report_id); + // Mark an existing report as archived. + // + // Returns false if there are no reports with |local_report_id| as ID. + bool MarkReportAsArchived(const std::string& local_report_id); + + // Mark an existing report as garbage collected. + // + // Returns false if there are no report with |local_report_id| as ID. + bool MarkReportAsGarbageCollected(const std::string& local_report_id); + private: bool Contains(const std::string& local_report_id); @@ -81,6 +96,8 @@ class InspectManager { const std::string& Path() { return path_; } inspect::StringProperty creation_time_; + inspect::UintProperty upload_attempts_; + inspect::StringProperty final_state_; inspect::StringProperty server_id_; inspect::StringProperty server_creation_time_; diff --git a/src/developer/feedback/crashpad_agent/queue.cc b/src/developer/feedback/crashpad_agent/queue.cc index b920b833df30..aadd47a70734 100644 --- a/src/developer/feedback/crashpad_agent/queue.cc +++ b/src/developer/feedback/crashpad_agent/queue.cc @@ -23,7 +23,7 @@ std::unique_ptr Queue::TryCreate(async_dispatcher_t* dispatcher, CrashpadDatabaseConfig database_config, CrashServer* crash_server, InspectManager* inspect_manager) { - auto database = Database::TryCreate(database_config); + auto database = Database::TryCreate(database_config, inspect_manager); if (!database) { return nullptr; } @@ -104,13 +104,14 @@ bool Queue::Upload(const UUID& local_report_id) { return true; } + inspect_manager_->IncrementUploadAttempt(local_report_id.ToString()); + std::string server_report_id; if (crash_server_->MakeRequest(report->GetAnnotations(), report->GetAttachments(), &server_report_id)) { FX_LOGS(INFO) << "Successfully uploaded crash report at https://crash.corp.google.com/" << server_report_id; database_->MarkAsUploaded(std::move(report), server_report_id); - inspect_manager_->MarkReportAsUploaded(local_report_id.ToString(), server_report_id); return true; } diff --git a/src/developer/feedback/crashpad_agent/tests/BUILD.gn b/src/developer/feedback/crashpad_agent/tests/BUILD.gn index cbae9ceaed94..b6a5e88d93c9 100644 --- a/src/developer/feedback/crashpad_agent/tests/BUILD.gn +++ b/src/developer/feedback/crashpad_agent/tests/BUILD.gn @@ -224,6 +224,8 @@ executable("database_unittest") { ] deps = [ + "//garnet/public/lib/timekeeper:testing", + "//sdk/lib/inspect/testing/cpp", "//src/developer/feedback/crashpad_agent:src", "//src/lib/fsl:fsl", "//src/lib/fxl/test:test_settings", diff --git a/src/developer/feedback/crashpad_agent/tests/crashpad_agent_unittest.cc b/src/developer/feedback/crashpad_agent/tests/crashpad_agent_unittest.cc index b9006ab19141..2469397624d7 100644 --- a/src/developer/feedback/crashpad_agent/tests/crashpad_agent_unittest.cc +++ b/src/developer/feedback/crashpad_agent/tests/crashpad_agent_unittest.cc @@ -782,7 +782,11 @@ TEST_F(CrashpadAgentTest, Check_InspectTreeAfterSuccessfulUpload) { ChildrenMatch(ElementsAre(AllOf( NodeMatches(NameMatches(kProgramName)), ChildrenMatch(ElementsAre(AllOf( - NodeMatches(PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty()))))), + NodeMatches(PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("final_state", "uploaded"), + UintIs("upload_attempts", 1u), + }))), ChildrenMatch(ElementsAre(NodeMatches(AllOf( NameMatches("crash_server"), PropertyList(UnorderedElementsAreArray({ StringIs("creation_time", Not(IsEmpty())), diff --git a/src/developer/feedback/crashpad_agent/tests/database_unittest.cc b/src/developer/feedback/crashpad_agent/tests/database_unittest.cc index 4c5a928b01e9..5c16c8013298 100644 --- a/src/developer/feedback/crashpad_agent/tests/database_unittest.cc +++ b/src/developer/feedback/crashpad_agent/tests/database_unittest.cc @@ -5,7 +5,12 @@ #include "src/developer/feedback/crashpad_agent/database.h" #include +#include +#include +#include +#include +#include "sdk/lib/inspect/testing/cpp/inspect.h" #include "src/lib/files/directory.h" #include "src/lib/files/path.h" #include "src/lib/fsl/vmo/strings.h" @@ -19,15 +24,24 @@ namespace feedback { namespace { using crashpad::UUID; +using inspect::testing::ChildrenMatch; +using inspect::testing::NameMatches; +using inspect::testing::NodeMatches; +using inspect::testing::PropertyList; +using inspect::testing::StringIs; using testing::Contains; using testing::ElementsAre; using testing::IsEmpty; using testing::IsSubsetOf; using testing::IsSupersetOf; using testing::Key; +using testing::Not; using testing::Pair; using testing::UnorderedElementsAreArray; +constexpr zx::time_utc kTime((zx::hour(7) + zx::min(14) + zx::sec(52)).get()); +constexpr char kTimeStr[] = "1970-01-01 07:14:52 GMT"; + constexpr uint64_t kMaxTotalReportsSizeInKb = 1024u; constexpr char kCrashpadDatabasePath[] = "/tmp/crashes"; @@ -69,7 +83,13 @@ std::string GenerateString(const uint64_t string_size_in_kb) { class DatabaseTest : public ::testing::Test { public: - void SetUp() override { SetUpDatabase(/*max_size_in_kb=*/kMaxTotalReportsSizeInKb); } + void SetUp() override { + clock_ = std::make_unique(); + inspector_ = std::make_unique(); + inspect_manager_ = std::make_unique(&inspector_->GetRoot(), clock_.get()); + + SetUpDatabase(/*max_size_in_kb=*/kMaxTotalReportsSizeInKb); + } void TearDown() override { ASSERT_TRUE(files::DeletePath(kCrashpadDatabasePath, /*recursive=*/true)); @@ -77,7 +97,8 @@ class DatabaseTest : public ::testing::Test { protected: void SetUpDatabase(const uint64_t max_size_in_kb) { - auto new_database = Database::TryCreate(CrashpadDatabaseConfig{max_size_in_kb}); + auto new_database = + Database::TryCreate(CrashpadDatabaseConfig{max_size_in_kb}, inspect_manager_.get()); FXL_CHECK(new_database) << "Error creating database"; database_ = std::move(new_database); attachments_dir_ = files::JoinPath(kCrashpadDatabasePath, kCrashpadAttachmentsDir); @@ -131,6 +152,12 @@ class DatabaseTest : public ::testing::Test { })); } + inspect::Hierarchy InspectTree() { + auto result = inspect::ReadFromVmo(inspector_->DuplicateVmo()); + FXL_CHECK(result.is_ok()); + return result.take_value(); + } + private: std::vector GetDirectoryContents(const std::string& path) { std::vector contents; @@ -148,12 +175,17 @@ class DatabaseTest : public ::testing::Test { } protected: - std::unique_ptr database_; + std::unique_ptr clock_; + std::unique_ptr inspect_manager_; std::string attachments_dir_; private: + std::unique_ptr inspector_; std::string completed_dir_; std::string pending_dir_; + + protected: + std::unique_ptr database_; }; TEST_F(DatabaseTest, Check_DatabaseIsEmpty_OnPruneDatabaseWithZeroSize) { @@ -416,6 +448,96 @@ TEST_F(DatabaseTest, Attempt_Archive_AfterReportIsPruned) { EXPECT_FALSE(database_->Archive(pruned_report)); } +TEST_F(DatabaseTest, Check_InspectTree_ReportUploaded) { + SetUp(); + clock_->Set(kTime); + + // Add a crash report. + UUID local_report_id; + MakeNewReportOrDie(&local_report_id); + + // Get the upload report. + auto upload_report = database_->GetUploadReport(local_report_id); + ASSERT_TRUE(upload_report); + + // Add the report to Inspect. + EXPECT_TRUE(inspect_manager_->AddReport("program", local_report_id.ToString())); + + // Mark the report as uploaded and check the Inspect tree. + EXPECT_TRUE(database_->MarkAsUploaded(std::move(upload_report), "server_report_id")); + EXPECT_THAT(InspectTree(), + ChildrenMatch(Contains(AllOf( + NodeMatches(NameMatches("reports")), + ChildrenMatch(ElementsAre(AllOf( + NodeMatches(NameMatches("program")), + ChildrenMatch(ElementsAre(AllOf( + NodeMatches(AllOf(NameMatches(local_report_id.ToString()), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTimeStr), + StringIs("final_state", "uploaded"), + })))), + ChildrenMatch(ElementsAre(NodeMatches(AllOf( + NameMatches("crash_server"), PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTimeStr), + StringIs("id", "server_report_id"), + })))))))))))))))); +} + +TEST_F(DatabaseTest, Check_InspectTree_ReportArchived) { + SetUp(); + clock_->Set(kTime); + + // Add a crash report. + UUID local_report_id; + MakeNewReportOrDie(&local_report_id); + + // Add the report to Inspect. + EXPECT_TRUE(inspect_manager_->AddReport("program", local_report_id.ToString())); + + // Archive the report and check the Inspect tree. + EXPECT_TRUE(database_->Archive(local_report_id)); + EXPECT_THAT( + InspectTree(), + ChildrenMatch(Contains(AllOf( + NodeMatches(NameMatches("reports")), + ChildrenMatch(ElementsAre(AllOf( + NodeMatches(NameMatches("program")), + ChildrenMatch(ElementsAre(AllOf(NodeMatches(AllOf( + NameMatches(local_report_id.ToString()), PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTimeStr), + StringIs("final_state", "archived"), + })))))))))))))); +} + +TEST_F(DatabaseTest, Check_InspectTree_ReportGarbageCollected) { + // Set up the database with a max size of 0, meaning any reports in the database with size > 0 + // will get garbage collected. + SetUpDatabase(/*max_size_in_kb=*/0u); + clock_->Set(kTime); + + // Add a crash report. + UUID local_report_id; + MakeNewReportOrDie( + /*attachments=*/{{kAttachmentKey, kAttachmentValue}}, &local_report_id); + + // Add the report to Inpsect. + EXPECT_TRUE(inspect_manager_->AddReport("program", local_report_id.ToString())); + + // Check that garbage collection occurs correctly and check the Inspect tree. + EXPECT_EQ(database_->GarbageCollect(), 1u); + EXPECT_THAT( + InspectTree(), + ChildrenMatch(Contains(AllOf( + NodeMatches(NameMatches("reports")), + ChildrenMatch(ElementsAre(AllOf(NodeMatches(NameMatches("program")), + ChildrenMatch(ElementsAre(AllOf(NodeMatches(AllOf( + NameMatches(local_report_id.ToString()), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTimeStr), + StringIs("final_state", "garbage_collected"), + })))))))))))))); +} + } // namespace } // namespace feedback diff --git a/src/developer/feedback/crashpad_agent/tests/inspect_manager_unittest.cc b/src/developer/feedback/crashpad_agent/tests/inspect_manager_unittest.cc index 04b3795456c1..7ae1e2f66299 100644 --- a/src/developer/feedback/crashpad_agent/tests/inspect_manager_unittest.cc +++ b/src/developer/feedback/crashpad_agent/tests/inspect_manager_unittest.cc @@ -164,10 +164,27 @@ TEST_F(InspectManagerTest, Fail_AddReport_DuplicateReport) { PropertyList(ElementsAre(StringIs("creation_time", kTime2Str)))))))))))))); } +TEST_F(InspectManagerTest, Succeed_IncrementUploadAttempt) { + clock_->Set(kTime2); + EXPECT_TRUE(inspect_manager_->AddReport("program", "local_report_id")); + EXPECT_TRUE(inspect_manager_->IncrementUploadAttempt("local_report_id")); + EXPECT_THAT(InspectTree(), + ChildrenMatch(Contains(AllOf( + NodeMatches(NameMatches(kInspectReportsName)), + ChildrenMatch(ElementsAre(AllOf( + NodeMatches(NameMatches("program")), + ChildrenMatch(ElementsAre(AllOf(NodeMatches(AllOf( + NameMatches("local_report_id"), PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTime2Str), + UintIs("upload_attempts", 1u), + })))))))))))))); +} + TEST_F(InspectManagerTest, Succeed_MarkReportAsUploaded) { clock_->Set(kTime2); EXPECT_TRUE(inspect_manager_->AddReport("program", "local_report_id")); clock_->Set(kTime3); + EXPECT_TRUE(inspect_manager_->IncrementUploadAttempt("local_report_id")); EXPECT_TRUE(inspect_manager_->MarkReportAsUploaded("local_report_id", "server_report_id")); EXPECT_THAT(InspectTree(), ChildrenMatch(Contains(AllOf( @@ -175,9 +192,12 @@ TEST_F(InspectManagerTest, Succeed_MarkReportAsUploaded) { ChildrenMatch(ElementsAre(AllOf( NodeMatches(NameMatches("program")), ChildrenMatch(ElementsAre(AllOf( - NodeMatches(AllOf( - NameMatches("local_report_id"), - PropertyList(ElementsAre(StringIs("creation_time", kTime2Str))))), + NodeMatches(AllOf(NameMatches("local_report_id"), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTime2Str), + StringIs("final_state", "uploaded"), + UintIs("upload_attempts", 1u), + })))), ChildrenMatch(ElementsAre(NodeMatches(AllOf( NameMatches("crash_server"), PropertyList(UnorderedElementsAreArray({ StringIs("creation_time", kTime3Str), @@ -185,6 +205,46 @@ TEST_F(InspectManagerTest, Succeed_MarkReportAsUploaded) { })))))))))))))))); } +TEST_F(InspectManagerTest, Succeed_MarkReportAsArchived) { + clock_->Set(kTime2); + EXPECT_TRUE(inspect_manager_->AddReport("program", "local_report_id")); + EXPECT_TRUE(inspect_manager_->MarkReportAsArchived("local_report_id")); + EXPECT_THAT(InspectTree(), + ChildrenMatch(Contains(AllOf( + NodeMatches(NameMatches(kInspectReportsName)), + ChildrenMatch(ElementsAre(AllOf( + NodeMatches(NameMatches("program")), + ChildrenMatch(ElementsAre(AllOf(NodeMatches(AllOf( + NameMatches("local_report_id"), PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTime2Str), + StringIs("final_state", "archived"), + })))))))))))))); +} + +TEST_F(InspectManagerTest, Succeed_MarkReportAsGarbageCollected) { + clock_->Set(kTime2); + EXPECT_TRUE(inspect_manager_->AddReport("program", "local_report_id")); + EXPECT_TRUE(inspect_manager_->MarkReportAsGarbageCollected("local_report_id")); + EXPECT_THAT( + InspectTree(), + ChildrenMatch(Contains(AllOf( + NodeMatches(NameMatches(kInspectReportsName)), + ChildrenMatch(ElementsAre(AllOf( + NodeMatches(NameMatches("program")), + ChildrenMatch(ElementsAre(AllOf(NodeMatches(AllOf( + NameMatches("local_report_id"), PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", kTime2Str), + StringIs("final_state", "garbage_collected"), + })))))))))))))); +} + +TEST_F(InspectManagerTest, Fail_IncrementUploadAttempt_UnknownReport) { + EXPECT_FALSE(inspect_manager_->IncrementUploadAttempt("unknown_report")); + EXPECT_THAT(InspectTree(), + ChildrenMatch(Contains( + AllOf(NodeMatches(NameMatches(kInspectReportsName)), ChildrenMatch(IsEmpty()))))); +} + TEST_F(InspectManagerTest, Fail_MarkReportAsUploaded_UnknownReport) { EXPECT_FALSE(inspect_manager_->MarkReportAsUploaded("unknown_report", "server_report_id")); EXPECT_THAT(InspectTree(), @@ -192,6 +252,20 @@ TEST_F(InspectManagerTest, Fail_MarkReportAsUploaded_UnknownReport) { AllOf(NodeMatches(NameMatches(kInspectReportsName)), ChildrenMatch(IsEmpty()))))); } +TEST_F(InspectManagerTest, Fail_MarkReportAsArchived_UnknownReport) { + EXPECT_FALSE(inspect_manager_->MarkReportAsArchived("unknown_report")); + EXPECT_THAT(InspectTree(), + ChildrenMatch(Contains( + AllOf(NodeMatches(NameMatches(kInspectReportsName)), ChildrenMatch(IsEmpty()))))); +} + +TEST_F(InspectManagerTest, Fail_MarkReportAsGarbageCollected_UnknownReport) { + EXPECT_FALSE(inspect_manager_->MarkReportAsGarbageCollected("unknown_report")); + EXPECT_THAT(InspectTree(), + ChildrenMatch(Contains( + AllOf(NodeMatches(NameMatches(kInspectReportsName)), ChildrenMatch(IsEmpty()))))); +} + TEST_F(InspectManagerTest, ExposeConfig_UploadEnabled) { inspect_manager_->ExposeConfig(Config{ /*crashpad_database=*/ diff --git a/src/developer/feedback/crashpad_agent/tests/queue_unittest.cc b/src/developer/feedback/crashpad_agent/tests/queue_unittest.cc index f7854a7c626e..38d6d2948630 100644 --- a/src/developer/feedback/crashpad_agent/tests/queue_unittest.cc +++ b/src/developer/feedback/crashpad_agent/tests/queue_unittest.cc @@ -35,6 +35,7 @@ using inspect::testing::NameMatches; using inspect::testing::NodeMatches; using inspect::testing::PropertyList; using inspect::testing::StringIs; +using inspect::testing::UintIs; using testing::Contains; using testing::ElementsAre; using testing::IsEmpty; @@ -467,47 +468,60 @@ TEST_F(QueueTest, Check_InspectTree) { NodeMatches(NameMatches(kInspectReportsName)), ChildrenMatch(IsSupersetOf({ AllOf(NodeMatches(NameMatches("program_1")), - ChildrenMatch(ElementsAre(AllOf( - NodeMatches(AllOf( - PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty())))))), - ChildrenMatch(ElementsAre( - NodeMatches(AllOf(NameMatches("crash_server"), - PropertyList(UnorderedElementsAreArray({ - StringIs("creation_time", Not(IsEmpty())), - StringIs("id", kStubServerReportId), - })))))))))), + ChildrenMatch( + ElementsAre(AllOf(NodeMatches(AllOf(PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("final_state", "uploaded"), + UintIs("upload_attempts", 1u), + })))), + ChildrenMatch(ElementsAre(NodeMatches( + AllOf(NameMatches("crash_server"), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("id", kStubServerReportId), + })))))))))), AllOf(NodeMatches(NameMatches("program_2")), - ChildrenMatch(ElementsAre(AllOf( - NodeMatches(AllOf( - PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty())))))), - ChildrenMatch(ElementsAre( - NodeMatches(AllOf(NameMatches("crash_server"), - PropertyList(UnorderedElementsAreArray({ - StringIs("creation_time", Not(IsEmpty())), - StringIs("id", kStubServerReportId), - })))))))))), + ChildrenMatch( + ElementsAre(AllOf(NodeMatches(AllOf(PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("final_state", "uploaded"), + UintIs("upload_attempts", 1u), + })))), + ChildrenMatch(ElementsAre(NodeMatches( + AllOf(NameMatches("crash_server"), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("id", kStubServerReportId), + })))))))))), AllOf(NodeMatches(NameMatches("program_3")), ChildrenMatch(UnorderedElementsAreArray({ - NodeMatches(AllOf( - NameMatches(expected_queue_contents_[0].ToString()), - PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty())))))), + NodeMatches(AllOf(NameMatches(expected_queue_contents_[0].ToString()), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + UintIs("upload_attempts", 1u), + })))), }))), AllOf(NodeMatches(NameMatches("program_4")), ChildrenMatch(UnorderedElementsAreArray({ - NodeMatches(AllOf( - NameMatches(expected_queue_contents_[1].ToString()), - PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty())))))), + NodeMatches(AllOf(NameMatches(expected_queue_contents_[1].ToString()), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + UintIs("upload_attempts", 1u), + })))), }))), AllOf(NodeMatches(NameMatches("program_5")), - ChildrenMatch(ElementsAre(AllOf( - NodeMatches(AllOf( - PropertyList(ElementsAre(StringIs("creation_time", Not(IsEmpty())))))), - ChildrenMatch(ElementsAre( - NodeMatches(AllOf(NameMatches("crash_server"), - PropertyList(UnorderedElementsAreArray({ - StringIs("creation_time", Not(IsEmpty())), - StringIs("id", kStubServerReportId), - })))))))))), + ChildrenMatch( + ElementsAre(AllOf(NodeMatches(AllOf(PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("final_state", "uploaded"), + UintIs("upload_attempts", 1u), + })))), + ChildrenMatch(ElementsAre(NodeMatches( + AllOf(NameMatches("crash_server"), + PropertyList(UnorderedElementsAreArray({ + StringIs("creation_time", Not(IsEmpty())), + StringIs("id", kStubServerReportId), + })))))))))), })))))); }