[Lldb-commits] [lldb] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize in the u… (PR #82096)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Fri Feb 16 22:54:39 PST 2024


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

…nittests

The unit tests only test the Python objects and don't actually use anything from the LLDB module. On the one hand that means that everything we do in ScriptInterpreterPythonImpl::Initialize is overkill, but on the other hand it means that we get test coverage for the initialization. In other words I'm not sure if I think this is the right direction.

While looking at #70453 I wanted to prove that the two test failures were unrelated to the call to PyImport_AppendInittab by doing the initialization manually.

>From 5e3abb8d28def3c8a4d1d2fac1ef74a6969fe7b7 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 16 Feb 2024 22:51:08 -0800
Subject: [PATCH] [lldb] Don't rely on ScriptInterpreterPythonImpl::Initialize
 in the unittests

The unit tests only test the Python objects and don't actually use
anything from the LLDB module. On the one hand that means that
everything we do in ScriptInterpreterPythonImpl::Initialize is overkill,
but on the other hand it means that we get test coverage for the
initialization. In other words I'm not sure if I think this is the right
direction.

While looking at #70453 I wanted to prove that the two test failures
were unrelated to the call to PyImport_AppendInittab by doing the
initialization manually.
---
 .../Python/PythonTestSuite.cpp                | 26 ++++---------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
index 5f0cc4c23db7b2..23162436d42c94 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -9,43 +9,27 @@
 #include "gtest/gtest.h"
 
 #include "Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h"
 #include "Plugins/ScriptInterpreter/Python/lldb-python.h"
 
-#include "lldb/Host/FileSystem.h"
-#include "lldb/Host/HostInfo.h"
-
 #include "PythonTestSuite.h"
 #include <optional>
 
-using namespace lldb_private;
-class TestScriptInterpreterPython : public ScriptInterpreterPythonImpl {
-public:
-  using ScriptInterpreterPythonImpl::Initialize;
-};
-
 void PythonTestSuite::SetUp() {
-  FileSystem::Initialize();
-  HostInfoBase::Initialize();
-  // ScriptInterpreterPython::Initialize() depends on HostInfo being
-  // initializedso it can compute the python directory etc.
-  TestScriptInterpreterPython::Initialize();
-
   // Although we don't care about concurrency for the purposes of running
   // this test suite, Python requires the GIL to be locked even for
   // deallocating memory, which can happen when you call Py_DECREF or
   // Py_INCREF.  So acquire the GIL for the entire duration of this
   // test suite.
+  Py_InitializeEx(0);
   m_gil_state = PyGILState_Ensure();
+  PyRun_SimpleString("import sys");
 }
 
 void PythonTestSuite::TearDown() {
   PyGILState_Release(m_gil_state);
 
-  TestScriptInterpreterPython::Terminate();
-  HostInfoBase::Terminate();
-  FileSystem::Terminate();
+  // We could call Py_FinalizeEx here, but initializing and finalizing Python is
+  // pretty slow, so just keep Python initialized across tests.
 }
 
 // The following functions are the Pythonic implementations of the required
@@ -219,7 +203,7 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
 }
 
 bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
-    PyObject *implementor, lldb::DebuggerSP debugger, 
+    PyObject *implementor, lldb::DebuggerSP debugger,
     StructuredDataImpl &args_impl,
     lldb_private::CommandReturnObject &cmd_retobj,
     lldb::ExecutionContextRefSP exe_ctx_ref_sp) {



More information about the lldb-commits mailing list