[lldb] Do some gardening in ProgressReportTest (NFC) #84278
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
JDevlieghere
commented
Mar 7, 2024
JDevlieghere
commented
- 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.
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes
Full diff: https://github.com/llvm/llvm-project/pull/84278.diff 1 Files Affected:
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);
}
|
- 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.
JDevlieghere
force-pushed
the
progress-gardening
branch
from
a9eae84 to
8b85a2d
Compare
JDevlieghere
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 27, 2024
- 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. (cherry picked from commit 4586366)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.