[Lldb-commits] [lldb] [lldb] Do some gardening in ProgressReportTest (NFC) (PR #84278)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

 - Factor our common setup code.
 - Split the ProgressManager test into separate tests as they test separate things.
 - Fix usage of EXPECT (which continues on failure) and ASSERT (which halts on failure). We must use the latter when calling GetEvent as otherwise we'll try to dereference a null EventSP.

---
Full diff: https://github.com/llvm/llvm-project/pull/84278.diff


1 Files Affected:

- (modified) lldb/unittests/Core/ProgressReportTest.cpp (+96-103) 


``````````diff
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index 98cbc475ce2835..1779d1e613b6f2 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -22,9 +22,29 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static std::chrono::milliseconds TIMEOUT(100);
+
 class ProgressReportTest : public ::testing::Test {
-  SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+public:
+  ListenerSP CreateListenerFor(uint32_t bit) {
+    // Set up the debugger, make sure that was done properly.
+    ArchSpec arch("x86_64-apple-macosx-");
+    Platform::SetHostPlatform(
+        PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+    m_debugger_sp = Debugger::CreateInstance();
+
+    // Get the debugger's broadcaster.
+    Broadcaster &broadcaster = m_debugger_sp->GetBroadcaster();
+
+    // Create a listener, make sure it can receive events and that it's
+    // listening to the correct broadcast bit.
+    m_listener_sp = Listener::MakeListener("progress-listener");
+    m_listener_sp->StartListeningForEvents(&broadcaster, bit);
+    return m_listener_sp;
+  }
 
+protected:
   // The debugger's initialization function can't be called with no arguments
   // so calling it using SubsystemRAII will cause the test build to fail as
   // SubsystemRAII will call Initialize with no arguments. As such we set it up
@@ -33,30 +53,14 @@ class ProgressReportTest : public ::testing::Test {
     std::call_once(TestUtilities::g_debugger_initialize_flag,
                    []() { Debugger::Initialize(nullptr); });
   };
+
+  DebuggerSP m_debugger_sp;
+  ListenerSP m_listener_sp;
+  SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
 };
 
 TEST_F(ProgressReportTest, TestReportCreation) {
-  std::chrono::milliseconds timeout(100);
-
-  // Set up the debugger, make sure that was done properly.
-  ArchSpec arch("x86_64-apple-macosx-");
-  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
-
-  DebuggerSP debugger_sp = Debugger::CreateInstance();
-  ASSERT_TRUE(debugger_sp);
-
-  // Get the debugger's broadcaster.
-  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
-
-  // Create a listener, make sure it can receive events and that it's
-  // listening to the correct broadcast bit.
-  ListenerSP listener_sp = Listener::MakeListener("progress-listener");
-
-  listener_sp->StartListeningForEvents(&broadcaster,
-                                       Debugger::eBroadcastBitProgress);
-  EXPECT_TRUE(
-      broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress));
-
+  ListenerSP listener_sp = CreateListenerFor(Debugger::eBroadcastBitProgress);
   EventSP event_sp;
   const ProgressEventData *data;
 
@@ -73,82 +77,64 @@ TEST_F(ProgressReportTest, TestReportCreation) {
   // in this order:
   // Starting progress: 1, 2, 3
   // Ending progress: 3, 2, 1
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetDetails(), "Starting report 1");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_FALSE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
+  EXPECT_EQ(data->GetDetails(), "Starting report 1");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_FALSE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
 
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetDetails(), "Starting report 2");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_FALSE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
+  EXPECT_EQ(data->GetDetails(), "Starting report 2");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_FALSE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
 
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
-  ASSERT_EQ(data->GetDetails(), "Starting report 3");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_FALSE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Progress report 3: Starting report 3");
+
+  EXPECT_EQ(data->GetDetails(), "Starting report 3");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_FALSE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Progress report 3: Starting report 3");
 
   // Progress report objects should be destroyed at this point so
   // get each report from the queue and check that they've been
   // destroyed in reverse order.
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetTitle(), "Progress report 3");
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_EQ(data->GetMessage(), "Progress report 3: Starting report 3");
+  EXPECT_EQ(data->GetTitle(), "Progress report 3");
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_EQ(data->GetMessage(), "Progress report 3: Starting report 3");
 
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetTitle(), "Progress report 2");
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
+  EXPECT_EQ(data->GetTitle(), "Progress report 2");
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
 
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetTitle(), "Progress report 1");
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
+  EXPECT_EQ(data->GetTitle(), "Progress report 1");
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
 }
 
 TEST_F(ProgressReportTest, TestProgressManager) {
-  std::chrono::milliseconds timeout(100);
-
-  // Set up the debugger, make sure that was done properly.
-  ArchSpec arch("x86_64-apple-macosx-");
-  Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
-
-  DebuggerSP debugger_sp = Debugger::CreateInstance();
-  ASSERT_TRUE(debugger_sp);
-
-  // Get the debugger's broadcaster.
-  Broadcaster &broadcaster = debugger_sp->GetBroadcaster();
-
-  // Create a listener, make sure it can receive events and that it's
-  // listening to the correct broadcast bit.
-  ListenerSP listener_sp = Listener::MakeListener("progress-category-listener");
-
-  listener_sp->StartListeningForEvents(&broadcaster,
-                                       Debugger::eBroadcastBitProgressCategory);
-  EXPECT_TRUE(broadcaster.EventTypeHasListeners(
-      Debugger::eBroadcastBitProgressCategory));
-
+  ListenerSP listener_sp =
+      CreateListenerFor(Debugger::eBroadcastBitProgressCategory);
   EventSP event_sp;
   const ProgressEventData *data;
 
@@ -160,28 +146,35 @@ TEST_F(ProgressReportTest, TestProgressManager) {
     Progress progress1("Progress report 1", "Starting report 1");
     Progress progress2("Progress report 1", "Starting report 2");
     Progress progress3("Progress report 1", "Starting report 3");
-    EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
-    EXPECT_FALSE(listener_sp->GetEvent(event_sp, timeout));
+    ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+    ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT));
   }
 
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetDetails(), "");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Progress report 1");
+  EXPECT_EQ(data->GetDetails(), "");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Progress report 1");
 
   // Pop another event from the queue, this should be the event for the final
   // report for this category.
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
-
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
-  ASSERT_EQ(data->GetDetails(), "");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Progress report 1");
+
+  EXPECT_EQ(data->GetDetails(), "");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Progress report 1");
+}
+
+TEST_F(ProgressReportTest, TestOverlappingEvents) {
+  ListenerSP listener_sp =
+      CreateListenerFor(Debugger::eBroadcastBitProgressCategory);
+  EventSP event_sp;
+  const ProgressEventData *data;
 
   // Create two progress reports of the same category that overlap with each
   // other. Here we want to ensure that the ID broadcasted for the initial and
@@ -192,28 +185,28 @@ TEST_F(ProgressReportTest, TestProgressManager) {
       std::make_unique<Progress>("Overlapping report 1", "Starting report 2");
   overlap_progress1.reset();
 
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
   // Get the ID used in the first report for this category.
   uint64_t expected_progress_id = data->GetID();
 
-  ASSERT_EQ(data->GetDetails(), "");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Overlapping report 1");
+  EXPECT_EQ(data->GetDetails(), "");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Overlapping report 1");
 
   overlap_progress2.reset();
 
-  EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
   data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
 
-  ASSERT_EQ(data->GetDetails(), "");
-  ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
-  ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
-  ASSERT_EQ(data->GetMessage(), "Overlapping report 1");
+  EXPECT_EQ(data->GetDetails(), "");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Overlapping report 1");
   // The progress ID for the final report should be the same as that for the
   // initial report.
-  ASSERT_EQ(data->GetID(), expected_progress_id);
+  EXPECT_EQ(data->GetID(), expected_progress_id);
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/84278