[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using carlosgalvezp wrote: > Eugene.Zelenko wrote: > > carlosgalvezp wrote: > > > Eugene.Zelenko wrote: > > > > Eugene.Zelenko wrote: > > > > > carlosgalvezp wrote: > > > > > > PiotrZSL wrote: > > > > > > > carlosgalvezp wrote: > > > > > > > > Eugene.Zelenko wrote: > > > > > > > > > Please keep alphabetical order (by check name) in this > > > > > > > > > section. > > > > > > > > I was planning to do that but noticed that the alphabetical > > > > > > > > order is already broken. It seems to be a source of friction > > > > > > > > and there's no official documentation that states it should be > > > > > > > > done like that, so I can understand if it gets broken often. Do > > > > > > > > you know if this is documented somewhere? If not, do we see > > > > > > > > value in keeping this convention? I suppose now we would need > > > > > > > > an NFC patch to fix the order again, causing churn. > > > > > > > I run into same issue also. I would say, let leave it as it is, > > > > > > > and fix it with one commit at the end of release. > > > > > > Good idea, let's do that! > > > > > Often it's also broken after rebases which may be automatic. > > > > Anyway, some kind of order is much better than disorder. > > > Definitely. Could we stick to some simple convention? For example always > > > append or prepend to the list of modifications to checks. Then before > > > release we put up a patch for reordering. > > I think it will be harder to reader. Sorting by check name is much better > > in this respect. And this was used in many releases. > To clarify, what I mean is: > > - Apply a simple convention (e.g. append or prepend to the list) //while > developing//. > - Right before creating a release, put up a patch to sort alphabetically. > Then it will be easy to read for users when it's released. > > Or do you mean that the list shall be alphabetically sorted at all times? It'll be much easier to sort partially sorted list at release time than completely unsorted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using Eugene.Zelenko wrote: > carlosgalvezp wrote: > > Eugene.Zelenko wrote: > > > Eugene.Zelenko wrote: > > > > carlosgalvezp wrote: > > > > > PiotrZSL wrote: > > > > > > carlosgalvezp wrote: > > > > > > > Eugene.Zelenko wrote: > > > > > > > > Please keep alphabetical order (by check name) in this section. > > > > > > > I was planning to do that but noticed that the alphabetical order > > > > > > > is already broken. It seems to be a source of friction and > > > > > > > there's no official documentation that states it should be done > > > > > > > like that, so I can understand if it gets broken often. Do you > > > > > > > know if this is documented somewhere? If not, do we see value in > > > > > > > keeping this convention? I suppose now we would need an NFC patch > > > > > > > to fix the order again, causing churn. > > > > > > I run into same issue also. I would say, let leave it as it is, and > > > > > > fix it with one commit at the end of release. > > > > > Good idea, let's do that! > > > > Often it's also broken after rebases which may be automatic. > > > Anyway, some kind of order is much better than disorder. > > Definitely. Could we stick to some simple convention? For example always > > append or prepend to the list of modifications to checks. Then before > > release we put up a patch for reordering. > I think it will be harder to reader. Sorting by check name is much better in > this respect. And this was used in many releases. To clarify, what I mean is: - Apply a simple convention (e.g. append or prepend to the list) //while developing//. - Right before creating a release, put up a patch to sort alphabetically. Then it will be easy to read for users when it's released. Or do you mean that the list shall be alphabetically sorted at all times? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using carlosgalvezp wrote: > Eugene.Zelenko wrote: > > Eugene.Zelenko wrote: > > > carlosgalvezp wrote: > > > > PiotrZSL wrote: > > > > > carlosgalvezp wrote: > > > > > > Eugene.Zelenko wrote: > > > > > > > Please keep alphabetical order (by check name) in this section. > > > > > > I was planning to do that but noticed that the alphabetical order > > > > > > is already broken. It seems to be a source of friction and there's > > > > > > no official documentation that states it should be done like that, > > > > > > so I can understand if it gets broken often. Do you know if this is > > > > > > documented somewhere? If not, do we see value in keeping this > > > > > > convention? I suppose now we would need an NFC patch to fix the > > > > > > order again, causing churn. > > > > > I run into same issue also. I would say, let leave it as it is, and > > > > > fix it with one commit at the end of release. > > > > Good idea, let's do that! > > > Often it's also broken after rebases which may be automatic. > > Anyway, some kind of order is much better than disorder. > Definitely. Could we stick to some simple convention? For example always > append or prepend to the list of modifications to checks. Then before release > we put up a patch for reordering. I think it will be harder to reader. Sorting by check name is much better in this respect. And this was used in many releases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using Eugene.Zelenko wrote: > Eugene.Zelenko wrote: > > carlosgalvezp wrote: > > > PiotrZSL wrote: > > > > carlosgalvezp wrote: > > > > > Eugene.Zelenko wrote: > > > > > > Please keep alphabetical order (by check name) in this section. > > > > > I was planning to do that but noticed that the alphabetical order is > > > > > already broken. It seems to be a source of friction and there's no > > > > > official documentation that states it should be done like that, so I > > > > > can understand if it gets broken often. Do you know if this is > > > > > documented somewhere? If not, do we see value in keeping this > > > > > convention? I suppose now we would need an NFC patch to fix the order > > > > > again, causing churn. > > > > I run into same issue also. I would say, let leave it as it is, and fix > > > > it with one commit at the end of release. > > > Good idea, let's do that! > > Often it's also broken after rebases which may be automatic. > Anyway, some kind of order is much better than disorder. Definitely. Could we stick to some simple convention? For example always append or prepend to the list of modifications to checks. Then before release we put up a patch for reordering. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using Eugene.Zelenko wrote: > carlosgalvezp wrote: > > PiotrZSL wrote: > > > carlosgalvezp wrote: > > > > Eugene.Zelenko wrote: > > > > > Please keep alphabetical order (by check name) in this section. > > > > I was planning to do that but noticed that the alphabetical order is > > > > already broken. It seems to be a source of friction and there's no > > > > official documentation that states it should be done like that, so I > > > > can understand if it gets broken often. Do you know if this is > > > > documented somewhere? If not, do we see value in keeping this > > > > convention? I suppose now we would need an NFC patch to fix the order > > > > again, causing churn. > > > I run into same issue also. I would say, let leave it as it is, and fix > > > it with one commit at the end of release. > > Good idea, let's do that! > Often it's also broken after rebases which may be automatic. Anyway, some kind of order is much better than disorder. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using carlosgalvezp wrote: > PiotrZSL wrote: > > carlosgalvezp wrote: > > > Eugene.Zelenko wrote: > > > > Please keep alphabetical order (by check name) in this section. > > > I was planning to do that but noticed that the alphabetical order is > > > already broken. It seems to be a source of friction and there's no > > > official documentation that states it should be done like that, so I can > > > understand if it gets broken often. Do you know if this is documented > > > somewhere? If not, do we see value in keeping this convention? I suppose > > > now we would need an NFC patch to fix the order again, causing churn. > > I run into same issue also. I would say, let leave it as it is, and fix it > > with one commit at the end of release. > Good idea, let's do that! Often it's also broken after rebases which may be automatic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb08d35f826a6: [clang-tidy] Ignore DISABLED_ in test suite
name in google-avoid-underscore-in… (authored by carlosgalvezp).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146655/new/
https://reviews.llvm.org/D146655
Files:
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
===
---
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
@@ -87,21 +87,31 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using "_" in test case name
"Illegal_Type_ParameterizedTestCaseName" according to Googletest FAQ
[google-readability-avoid-underscore-in-googletest-name]
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
TEST(TestCaseName, DISABLED_TestName) {}
+TEST(DISABLED_TestCaseName, TestName) {}
+TEST(DISABLED_TestCaseName, DISABLED_TestName) {}
TEST_F(TestCaseFixtureName, TestName) {}
TEST_F(TestCaseFixtureName, DISABLED_TestName) {}
+TEST_F(DISABLED_TestCaseFixtureName, TestName) {}
+TEST_F(DISABLED_TestCaseFixtureName, DISABLED_TestName) {}
TEST_P(ParameterizedTestCaseFixtureName, TestName) {}
TEST_P(ParameterizedTestCaseFixtureName, DISABLED_TestName) {}
+TEST_P(DISABLED_ParameterizedTestCaseFixtureName, TestName) {}
+TEST_P(DISABLED_ParameterizedTestCaseFixtureName, DISABLED_TestName) {}
TYPED_TEST(TypedTestName, TestName) {}
TYPED_TEST(TypedTestName, DISABLED_TestName) {}
+TYPED_TEST(DISABLED_TypedTestName, TestName) {}
+TYPED_TEST(DISABLED_TypedTestName, DISABLED_TestName) {}
TYPED_TEST_P(TypeParameterizedTestName, TestName) {}
TYPED_TEST_P(TypeParameterizedTestName, DISABLED_TestName) {}
+TYPED_TEST_P(DISABLED_TypeParameterizedTestName, TestName) {}
+TYPED_TEST_P(DISABLED_TypeParameterizedTestName, DISABLED_TestName) {}
FRIEND_TEST(FriendTest, Is_NotChecked) {}
FRIEND_TEST(Friend_Test, IsNotChecked) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -234,6 +234,10 @@
string for ``Prefix`` or ``Suffix`` options could result in the style not
being used.
+- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name
+ ` when using
+ ``DISABLED_`` in the test suite name.
+
Removed checks
^^
Index:
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
@@ -51,8 +51,10 @@
const Token *TestNameToken = Args->getUnexpArgument(1);
if (!TestCaseNameToken || !TestNameToken)
return;
-std::string TestCaseName = PP->getSpelling(*TestCaseNameToken);
-if (TestCaseName.find('_') != std::string::npos)
+std::string TestCaseNameMaybeDisabled =
PP->getSpelling(*TestCaseNameToken);
+StringRef TestCaseName = TestCaseNameMaybeDisabled;
+TestCaseName.consume_front(KDisabledTestPrefix);
+if (TestCaseName.contains('_'))
Check->diag(TestCaseNameToken->getLocation(),
"avoid using \"_\" in test case name \"%0\" according to "
"Googletest FAQ")
Index: clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
@@ -87,21 +87,31 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using "_" in test case name "Illegal_Type_ParameterizedTestCaseName" according to Googletest FAQ [google-readability-avoid-underscore-in-googletest-name]
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-// https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using PiotrZSL wrote: > carlosgalvezp wrote: > > Eugene.Zelenko wrote: > > > Please keep alphabetical order (by check name) in this section. > > I was planning to do that but noticed that the alphabetical order is > > already broken. It seems to be a source of friction and there's no official > > documentation that states it should be done like that, so I can understand > > if it gets broken often. Do you know if this is documented somewhere? If > > not, do we see value in keeping this convention? I suppose now we would > > need an NFC patch to fix the order again, causing churn. > I run into same issue also. I would say, let leave it as it is, and fix it > with one commit at the end of release. Good idea, let's do that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
PiotrZSL added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using carlosgalvezp wrote: > Eugene.Zelenko wrote: > > Please keep alphabetical order (by check name) in this section. > I was planning to do that but noticed that the alphabetical order is already > broken. It seems to be a source of friction and there's no official > documentation that states it should be done like that, so I can understand if > it gets broken often. Do you know if this is documented somewhere? If not, do > we see value in keeping this convention? I suppose now we would need an NFC > patch to fix the order again, causing churn. I run into same issue also. I would say, let leave it as it is, and fix it with one commit at the end of release. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
carlosgalvezp added inline comments.
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237
+- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name
+ ` when using
Eugene.Zelenko wrote:
> Please keep alphabetical order (by check name) in this section.
I was planning to do that but noticed that the alphabetical order is already
broken. It seems to be a source of friction and there's no official
documentation that states it should be done like that, so I can understand if
it gets broken often. Do you know if this is documented somewhere? If not, do
we see value in keeping this convention? I suppose now we would need an NFC
patch to fix the order again, causing churn.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp:90
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
LegalizeAdulthood wrote:
> PiotrZSL wrote:
> > Would be good if this link would exist also in documentation for this check.
> My experience has been that these external links move around with gtest and
> are a source of churn.
Good point, I can add on a separate NFC patch :) I believe if we use the
github.io (instead of github.com) the links can be a bit more stable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146655/new/
https://reviews.llvm.org/D146655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
LegalizeAdulthood added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp:90
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
PiotrZSL wrote:
> Would be good if this link would exist also in documentation for this check.
My experience has been that these external links move around with gtest and are
a source of churn.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146655/new/
https://reviews.llvm.org/D146655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
PiotrZSL added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp:90
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
Would be good if this link would exist also in documentation for this check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146655/new/
https://reviews.llvm.org/D146655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:237 +- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name + ` when using Please keep alphabetical order (by check name) in this section. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146655/new/ https://reviews.llvm.org/D146655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
carlosgalvezp updated this revision to Diff 507467.
carlosgalvezp added a comment.
Remove excessive newline
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146655/new/
https://reviews.llvm.org/D146655
Files:
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
===
---
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
@@ -87,21 +87,31 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using "_" in test case name
"Illegal_Type_ParameterizedTestCaseName" according to Googletest FAQ
[google-readability-avoid-underscore-in-googletest-name]
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
TEST(TestCaseName, DISABLED_TestName) {}
+TEST(DISABLED_TestCaseName, TestName) {}
+TEST(DISABLED_TestCaseName, DISABLED_TestName) {}
TEST_F(TestCaseFixtureName, TestName) {}
TEST_F(TestCaseFixtureName, DISABLED_TestName) {}
+TEST_F(DISABLED_TestCaseFixtureName, TestName) {}
+TEST_F(DISABLED_TestCaseFixtureName, DISABLED_TestName) {}
TEST_P(ParameterizedTestCaseFixtureName, TestName) {}
TEST_P(ParameterizedTestCaseFixtureName, DISABLED_TestName) {}
+TEST_P(DISABLED_ParameterizedTestCaseFixtureName, TestName) {}
+TEST_P(DISABLED_ParameterizedTestCaseFixtureName, DISABLED_TestName) {}
TYPED_TEST(TypedTestName, TestName) {}
TYPED_TEST(TypedTestName, DISABLED_TestName) {}
+TYPED_TEST(DISABLED_TypedTestName, TestName) {}
+TYPED_TEST(DISABLED_TypedTestName, DISABLED_TestName) {}
TYPED_TEST_P(TypeParameterizedTestName, TestName) {}
TYPED_TEST_P(TypeParameterizedTestName, DISABLED_TestName) {}
+TYPED_TEST_P(DISABLED_TypeParameterizedTestName, TestName) {}
+TYPED_TEST_P(DISABLED_TypeParameterizedTestName, DISABLED_TestName) {}
FRIEND_TEST(FriendTest, Is_NotChecked) {}
FRIEND_TEST(Friend_Test, IsNotChecked) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -234,6 +234,10 @@
string for ``Prefix`` or ``Suffix`` options could result in the style not
being used.
+- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name
+ ` when using
+ ``DISABLED_`` in the test suite name.
+
Removed checks
^^
Index:
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
@@ -51,8 +51,10 @@
const Token *TestNameToken = Args->getUnexpArgument(1);
if (!TestCaseNameToken || !TestNameToken)
return;
-std::string TestCaseName = PP->getSpelling(*TestCaseNameToken);
-if (TestCaseName.find('_') != std::string::npos)
+std::string TestCaseNameMaybeDisabled =
PP->getSpelling(*TestCaseNameToken);
+StringRef TestCaseName = TestCaseNameMaybeDisabled;
+TestCaseName.consume_front(KDisabledTestPrefix);
+if (TestCaseName.contains('_'))
Check->diag(TestCaseNameToken->getLocation(),
"avoid using \"_\" in test case name \"%0\" according to "
"Googletest FAQ")
Index: clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
@@ -87,21 +87,31 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using "_" in test case name "Illegal_Type_ParameterizedTestCaseName" according to Googletest FAQ [google-readability-avoid-underscore-in-googletest-name]
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-// https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+// https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
TEST(TestCaseName,
[PATCH] D146655: [clang-tidy] Ignore DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name
carlosgalvezp updated this revision to Diff 507465.
carlosgalvezp retitled this revision from "[clang-tidy] Ignore more DISABLED_
in google-avoid-underscore-in-googletest-name" to "[clang-tidy] Ignore
DISABLED_ in test suite name in google-avoid-underscore-in-googletest-name".
carlosgalvezp added a comment.
Update commit message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146655/new/
https://reviews.llvm.org/D146655
Files:
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
===
---
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
@@ -87,21 +87,31 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using "_" in test case name
"Illegal_Type_ParameterizedTestCaseName" according to Googletest FAQ
[google-readability-avoid-underscore-in-googletest-name]
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
https://github.com/google/googletest/blob/master/googletest/docs/faq.md#why-should-test-suite-names-and-test-names-not-contain-underscore
+//
https://google.github.io/googletest/faq.html#why-should-test-suite-names-and-test-names-not-contain-underscore
TEST(TestCaseName, TestName) {}
TEST(TestCaseName, DISABLED_TestName) {}
+TEST(DISABLED_TestCaseName, TestName) {}
+TEST(DISABLED_TestCaseName, DISABLED_TestName) {}
TEST_F(TestCaseFixtureName, TestName) {}
TEST_F(TestCaseFixtureName, DISABLED_TestName) {}
+TEST_F(DISABLED_TestCaseFixtureName, TestName) {}
+TEST_F(DISABLED_TestCaseFixtureName, DISABLED_TestName) {}
TEST_P(ParameterizedTestCaseFixtureName, TestName) {}
TEST_P(ParameterizedTestCaseFixtureName, DISABLED_TestName) {}
+TEST_P(DISABLED_ParameterizedTestCaseFixtureName, TestName) {}
+TEST_P(DISABLED_ParameterizedTestCaseFixtureName, DISABLED_TestName) {}
TYPED_TEST(TypedTestName, TestName) {}
TYPED_TEST(TypedTestName, DISABLED_TestName) {}
+TYPED_TEST(DISABLED_TypedTestName, TestName) {}
+TYPED_TEST(DISABLED_TypedTestName, DISABLED_TestName) {}
TYPED_TEST_P(TypeParameterizedTestName, TestName) {}
TYPED_TEST_P(TypeParameterizedTestName, DISABLED_TestName) {}
+TYPED_TEST_P(DISABLED_TypeParameterizedTestName, TestName) {}
+TYPED_TEST_P(DISABLED_TypeParameterizedTestName, DISABLED_TestName) {}
FRIEND_TEST(FriendTest, Is_NotChecked) {}
FRIEND_TEST(Friend_Test, IsNotChecked) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -234,6 +234,11 @@
string for ``Prefix`` or ``Suffix`` options could result in the style not
being used.
+- Fixed an issue in :doc:`google-avoid-underscore-in-googletest-name
+ ` when using
+ ``DISABLED_`` in the test suite name.
+
+
Removed checks
^^
Index:
clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
+++ clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp
@@ -51,8 +51,10 @@
const Token *TestNameToken = Args->getUnexpArgument(1);
if (!TestCaseNameToken || !TestNameToken)
return;
-std::string TestCaseName = PP->getSpelling(*TestCaseNameToken);
-if (TestCaseName.find('_') != std::string::npos)
+std::string TestCaseNameMaybeDisabled =
PP->getSpelling(*TestCaseNameToken);
+StringRef TestCaseName = TestCaseNameMaybeDisabled;
+TestCaseName.consume_front(KDisabledTestPrefix);
+if (TestCaseName.contains('_'))
Check->diag(TestCaseNameToken->getLocation(),
"avoid using \"_\" in test case name \"%0\" according to "
"Googletest FAQ")
Index: clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google/avoid-underscore-in-googletest-name.cpp
@@ -87,21 +87,31 @@
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: avoid using "_" in test case name "Illegal_Type_ParameterizedTestCaseName" according to Googletest FAQ [google-readability-avoid-underscore-in-googletest-name]
// Underscores are allowed to disable a test with the DISABLED_ prefix.
-//
