[PATCH] D61187: [clangd] Move clangd tests to clangd directory. check-clangd is no longer part of check-clang-tools.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I'm confused by some of the decisions made in this change, see below.



================
Comment at: clang-tools-extra/trunk/clangd/test/lit.cfg.in:29
+  config.available_features.add('clangd-xpc-support')
+
----------------
trailing newline


================
Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:1
+ at LIT_SITE_CFG_IN_HEADER@
+# This is a shim to run the gtest unittests in ../unittests using lit.
----------------
Every other LLVM project puts the lit.cfg.in for the unit tests in foo/test/Unit/lit.cfg.in. Any reason why clangd is different?

Also, almost all LLVM projects call their lit.cfg lit.cfg.py and this file lit.cfg.py.in since it helps Windows. Can you rename the lit.cfg(.in) files you added to have .py in them please?

Also, why did you merge the site.cfg.py.in file with the non-generated non-site file? This too is different from all other LLVM projects.


================
Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:11
+# Point the dynamic loader at dynamic libraries in 'lib'.
+# XXX: it seems every project has a copy of this logic. Move it somewhere.
+import platform
----------------
Did you mean to commit this XXX?


================
Comment at: clang-tools-extra/trunk/clangd/unittests/lit.cfg.in:23
+
+
----------------
Lots of trailing newlines.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61187