[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

Fri Sep 1 11:35:52 PDT 2023

hiraditya added a comment.

In D158566#4633847 <https://reviews.llvm.org/D158566#4633847>, @kadircet wrote:

> In D158566#4616417 <https://reviews.llvm.org/D158566#4616417>, @ilya-biryukov wrote:
>
>> Open question: I also feel like the best option here is to fix the tests, but I'm not sure how hard that would be. @sammccall any thoughts?
>> I suspect the particular tests are flaky is because they rely on timeouts, not sure it's easy to disentangle them. Therefore, some workaround seems reasonable
>
> FWIW, i've put some details in https://github.com/llvm/llvm-project/issues/64964#issuecomment-1702249740 and we had some previous discussions but unfortunately these tests have timeouts as a "poor-mans-deadlock-detection". i don't think we can get rid of the timeouts, without sacrificing that detection.
> I can't remember how misleading buildbot outputs were, when deadlocks happened, before we introduced timeouts though. So one alternative is let the buildbots hang instead.
>
> ---
>
> Regarding the approach in this patch, I don't feel strongly about it but I don't think it's a good idea to let people build clangd, without testing it on environments they care about. They might suppress legitimate issues. (there's also some value though, e.g. maybe they already performed testing before, and don't want to run tests again, but in such a scenario we've non-check equivalents of targets to only run builds).

This option only allows more flexibility to the developers and by default it is on so anyone who doesn't care about switching off the test on constrained systems can continue to do so. Do you have specific concerns on why this will prevent developers from testing?

> Are you building clangd deliberately or is it just being pulled in via check-clang-tools? If you don't want to ship clangd with your toolchain at all, I think it's better to set `CLANG_ENABLE_CLANGD` to `OFF`.

We want to build clangd and it gets tested on a regular basis but the timeout happens on mac builds that are overloaded at times. I don't want to disable the build of clangd just because 1 test is failing.

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

https://reviews.llvm.org/D158566