[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 12 08:40:02 PDT 2018


sammccall added inline comments.


================
Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;
----------------
ioeric wrote:
> sammccall wrote:
> > ioeric wrote:
> > > Also add a test for `enqueueAll` with multiple TUs ?
> > Is it important to call `enqueueAll` specifically vs `enqueue` multiple times?
> > 
> > We don't have a good test fixture for a compilation database, and `enqueueAll` is trivial...
> I think the randomization code worths a test. 
> 
> How about adding a test in ClangdServer with the auto index enabled? I think we'd also want coverage in ClangdServer anyway.
How would you suggest testing the randomization :-)

The problem with a ClangdServer test is that it doesn't know anything about autoindex.
AutoIndex lives in ClangdLSPServer, because that's where the compilation database lives (because people keep cramming LSP extensions in to manipulate it, and I haven't been able to remove them).
Nobody has worked out how to test ClangdLSPServer yet. It's a worthy project, but...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032





More information about the cfe-commits mailing list