[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

[Kar Epker via Phabricator via cfe-commits]

karepker marked 2 inline comments as done and an inline comment as not done.
karepker added inline comments.


================
Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27
+static bool isGoogletestTestMacro(StringRef MacroName) {
+  static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P",
+                                               "TYPED_TEST", "TYPED_TEST_P"};
----------------
MyDeveloperDay wrote:
> Is there value in making the list of macros and option?, I've worked with other unit testing packages (some inhouse) that use the same format as google test, but don't use TEST() itself
> 
> e.g.  (to name a few)
> 
> ```
> TEST_CASE(testclass, testname)
> BOOST_TEST(statement, floating_point_comparison_manipulation);
> BOOST_DATA_TEST_CASE(test_case_name, dataset)
> BOOST_FIXTURE_TEST_CASE( test_case2, F )
> BOOST_AUTO_TEST_CASE( test_case3 )
> 
> ```
> 
> too many for you to capture in the checker...but a nice addition for those who use alternative frameworks and would like to benefit from similar  "no underscore" naming conventions
> 
I'm not familiar with, e.g. the boost testing framework, so I don't know how closely it mirrors Googletest, but I think my preference would be to have a separate check for other testing frameworks.

While the testing frameworks might share some high-level similarities, there could be some different corner cases which would make having a separate check worth it as opposed to making this code more complex by trying to generalize it for several cases. At the very least, a different diagnostic message would be required. Factoring out similar functionality into some utility functions might reduce some of the boilerplate from implementing separate checks.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56424