[Lldb-commits] [PATCH] D71234: [lldb/Lua] Implement a Simple Lua Script Interpreter Prototype

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lldb/unittests/ScriptInterpreter/Lua/CMakeLists.txt:14-15
+
+target_include_directories(ScriptInterpreterLuaTests PRIVATE ${LUA_INCLUDE_DIR})
+target_link_libraries(ScriptInterpreterLuaTests PRIVATE ${LUA_LIBRARIES})
----------------
I think you wouldn't need these if the other CMakeLists file would use PUBLIC keyword for headers, and INTERFACE for libraries.


================
Comment at: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp:53
+TEST_F(ScriptInterpreterTest, ExecuteOneLine) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
----------------
JDevlieghere wrote:
> labath wrote:
> > I'm not actually opposed to this (and it's nice to know that this could work if we need it), but why do this as a unit test? It seems like this could be tested equally well with a `script` command through the lldb driver. I was imagining the unit tests would be most useful for the lower level functionality -- roughly corresponding to the `Lua` class. Right now that class is pretty simple and doesn't really need a unit test, but I expect it will become more complex over time...
> Yes, I just wanted to show that it's possible :-) I've also included a test for the Lua class. 
Ok, that's cool.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71234/new/

https://reviews.llvm.org/D71234