[Lldb-commits] [lldb] [lldb][progress][NFC] Add unit test for progress reports (PR #79533)
Chelsea Cassanova via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 31 11:35:59 PST 2024
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/79533
>From 10343b6cdad410e09546dd5a98e29d272300ed2e Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Thu, 25 Jan 2024 16:40:42 -0800
Subject: [PATCH 1/4] [lldb][progress][NFC] Add unit test for progress reports
This test is being added as a way to check the behaviour of how progress
events are broadcasted when reports are started and ended with the
current implementation of progress reports. Here we're mainly checking
and ensuring that the current behaviour is that progress events are
broadcasted individually and placed in the event queue in order of
their creation.
---
lldb/unittests/Core/CMakeLists.txt | 1 +
lldb/unittests/Core/ProgressReportTest.cpp | 105 +++++++++++++++++++++
2 files changed, 106 insertions(+)
create mode 100644 lldb/unittests/Core/ProgressReportTest.cpp
diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt
index b3cddd150635b..d40c357e3f463 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -7,6 +7,7 @@ add_lldb_unittest(LLDBCoreTests
FormatEntityTest.cpp
MangledTest.cpp
ModuleSpecTest.cpp
+ ProgressReportTest.cpp
RichManglingContextTest.cpp
SourceLocationSpecTest.cpp
SourceManagerTest.cpp
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
new file mode 100644
index 0000000000000..bdc168c9e077d
--- /dev/null
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -0,0 +1,105 @@
+#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Listener.h"
+#include "gtest/gtest.h"
+#include <thread>
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+class ProgressReportTest : public ::testing::Test {
+public:
+ void SetUp() override {
+ FileSystem::Initialize();
+ HostInfo::Initialize();
+ PlatformMacOSX::Initialize();
+ Debugger::Initialize(nullptr);
+ }
+ void TearDown() override {
+ Debugger::Terminate();
+ PlatformMacOSX::Terminate();
+ HostInfo::Terminate();
+ FileSystem::Terminate();
+ }
+};
+} // namespace
+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));
+
+ EventSP event_sp;
+ const ProgressEventData *data;
+
+ // Scope this for RAII on the progress objects
+ // Create progress reports and check that their respective events for having
+ // started are broadcasted
+ {
+ Progress progress1("Progress report 1", "Starting report 1");
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ ASSERT_EQ(data->GetDetails(), "Starting report 1");
+
+ Progress progress2("Progress report 2", "Starting report 2");
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ ASSERT_EQ(data->GetDetails(), "Starting report 2");
+
+ Progress progress3("Progress report 3", "Starting report 3");
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ ASSERT_TRUE(event_sp);
+
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ ASSERT_EQ(data->GetDetails(), "Starting report 3");
+
+ std::this_thread::sleep_for(timeout);
+ }
+
+ // 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
+ std::this_thread::sleep_for(timeout);
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+
+ ASSERT_EQ(data->GetTitle(), "Progress report 3");
+ ASSERT_TRUE(data->GetCompleted());
+
+ std::this_thread::sleep_for(timeout);
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+
+ ASSERT_EQ(data->GetTitle(), "Progress report 2");
+ ASSERT_TRUE(data->GetCompleted());
+
+ std::this_thread::sleep_for(timeout);
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+
+ ASSERT_EQ(data->GetTitle(), "Progress report 1");
+ ASSERT_TRUE(data->GetCompleted());
+}
>From 9843ff790aef57246ba61bbb4ce4e74957a9b05e Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Fri, 26 Jan 2024 11:08:11 -0800
Subject: [PATCH 2/4] Simplify set up, remove timeouts, add license
Uses `SubsystemRAII` to initialize the necessary platform and host
information. Since the debugger cannot be initialized with no
arguments (which is how `SubsystemRAII` will initialize everything), the
debugger will be initialized the usual way. Also removes the thread
timeouts and adds the LLVM license header.
---
lldb/unittests/Core/ProgressReportTest.cpp | 38 ++++++++++------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index bdc168c9e077d..7d66f45b19689 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -1,5 +1,14 @@
+//===-- ProgressReportTest.cpp --------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Progress.h"
#include "lldb/Host/FileSystem.h"
@@ -11,23 +20,17 @@
using namespace lldb;
using namespace lldb_private;
-namespace {
class ProgressReportTest : public ::testing::Test {
-public:
- void SetUp() override {
- FileSystem::Initialize();
- HostInfo::Initialize();
- PlatformMacOSX::Initialize();
- Debugger::Initialize(nullptr);
- }
- void TearDown() override {
- Debugger::Terminate();
- PlatformMacOSX::Terminate();
- HostInfo::Terminate();
- FileSystem::Terminate();
- }
+ SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+
+ // 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
+ // here the usual way.
+ void SetUp() override { Debugger::Initialize(nullptr); }
+ void TearDown() override { Debugger::Terminate(); }
};
-} // namespace
+
TEST_F(ProgressReportTest, TestReportCreation) {
std::chrono::milliseconds timeout(100);
@@ -75,28 +78,23 @@ TEST_F(ProgressReportTest, TestReportCreation) {
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
ASSERT_EQ(data->GetDetails(), "Starting report 3");
-
- std::this_thread::sleep_for(timeout);
}
// 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
- std::this_thread::sleep_for(timeout);
EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
ASSERT_EQ(data->GetTitle(), "Progress report 3");
ASSERT_TRUE(data->GetCompleted());
- std::this_thread::sleep_for(timeout);
EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
ASSERT_EQ(data->GetTitle(), "Progress report 2");
ASSERT_TRUE(data->GetCompleted());
- std::this_thread::sleep_for(timeout);
EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
>From bb719546e2e568427b7324183290a3b58fb0f64e Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Fri, 26 Jan 2024 15:33:38 -0800
Subject: [PATCH 3/4] Check all events outside of scope, use correct check for
total
Checks that the progress events were received in order of report
creation and deletion outside of the scope that the events were created
in, also checks for more information from the progress event data.
`IsFinite()` from DebuggerEvents would return true if the total was not
UINT64_MAX. Since we no longer use this to specify that an event has no
total this has been changed to check that the total is not 1.
---
lldb/unittests/Core/ProgressReportTest.cpp | 66 ++++++++++++++--------
1 file changed, 44 insertions(+), 22 deletions(-)
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index 7d66f45b19689..f62aeda341ace 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -21,18 +21,19 @@ using namespace lldb;
using namespace lldb_private;
class ProgressReportTest : public ::testing::Test {
- SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
-
- // 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
- // here the usual way.
- void SetUp() override { Debugger::Initialize(nullptr); }
- void TearDown() override { Debugger::Terminate(); }
+ SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+
+ // 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
+ // here the usual way.
+ void SetUp() override { Debugger::Initialize(nullptr); }
+ void TearDown() override { Debugger::Terminate(); }
};
TEST_F(ProgressReportTest, TestReportCreation) {
std::chrono::milliseconds timeout(100);
+ const unsigned long long NO_TOTAL = 1;
// Set up the debugger, make sure that was done properly
ArchSpec arch("x86_64-apple-macosx-");
@@ -51,7 +52,7 @@ TEST_F(ProgressReportTest, TestReportCreation) {
listener_sp->StartListeningForEvents(&broadcaster,
Debugger::eBroadcastBitProgress);
EXPECT_TRUE(
- broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress));
+ broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress));
EventSP event_sp;
const ProgressEventData *data;
@@ -61,24 +62,39 @@ TEST_F(ProgressReportTest, TestReportCreation) {
// started are broadcasted
{
Progress progress1("Progress report 1", "Starting report 1");
- EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ Progress progress2("Progress report 2", "Starting report 2");
+ Progress progress3("Progress report 3", "Starting report 3");
+ }
- data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
- ASSERT_EQ(data->GetDetails(), "Starting report 1");
+ // Start popping events from the queue, they should have been recevied
+ // in this order:
+ // Starting progress: 1, 2, 3
+ // Ending progress: 3, 2, 1
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
- Progress progress2("Progress report 2", "Starting report 2");
- EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ ASSERT_EQ(data->GetDetails(), "Starting report 1");
+ ASSERT_FALSE(data->IsFinite());
+ ASSERT_FALSE(data->GetCompleted());
+ ASSERT_EQ(data->GetTotal(), NO_TOTAL);
+ ASSERT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
- data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
- ASSERT_EQ(data->GetDetails(), "Starting report 2");
+ EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
- Progress progress3("Progress report 3", "Starting report 3");
- EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
- ASSERT_TRUE(event_sp);
+ ASSERT_EQ(data->GetDetails(), "Starting report 2");
+ ASSERT_FALSE(data->IsFinite());
+ ASSERT_FALSE(data->GetCompleted());
+ ASSERT_EQ(data->GetTotal(), NO_TOTAL);
+ ASSERT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
- data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
- ASSERT_EQ(data->GetDetails(), "Starting report 3");
- }
+ EXPECT_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(), NO_TOTAL);
+ ASSERT_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
@@ -88,16 +104,22 @@ TEST_F(ProgressReportTest, TestReportCreation) {
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_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_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");
}
>From f3f2d84eda3993e29fb626393279db9ead3b8cfd Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Tue, 30 Jan 2024 12:43:36 -0800
Subject: [PATCH 4/4] Use static var to check progress total
Use the changes implemented in
https://github.com/llvm/llvm-project/pull/79912 to use a static variable
from `Progress` to ensure that the report was created with a
non-deterministic total.
---
lldb/unittests/Core/ProgressReportTest.cpp | 37 +++++++++++-----------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index f62aeda341ace..77222560d5702 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -21,45 +21,44 @@ using namespace lldb;
using namespace lldb_private;
class ProgressReportTest : public ::testing::Test {
- SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
-
- // 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
- // here the usual way.
- void SetUp() override { Debugger::Initialize(nullptr); }
- void TearDown() override { Debugger::Terminate(); }
+ SubsystemRAII<FileSystem, HostInfo, PlatformMacOSX> subsystems;
+
+ // 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
+ // here the usual way.
+ void SetUp() override { Debugger::Initialize(nullptr); }
+ void TearDown() override { Debugger::Terminate(); }
};
TEST_F(ProgressReportTest, TestReportCreation) {
std::chrono::milliseconds timeout(100);
- const unsigned long long NO_TOTAL = 1;
- // Set up the debugger, make sure that was done properly
+ // 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
+ // 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
+ // 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));
+ broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress));
EventSP event_sp;
const ProgressEventData *data;
- // Scope this for RAII on the progress objects
+ // Scope this for RAII on the progress objects.
// Create progress reports and check that their respective events for having
- // started are broadcasted
+ // started and ended are broadcasted.
{
Progress progress1("Progress report 1", "Starting report 1");
Progress progress2("Progress report 2", "Starting report 2");
@@ -76,7 +75,7 @@ TEST_F(ProgressReportTest, TestReportCreation) {
ASSERT_EQ(data->GetDetails(), "Starting report 1");
ASSERT_FALSE(data->IsFinite());
ASSERT_FALSE(data->GetCompleted());
- ASSERT_EQ(data->GetTotal(), NO_TOTAL);
+ ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
ASSERT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
@@ -85,7 +84,7 @@ TEST_F(ProgressReportTest, TestReportCreation) {
ASSERT_EQ(data->GetDetails(), "Starting report 2");
ASSERT_FALSE(data->IsFinite());
ASSERT_FALSE(data->GetCompleted());
- ASSERT_EQ(data->GetTotal(), NO_TOTAL);
+ ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
ASSERT_EQ(data->GetMessage(), "Progress report 2: Starting report 2");
EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
@@ -93,12 +92,12 @@ TEST_F(ProgressReportTest, TestReportCreation) {
ASSERT_EQ(data->GetDetails(), "Starting report 3");
ASSERT_FALSE(data->IsFinite());
ASSERT_FALSE(data->GetCompleted());
- ASSERT_EQ(data->GetTotal(), NO_TOTAL);
+ ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
ASSERT_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
+ // destroyed in reverse order.
EXPECT_TRUE(listener_sp->GetEvent(event_sp, timeout));
data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
More information about the lldb-commits
mailing list