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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
