HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.
Although I would put it in one test, this is fine by me.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1201-1206
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 2u;
----------------
curdeius wrote:
> What I meant by "merging the tests together" is just writing:
> ```
> TEST_F(ShortNamespaceLinesTest,
> MultipleUnwrappedLine) {
> auto Style = getLLVMStyle();
> Style.ShortNamespaceLines = 2u;
>
> EXPECT_EQ(...);
>
> EXPECT_EQ(...);
> }
> ```
> This will get rid of duplicated setup (even if small), and, what's more
> important, show the behaviour with the same style on different cases.
>
> Or even, but I'm not forcing you to do so, having a single test case for all
> `ShortNamespaceLinesTest.*` tests. It groups the test for the same feature
> together.
I would even go further and put the 0, 1, and 2 in one function.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87587/new/
https://reviews.llvm.org/D87587
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits