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

[Pavel Labath via Phabricator via lldb-commits]

labath 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)
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96202