[PATCH] D113268: [clangd] Fix tests with forward slash as separator on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 388857.
mstorsjo added a comment.

Added a comment about the surprising change where the order of append and native is switched. Made the modification to `testRoot()` less hacky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

Files:
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/TestFS.cpp
  clang-tools-extra/clangd/unittests/TestFS.h
  clang-tools-extra/clangd/unittests/URITests.cpp


Index: clang-tools-extra/clangd/unittests/URITests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/URITests.cpp
+++ clang-tools-extra/clangd/unittests/URITests.cpp
@@ -133,8 +133,10 @@
 
 TEST(URITest, Resolve) {
 #ifdef _WIN32
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), "c:\\x\\y\\z");
-  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), "c:\\x\\y\\z");
+  llvm::SmallString<16> Ref("c:\\x\\y\\z");
+  llvm::sys::path::native(Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c%3a/x/y/z")), Ref);
+  EXPECT_THAT(resolveOrDie(parseOrDie("file:///c:/x/y/z")), Ref);
 #else
   EXPECT_EQ(resolveOrDie(parseOrDie("file:/a/b/c")), "/a/b/c");
   EXPECT_EQ(resolveOrDie(parseOrDie("file://auth/a/b/c")), "//auth/a/b/c");
@@ -148,13 +150,17 @@
 
 TEST(URITest, ResolveUNC) {
 #ifdef _WIN32
+  llvm::SmallString<32> RefExample("\\\\example.com\\x\\y\\z");
+  llvm::sys::path::native(RefExample);
+  llvm::SmallString<16> RefLocalhost("\\\\127.0.0.1\\x\\y\\z");
+  llvm::sys::path::native(RefLocalhost);
   EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
-              "\\\\example.com\\x\\y\\z");
+              RefExample);
   EXPECT_THAT(resolveOrDie(parseOrDie("file://127.0.0.1/x/y/z")),
-              "\\\\127.0.0.1\\x\\y\\z");
+              RefLocalhost);
   // Ensure non-traditional file URI still resolves to correct UNC path.
   EXPECT_THAT(resolveOrDie(parseOrDie("file:////127.0.0.1/x/y/z")),
-              "\\\\127.0.0.1\\x\\y\\z");
+              RefLocalhost);
 #else
   EXPECT_THAT(resolveOrDie(parseOrDie("file://example.com/x/y/z")),
               "//example.com/x/y/z");
Index: clang-tools-extra/clangd/unittests/TestFS.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -75,7 +75,7 @@
 };
 
 // Returns an absolute (fake) test directory for this OS.
-const char *testRoot();
+std::string testRoot();
 
 // Returns a suitable absolute path for this OS.
 std::string testPath(PathRef File,
Index: clang-tools-extra/clangd/unittests/TestFS.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.cpp
+++ clang-tools-extra/clangd/unittests/TestFS.cpp
@@ -71,9 +71,11 @@
                                   FileName, std::move(CommandLine), "")};
 }
 
-const char *testRoot() {
+std::string testRoot() {
 #ifdef _WIN32
-  return "C:\\clangd-test";
+  llvm::SmallString<16> NativeRoot("C:\\clangd-test");
+  llvm::sys::path::native(NativeRoot);
+  return std::string(NativeRoot);
 #else
   return "/clangd-test";
 #endif
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -541,15 +541,20 @@
           Body);
     Body = Body.ltrim('/');
     llvm::SmallString<16> Path(Body);
-    path::native(Path);
     fs::make_absolute(TestScheme::TestDir, Path);
+    // Convert the whole path to native separators after concatenating with
+    // TestDir; TestDir might contain separators other than the preferred
+    // on Windows if forward slashes are preferred.
+    path::native(Path);
     return std::string(Path);
   }
 
   llvm::Expected<URI>
   uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
     llvm::StringRef Body = AbsolutePath;
-    if (!Body.consume_front(TestScheme::TestDir))
+    llvm::SmallString<16> NativeTestDir(TestDir);
+    llvm::sys::path::native(NativeTestDir);
+    if (!Body.consume_front(NativeTestDir))
       return error("Path {0} doesn't start with root {1}", AbsolutePath,
                    TestDir);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113268.388857.patch
Type: text/x-patch
Size: 3827 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211122/5115f5ab/attachment.bin>