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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
