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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits