[PATCH] D103685: [clangd] Drop TestTUs dependency on gtest

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 7 02:38:28 PDT 2021


sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:122
   if (!AST->getDiagnostics()) {
-    ADD_FAILURE() << "TestTU should always build an AST with a fresh Preamble"
-                  << Code;
+    llvm::errs() << "TestTU should always build an AST with a fresh Preamble"
+                 << Code;
----------------
kadircet wrote:
> sammccall wrote:
> > this check is now a log message that could still let the test pass
> > Make it an assert instead?
> > 
> > (I think assert without any early return is appropriate per normal llvm style)
> aborting instead of just `assert(false)`, since the rest of the code assumes AST->getDiagnostics has a value.
This is actually a can't-happen condition even if the test is messed up, and we normally assert on those before dereferencing a pointer. But up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103685/new/

https://reviews.llvm.org/D103685



More information about the cfe-commits mailing list