[PATCH] D72066: [clangd] Assert that the testcases in LocateSymbol.All have no diagnostics
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 3 00:26:27 PST 2020
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
Thanks!
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:519
+ ASSERT_TRUE(AST.getDiagnostics().empty())
+ << AST.getDiagnostics().begin()->Message << Test;
+
----------------
nit: I would rather keep the loop and just use `ADD_FAILURE()` instead of `llvm::errs()`.
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:573
+ // parsing.
+ TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+
----------------
nit: this one can be dropped.
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:577
+ for (auto &Diag : AST.getDiagnostics()) {
+ ASSERT_TRUE(Diag.Severity <= DiagnosticsEngine::Warning)
+ << Diag.Message << Test;
----------------
nit: this looks like an overkill to me, in the end we are trying to test hover behavior, not diagnostics. I would drop the whole loop here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72066/new/
https://reviews.llvm.org/D72066
More information about the cfe-commits
mailing list