[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 2 02:05:13 PDT 2019


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTesting.cpp:74
+    return true;
+  llvm::consumeError(PrepareResult.takeError());
+  return false;
----------------
ilya-biryukov wrote:
> Maybe print the input in case of failure?
This is a matcher against the input, so gtest will always print it.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:44
+    // Snippet is an expression.
+    Expression,
+  };
----------------
ilya-biryukov wrote:
> I wonder whether we could use `Function` and get rid of 'expression' mode completely? WDYT?
> One can easily turn an expression into a statement by adding a semicolon.
We could, but I think it obfuscates intent a little bit, and doesn't really simplify anything (we're already paying for the function wrapping logic, adding a couple more strings is ~free)


================
Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:86
+  for (const auto &Case : expandCases(MarkedCode))                             \
+  EXPECT_THAT(Case, isAvailable())
+
----------------
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > NIT: add a level of indent
> NIT: fully-qualify in a macro `::clang::clangd::TweakTest::isAvailable()`
Indentation is clang-format's fault. Wrapped it in the do-while-0 to fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65525





More information about the cfe-commits mailing list