[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