[Lldb-commits] [PATCH] D96202: [lldb/test] Automatically find debug servers to test

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.


================
Comment at: lldb/test/API/CMakeLists.txt:115
-    set(LLDB_TEST_SERVER ${debugserver_path})
-    add_lldb_test_dependency(debugserver)
-  elseif(TARGET lldb-server)
----------------
labath wrote:
> teemperor wrote:
> > I believe this removed line is causing us to have missing dependencies. I just did a clean build of LLDB and it turns out that `debugserver` is no longer a dependency of `check-lldb`, which causes a bunch of weird test failures such as:
> > 
> > ```
> > ********************
> > FAIL: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests/StandardStartupTest.TestStopReplyContainsThreadPcs (2268 of 2282)
> > ******************** TEST 'lldb-unit :: tools/lldb-server/tests/./LLDBServerTests/StandardStartupTest.TestStopReplyContainsThreadPcs' FAILED ********************
> > Note: Google Test filter = StandardStartupTest.TestStopReplyContainsThreadPcs
> > [==========] Running 1 test from 1 test case.
> > [----------] Global test environment set-up.
> > [----------] 1 test from StandardStartupTest
> > [ RUN      ] StandardStartupTest.TestStopReplyContainsThreadPcs
> > /Users/teemperor/3llvm/llvm-project/lldb/unittests/tools/lldb-server/tests/TestBase.h:49: Failure
> > Value of: llvm::detail::TakeExpected(ClientOr)
> > Expected: succeeded
> >   Actual: failed  (executable doesn't exist: '/Users/teemperor/3llvm/rel/bin/debugserver')
> > [  FAILED  ] StandardStartupTest.TestStopReplyContainsThreadPcs (5 ms)
> > [----------] 1 test from StandardStartupTest (5 ms total)
> > 
> > [----------] Global test environment tear-down
> > [==========] 1 test from 1 test case ran. (5 ms total)
> > [  PASSED  ] 0 tests.
> > [  FAILED  ] 1 test, listed below:
> > [  FAILED  ] StandardStartupTest.TestStopReplyContainsThreadPcs
> > 
> >  1 FAILED TEST
> > ```
> > 
> > I haven't bisected yet what exactly did remove the `debugserver` dependency but I'm pretty sure it's this patch (due to being landed recently and me not finding any other place where we added this dependency)
> Thanks for letting me know, and sorry about the breakage. Your analysis sounds believable.
> I guess this should be replaced by something like
> ```
> if(TARGET debugserver)
>   add_lldb_test_dependency(debugserver)
> endif()
> if(TARGET lldb-server)
>   add_lldb_test_dependency(lldb-server)
> endif()
> ```
> 
> I can do that tomorrow.
I'm still in office for a while so I can do it now :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96202