Skip to content

Conversation

@JDevlieghere
Copy link
Member

Add a simple unit test for the ContinueRequestHandler that checks that it returns an error when the (dummy process) is not stopped.

Add a simple unit test for the ContinueRequestHandler that checks that
it returns an error when the (dummy process) is not stopped.
@llvmbot
Copy link
Member

llvmbot commented May 19, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Add a simple unit test for the ContinueRequestHandler that checks that it returns an error when the (dummy process) is not stopped.


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

2 Files Affected:

  • (modified) lldb/unittests/DAP/CMakeLists.txt (+1)
  • (added) lldb/unittests/DAP/Handler/ContinueTest.cpp (+45)
diff --git a/lldb/unittests/DAP/CMakeLists.txt b/lldb/unittests/DAP/CMakeLists.txt
index cd421401f167b..d9dc4fd454a59 100644
--- a/lldb/unittests/DAP/CMakeLists.txt
+++ b/lldb/unittests/DAP/CMakeLists.txt
@@ -2,6 +2,7 @@ add_lldb_unittest(DAPTests
   DAPTest.cpp
   FifoFilesTest.cpp
   Handler/DisconnectTest.cpp
+  Handler/ContinueTest.cpp
   JSONUtilsTest.cpp
   LLDBUtilsTest.cpp
   ProtocolTypesTest.cpp
diff --git a/lldb/unittests/DAP/Handler/ContinueTest.cpp b/lldb/unittests/DAP/Handler/ContinueTest.cpp
new file mode 100644
index 0000000000000..a67a1a25492c3
--- /dev/null
+++ b/lldb/unittests/DAP/Handler/ContinueTest.cpp
@@ -0,0 +1,45 @@
+//===-- ContinueTest.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 "DAP.h"
+#include "Handler/RequestHandler.h"
+#include "Protocol/ProtocolRequests.h"
+#include "TestBase.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_dap;
+using namespace lldb_dap_tests;
+using namespace lldb_dap::protocol;
+
+class ContinueRequestHandlerTest : public DAPTestBase {};
+
+TEST_F(ContinueRequestHandlerTest, NotStopped) {
+  SBTarget target;
+  dap->debugger.SetSelectedTarget(target);
+
+  ContinueRequestHandler handler(*dap);
+
+  ContinueArguments args_all_threads;
+  args_all_threads.singleThread = false;
+  args_all_threads.threadId = 0;
+
+  auto result_all_threads = handler.Run(args_all_threads);
+  EXPECT_THAT_EXPECTED(result_all_threads,
+                       llvm::FailedWithMessage("not stopped"));
+
+  ContinueArguments args_single_thread;
+  args_single_thread.singleThread = true;
+  args_single_thread.threadId = 1234;
+
+  auto result_single_thread = handler.Run(args_single_thread);
+  EXPECT_THAT_EXPECTED(result_single_thread,
+                       llvm::FailedWithMessage("not stopped"));
+}

Copy link
Contributor

@ashgti ashgti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Is there anyway we can make a mock or something for the SBProcess? That would be helpful for these tests, but I am not sure if we have that somewhere in the lldb testing utilities or not already.

@JDevlieghere
Copy link
Member Author

Is there anyway we can make a mock or something for the SBProcess? That would be helpful for these tests, but I am not sure if we have that somewhere in the lldb testing utilities or not already.

We have a few unit tests that inherit from lldb_private::Process for mocking, but I've never looked into mocking SBProcess. Maybe that's not too hard with GMock?

@JDevlieghere JDevlieghere merged commit d8665bb into llvm:main May 19, 2025
12 checks passed
@JDevlieghere JDevlieghere deleted the lldb-dap-continue-test branch May 19, 2025 18:39
@labath
Copy link
Collaborator

labath commented May 20, 2025

gmock doesn't let you do anything that wouldn't be possible without it. For this to work, the code which interacts with the mock needs to be ready to accept something other than the real object.... somehow. That usually means making the mocked object polymorphic, but that is a nonstarter here due to the interface stability. Other options I see are:

  • write a wrapper around SBProcess and then mock that
  • make everything a template so that it can be instantiated with a different (mock) implementation
  • use the fact that liblldb is a (shared) library to write an "alternative implementation" of that library

Each of those options comes with a fairly high cost (both upfront and in ongoing maintenance)...

@ashgti
Copy link
Contributor

ashgti commented May 20, 2025

While not the same as a mock, I was able to use a CoreFile to allow us to make a SBTarget and SBProcess for the unit tests, see #140738

@jimingham
Copy link
Collaborator

You can also use ScriptedProcesses for testing. They would allow wrapping a real process and changing the answers it hands out in some way useful to you, or just making up the process state out of whole cloth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants