[PATCH] D44954: [clangd] Add "member" symbols to the index

Marc-Andre Laperle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 1 12:33:01 PDT 2018


malaperle added inline comments.


================
Comment at: unittests/clangd/CodeCompleteTests.cpp:741
 
+TEST(CompletionTest, Enums) {
+  EXPECT_THAT(completions(R"cpp(
----------------
ioeric wrote:
> malaperle wrote:
> > ioeric wrote:
> > > It's not clear to me what the following tests (`Enums`, `AnonymousNamespace`, `InMainFile`) are testing. Do they test code completion or  symbol collector? If these test symbol collection, they should be moved int SymbolCollectorTest.cpp
> > They are testing that code completion works as intended regardless of how symbol collector is implemented. It's similar to our previous discussion in D44882 about "black box testing". I can remove them but it seems odd to me to not have code completion level tests for all cases because we assume that it will behave a certain way at the symbol collection and querying levels.
> FWIW, I am not against those tests at all; increasing coverage is always a nice thing to do IMO. I just thought it would make more sense to add them in a separate patch if they are not related to changes in this patch; I found unrelated tests a bit confusing otherwise.
> I found unrelated tests a bit confusing otherwise.

Fair point!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954





More information about the cfe-commits mailing list