[Lldb-commits] [lldb] 12fa4b1 - [lldb] Make sure that a `Progress` "completed" update is always reported at destruction (#102097)

[via lldb-commits]

Author: royitaqi
Date: 2024-08-07T07:58:34-07:00
New Revision: 12fa4b17dcededbd14abfc3ae72f1b798349e847

URL: https://github.com/llvm/llvm-project/commit/12fa4b17dcededbd14abfc3ae72f1b798349e847
DIFF: https://github.com/llvm/llvm-project/commit/12fa4b17dcededbd14abfc3ae72f1b798349e847.diff

LOG: [lldb] Make sure that a `Progress` "completed" update is always reported at destruction (#102097)

Make all `Progress` destructions to cause `progressEnd` events,
regardless of the value of `m_completed` before the destruction.

Currently, a `Progress` instance with `m_completed != 0 && m_complete !=
m_total` will cause a `progressUpdate` event (not `progressEnd`) at
destruction and. This contradicts with the classdoc: "a progress completed
update is reported even if the user doesn't explicitly cause one to be sent."

Added: 
    

Modified: 
    lldb/source/Core/Progress.cpp
    lldb/unittests/Core/ProgressReportTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 1a779e2ddf924d..e0ba1a63c508e5 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -45,8 +45,7 @@ Progress::~Progress() {
   // Make sure to always report progress completed when this object is
   // destructed so it indicates the progress dialog/activity should go away.
   std::lock_guard<std::mutex> guard(m_mutex);
-  if (!m_completed)
-    m_completed = m_total;
+  m_completed = m_total;
   ReportProgress();
 
   // Report to the ProgressManager if that subsystem is enabled.

diff  --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index 141244feb1f08a..0149b1de77add7 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -133,6 +133,81 @@ TEST_F(ProgressReportTest, TestReportCreation) {
   EXPECT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
 }
 
+TEST_F(ProgressReportTest, TestReportDestructionWithPartialProgress) {
+  ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgress);
+  EventSP event_sp;
+  const ProgressEventData *data;
+
+  // Create a finite progress report and only increment to a non-completed
+  // state before destruction.
+  {
+    Progress progress("Finite progress", "Report 1", 100);
+    progress.Increment(3);
+  }
+
+  // Verify that the progress in the events are:
+  // 1. At construction: 0 out of 100
+  // 2. At increment: 3 out of 100
+  // 3. At destruction: 100 out of 100
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  EXPECT_EQ(data->GetDetails(), "Report 1");
+  EXPECT_TRUE(data->IsFinite());
+  EXPECT_EQ(data->GetCompleted(), (uint64_t)0);
+  EXPECT_EQ(data->GetTotal(), (uint64_t)100);
+  EXPECT_EQ(data->GetMessage(), "Finite progress: Report 1");
+
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  EXPECT_EQ(data->GetDetails(), "Report 1");
+  EXPECT_TRUE(data->IsFinite());
+  EXPECT_EQ(data->GetCompleted(), (uint64_t)3);
+  EXPECT_EQ(data->GetTotal(), (uint64_t)100);
+  EXPECT_EQ(data->GetMessage(), "Finite progress: Report 1");
+
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  EXPECT_EQ(data->GetDetails(), "Report 1");
+  EXPECT_TRUE(data->IsFinite());
+  EXPECT_EQ(data->GetCompleted(), (uint64_t)100);
+  EXPECT_EQ(data->GetTotal(), (uint64_t)100);
+  EXPECT_EQ(data->GetMessage(), "Finite progress: Report 1");
+
+  // Create an infinite progress report and increment by some amount.
+  {
+    Progress progress("Infinite progress", "Report 2");
+    progress.Increment(3);
+  }
+
+  // Verify that the progress in the events are:
+  // 1. At construction: 0
+  // 2. At increment: 3
+  // 3. At destruction: Progress::kNonDeterministicTotal
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  EXPECT_EQ(data->GetDetails(), "Report 2");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_EQ(data->GetCompleted(), (uint64_t)0);
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Infinite progress: Report 2");
+
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  EXPECT_EQ(data->GetDetails(), "Report 2");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_EQ(data->GetCompleted(), (uint64_t)3);
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Infinite progress: Report 2");
+
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  EXPECT_EQ(data->GetDetails(), "Report 2");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_EQ(data->GetCompleted(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Infinite progress: Report 2");
+}
+
 TEST_F(ProgressReportTest, TestProgressManager) {
   ListenerSP listener_sp =
       CreateListenerFor(lldb::eBroadcastBitProgressCategory);