ioeric added inline comments.

================
Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;
----------------
sammccall wrote:
> 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...
> How would you suggest testing the randomization :-)
Not suggesting testing the behavior; just test that it works really. I guess a 
single file would also do it.

> The problem with a ClangdServer test is that it doesn't know anything about 
> autoindex.
Ah, I see. That's a bit unfortunate. ClangdServer seems to be a better place 
for the auto index. I wonder if we could move AutoIndex into ClangdServer? I'm 
not very familiar with CDB APIs, but intuitively this seems doable if we tweak 
the interface of CDB a bit e.g. inject the callback into 
`GlobalCompilationDatabase` without going through `CompilationDB`? Not sure if 
how well this would play with the CDB design though.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to