Add unit-test for UpwardFileSearch().

.github/bin/build-and-test.sh

@@ -54,7 +54,7 @@ BAZEL_OPTS="${BAZEL_OPTS} --cxxopt=-Wno-array-bounds"             # kythe
 
 case "$MODE" in
 test)
-    bazel test $BAZEL_OPTS //...
+    bazel test --test_output=errors $BAZEL_OPTS //...
     ;;
 
 compile|clean)

.github/workflows/macos-compile.yml

@@ -21,4 +21,4 @@ jobs:
         fetch-depth: 0
     - name: Build
       run: |
-        bazel test --cxxopt=-Wno-range-loop-analysis ...
+        bazel test --test_output=errors --cxxopt=-Wno-range-loop-analysis ...

common/util/file_util.cc

@@ -174,7 +174,18 @@ absl::Status SetContents(absl::string_view filename,
 }
 
 std::string JoinPath(absl::string_view base, absl::string_view name) {
-  return absl::StrCat(base, "/", name);
+  // TODO: this will certainly be different on Windows
+  static constexpr absl::string_view kFileSeparator = "/";
+
+  // Make sure that we don't concatenate multiple superfluous separators.
+  // Note, since we're using string_view, the substr() operations are cheap.
+  while (!base.empty() && base[base.length() - 1] == kFileSeparator[0]) {
+    base = base.substr(0, base.length() - 1);
+  }
+  while (!name.empty() && name[0] == kFileSeparator[0]) {
+    name = name.substr(1);
+  }
+  return absl::StrCat(base, kFileSeparator, name);
 }
 
 absl::Status CreateDir(absl::string_view dir) {

common/util/file_util_test.cc

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include "absl/status/status.h"
+#include "absl/strings/match.h"
 #include "absl/strings/string_view.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -27,6 +28,8 @@ using testing::HasSubstr;
 
 #undef EXPECT_OK
 #define EXPECT_OK(value) EXPECT_TRUE((value).ok())
+#undef ASSERT_OK
+#define ASSERT_OK(value) ASSERT_TRUE((value).ok())
 
 namespace verible {
 namespace util {
@@ -61,6 +64,23 @@ TEST(FileUtil, Stem) {
   EXPECT_EQ(file::Stem("/foo/bar"), "/foo/bar");
 }
 
+TEST(FileUtil, JoinPath) {
+  EXPECT_EQ(file::JoinPath("foo", ""), "foo/");
+  EXPECT_EQ(file::JoinPath("", "bar"), "/bar");
+  EXPECT_EQ(file::JoinPath("foo", "bar"), "foo/bar");
+
+  // Assemble an absolute path
+  EXPECT_EQ(file::JoinPath("", "bar"), "/bar");
+  EXPECT_EQ(file::JoinPath("/", "bar"), "/bar");
+  EXPECT_EQ(file::JoinPath("///", "///bar"), "/bar");
+
+  // Lightly canonicalize multiple consecutive slashes
+  EXPECT_EQ(file::JoinPath("foo/", "bar"), "foo/bar");
+  EXPECT_EQ(file::JoinPath("foo///", "bar"), "foo/bar");
+  EXPECT_EQ(file::JoinPath("foo/", "/bar"), "foo/bar");
+  EXPECT_EQ(file::JoinPath("foo/", "///bar"), "foo/bar");
+}
+
 TEST(FileUtil, CreateDir) {
   const std::string test_dir = file::JoinPath(testing::TempDir(), "test_dir");
   const std::string test_file = file::JoinPath(test_dir, "foo");
@@ -152,6 +172,55 @@ TEST(FileUtil, FileExistsDirectoryErrorMessage) {
   EXPECT_THAT(s.message(), HasSubstr("is a directory"));
 }
 
+static bool CreateFsStructure(absl::string_view base_dir,
+                              const std::vector<absl::string_view>& tree) {
+  for (absl::string_view path : tree) {
+    const std::string full_path = file::JoinPath(base_dir, path);
+    if (absl::EndsWith(path, "/")) {
+      if (!file::CreateDir(full_path).ok()) return false;
+    } else {
+      if (!file::SetContents(full_path, "(content)").ok()) return false;
+    }
+  }
+  return true;
+}
+
+TEST(FileUtil, UpwardFileSearchTest) {
+  const std::string root_dir = file::JoinPath(testing::TempDir(), "up-search");
+  ASSERT_OK(file::CreateDir(root_dir));
+  ASSERT_TRUE(CreateFsStructure(root_dir, {
+                                              "toplevel-file",
+                                              "foo/",
+                                              "foo/foo-file",
+                                              "foo/bar/",
+                                              "foo/bar/baz/",
+                                              "foo/bar/baz/baz-file",
+                                          }));
+  std::string result;
+  // Same directory
+  EXPECT_OK(file::UpwardFileSearch(file::JoinPath(root_dir, "foo"), "foo-file",
+                                   &result));
+  // We don't compare the full path, as the UpwardFileSearch() makes it an
+  // realpath which might not entirely match our root_dir prefix anymore; but
+  // we do know that the suffix should be the same.
+  EXPECT_TRUE(absl::EndsWith(result, "up-search/foo/foo-file"));
+
+  // Somewhere below
+  EXPECT_OK(file::UpwardFileSearch(file::JoinPath(root_dir, "foo/bar/baz"),
+                                   "foo-file", &result));
+  EXPECT_TRUE(absl::EndsWith(result, "up-search/foo/foo-file"));
+
+  // Find toplevel file
+  EXPECT_OK(file::UpwardFileSearch(file::JoinPath(root_dir, "foo/bar/baz"),
+                                   "toplevel-file", &result));
+  EXPECT_TRUE(absl::EndsWith(result, "up-search/toplevel-file"));
+
+  // Negative test.
+  auto status = file::UpwardFileSearch(file::JoinPath(root_dir, "foo/bar/baz"),
+                                       "unknownfile", &result);
+  EXPECT_FALSE(status.ok());
+}
+
 TEST(FileUtil, ReadEmptyDirectory) {
   const std::string test_dir = file::JoinPath(testing::TempDir(), "empty_dir");
   ASSERT_TRUE(file::CreateDir(test_dir).ok());