[PATCH] D43590: [clang-format] Fix regression when getStyle() called with empty filename

Bjorn Pettersson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 24 05:40:07 PST 2018


bjope added inline comments.


================
Comment at: unittests/Format/FormatTest.cpp:11727
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+  auto Style1 = getStyle("file", "", "Google");
+  ASSERT_TRUE((bool)Style1);
----------------
bjope wrote:
> Do you really want to try to find a ".clang-format" file here, with fallback to "Google" if no such file is found?
> 
> When I'm building I usually end up having my build directory inside the llvm repo. And since there is a .clang-format file checked in to llvm that file is found, as it searches for a .clang-format file somewhere in the directory structure above the current dir when running the test (if I remember correctly?). We have had such problem before.
> 
> Can't you just as well do
>   auto Style1 = getStyle("Google", "", "Google");
> or is that not triggering the original bug?
> 
> Right now our build bots ends up like this (I guess it has found the .clang-format in my llvm/clang repo and decided to use "LLVM" as format for "Style1"):
> 
> ```
> FAIL: Clang-Unit :: Format/./FormatTests/FormatStyle.GetStyleWithEmptyFileName (14009 of 36611)
> ******************** TEST 'Clang-Unit :: Format/./FormatTests/FormatStyle.GetStyleWithEmptyFileName' FAILED ********************
> Note: Google Test filter = FormatStyle.GetStyleWithEmptyFileName
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from FormatStyle
> [ RUN      ] FormatStyle.GetStyleWithEmptyFileName
> ../tools/clang/unittests/Format/FormatTest.cpp:11729: Failure
>       Expected: *Style1
>       Which is: 456-byte object <FE-FF FF-FF 00-00 00-00 00-00 00-00 02-00 00-00 01-01 01-00 00-00 00-00 04-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 01-01 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 00-00 01-01 01-00 01-01 6E-53 01-00 00-00 00-00 00-00 01-00 00-00 00-01 00-00 00-00 00-00 01-00 00-00 08-00 00-00 00-00 00-00 10-73 77-01 00-00 00-00 50-73 77-01 00-00 00-00>
> To be equal to: getGoogleStyle()
>       Which is: 456-byte object <FF-FF FF-FF 00-00 00-00 00-00 00-00 01-00 00-00 01-01 01-00 00-00 00-00 04-00 00-00 01-01 00-00 00-00 00-00 00-00 00-00 01-01 01-01 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 38-4A 77-01 00-00 00-00 01-01 01-00 01-01 00-00 01-00 00-00 00-00 00-00 02-00 00-00 00-01 00-00 00-73 77-01 02-00 00-00 08-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00>
> [  FAILED  ] FormatStyle.GetStyleWithEmptyFileName (1 ms)
> [----------] 1 test from FormatStyle (1 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (1 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] FormatStyle.GetStyleWithEmptyFileName
> 
> ```
> 
I've proposed a fix here: https://reviews.llvm.org/D43732
By using an InMemoryFileSystem to avoid relying on no .clang-format present in the path of the real file system when running the test.


Repository:
  rC Clang

https://reviews.llvm.org/D43590





More information about the cfe-commits mailing list