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

2024-03-06 Thread Jonas Devlieghere via lldb-commits 

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

 - 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.

>From a9eae8495f035d06cf810b78e5c7371cbe3c9bcc Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 6 Mar 2024 21:54:45 -0800
Subject: [PATCH] [lldb] Do some gardening in ProgressReportTest (NFC)

 - 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.
---
 lldb/unittests/Core/ProgressReportTest.cpp | 199 ++---
 1 file changed, 96 insertions(+), 103 deletions(-)

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 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 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(), Progr

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

2024-03-06 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

 - 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 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 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->GetEve

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

2024-03-07 Thread Chelsea Cassanova via lldb-commits 

https://github.com/chelcassanova approved this pull request.

LGTM!

https://github.com/llvm/llvm-project/pull/84278
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2024-03-07 Thread Jonas Devlieghere via lldb-commits 

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

>From 8b85a2dd23e29cd9eca807d70ffb95bc4b1dcc82 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 6 Mar 2024 21:54:45 -0800
Subject: [PATCH] [lldb] Do some gardening in ProgressReportTest (NFC)

 - 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.
---
 lldb/unittests/Core/ProgressReportTest.cpp | 199 ++---
 1 file changed, 96 insertions(+), 103 deletions(-)

diff --git a/lldb/unittests/Core/ProgressReportTest.cpp 
b/lldb/unittests/Core/ProgressReportTest.cpp
index e0253cbc4ec59b..1f993180fd8392 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 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 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_

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

2024-03-07 Thread Jonas Devlieghere via lldb-commits 

https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/84278
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits