GH-75: Add test and fix bugs.

abyss7 · Nov 5, 2016 · 85a9abc · 85a9abc
1 parent 477b881
commit 85a9abc
Show file tree

Hide file tree

Showing 11 changed files with 116 additions and 32 deletions.
diff --git a/build/run_gtest_tests b/build/run_gtest_tests
@@ -1,8 +1,9 @@
 #!/bin/sh
 
-test_dir=`dirname $0`/../out/Test.gn
+script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+test_dir="${script_dir}/../out/Test.gn"
 
 export ASAN_OPTIONS=new_delete_type_mismatch=0
 export LSAN_OPTIONS=suppressions=`dirname $0`/lsan_suppression.txt
 
-"${test_dir}/unit_tests" "$@"
+"${test_dir}/unit_tests" --data="${test_dir}/../../data" "$@"
diff --git a/data/issue-75-test.sqlite b/data/issue-75-test.sqlite
diff --git a/src/base/c_utils_forward.h b/src/base/c_utils_forward.h
@@ -14,6 +14,7 @@ Literal GetEnv(Literal env_name, Literal default_env = Literal::empty);
 Literal GetHomeDir(String* error = nullptr);
 void GetLastError(String* error);
 bool GetSelfPath(String& result, String* error = nullptr);
+String NormalizePath(const String& path, String* error = nullptr);
 Literal SetEnv(Literal env_name, const String& value, String* error = nullptr);
 bool SetPermissions(const String& path, int mask, String* error = nullptr);
 

diff --git a/src/base/c_utils_posix.h b/src/base/c_utils_posix.h
@@ -104,6 +104,17 @@ inline bool GetSelfPath(String& result, String* error) {
   return true;
 }
 
+inline String NormalizePath(const String& path, String* error) {
+  char new_path[PATH_MAX];
+
+  if (realpath(path.c_str(), new_path) == nullptr) {
+    GetLastError(error);
+    return path;
+  }
+
+  return String(new_path);
+}
+
 inline bool SetPermissions(const String& path, int mask, String* error) {
   if (chmod(path.c_str(), mask) == -1) {
     GetLastError(error);

diff --git a/src/base/file/file.h b/src/base/file/file.h
@@ -40,7 +40,7 @@ class File final : public Data {
   static bool Copy(const String& src_path, const String& dst_path,
                    String* error = nullptr);
   static bool Link(const String& src_path, const String& dst_path,
-                   String* error = nullptr);
+                   bool hard = true, String* error = nullptr);
   static bool Move(const String& src, const String& dst,
                    String* error = nullptr);
   static bool Delete(const String& path, String* error = nullptr);

diff --git a/src/base/file/file_posix.cc b/src/base/file/file_posix.cc
@@ -284,29 +284,47 @@ bool File::Copy(const String& src_path, const String& dst_path, String* error) {
 }
 
 // static
-bool File::Link(const String& src, const String& dst, String* error) {
-  auto Link = [&src, &dst]() -> int {
+bool File::Link(const String& src, const String& dst, bool hard,
+                String* error) {
+  auto Link = [&src, &dst](bool hard) -> int {
+    if (hard) {
 #if defined(OS_LINUX)
-    // Linux doesn't guarantee that |link()| do dereferences symlinks, thus
-    // we use |linkat()| which does for sure.
-    return linkat(AT_FDCWD, src.c_str(), AT_FDCWD, dst.c_str(),
-                  AT_SYMLINK_FOLLOW);
+      // Linux doesn't guarantee that |link()| do dereferences symlinks, thus
+      // we use |linkat()| which does for sure.
+      return linkat(AT_FDCWD, src.c_str(), AT_FDCWD, dst.c_str(),
+                    AT_SYMLINK_FOLLOW);
 #elif defined(OS_MACOSX)
-    return link(src.c_str(), dst.c_str());
+      return link(src.c_str(), dst.c_str());
 #else
 #pragma message "This platform doesn't support hardlinks!"
-    errno = EACCES;
-    return -1;
+      errno = EACCES;
+      return -1;
+#endif
+    } else {
+#if defined(OS_LINUX) || defined(OS_MACOSX)
+      return symlink(src.c_str(), dst.c_str());
+#else
+#pragma message "This platform doesn't support softlinks!"
+      errno = EACCES;
+      return -1;
 #endif
+    }
   };
 
-  // Try to create hard-link at first.
-  if (Link() == 0 ||
-      (errno == EEXIST && unlink(dst.c_str()) == 0 && Link() == 0)) {
+  // Try to create hard or soft link at first.
+  if (Link(hard) == 0 ||
+      (errno == EEXIST && unlink(dst.c_str()) == 0 && Link(hard) == 0)) {
     return true;
   }
 
-  return File::Copy(src, dst, error);
+  if (!File::Copy(src, dst, error)) {
+    if (error) {
+        error->assign("Failed to link " + dst + " to " + src + ": " + *error);
+    }
+    return false;
+  }
+
+  return true;
 }
 
 // static

diff --git a/src/base/file/file_test.cc b/src/base/file/file_test.cc
@@ -150,7 +150,7 @@ TEST(FileTest, Copy) {
   EXPECT_EQ(expected_content2, content);
 }
 
-TEST(FileTest, Link) {
+TEST(FileTest, HardLink) {
   const auto expected_content = "All your base are belong to us"_l;
   const TemporaryDir temp_dir;
   const String file1 = String(temp_dir) + "/1";
@@ -193,5 +193,9 @@ TEST(FileTest, Link) {
   EXPECT_EQ(inode, st.st_ino);
 }
 
+TEST(FileTest, DISABLED_SoftLink) {
+  // TODO: implement this test.
+}
+
 }  // namespace base
 }  // namespace dist_clang
diff --git a/src/cache/database_sqlite.cc b/src/cache/database_sqlite.cc
@@ -258,21 +258,21 @@ bool SQLite::CheckIntegrity() const {
     return false;
   }
 
+  // "PRAGMA quick_check" behaves like a SELECT with a single row.
+  result = sqlite3_step(stmt);
+
   bool check = false;
-  while ((result = sqlite3_step(stmt))) {
-    if (result == SQLITE_ROW) {
-      const String value =
-          reinterpret_cast<const char*>(sqlite3_column_text(stmt, 0));
-      if (value == "ok") {
-        check = true;
-        break;
-      } else {
-        LOG(DB_ERROR) << "Integrity check error: " << value;
-      }
-    } else if (result != SQLITE_DONE) {
-      LOG(DB_ERROR) << "Failed to get integrity check result with error: "
-                    << sqlite3_errstr(result);
+  if (result == SQLITE_ROW) {
+    const String value =
+        reinterpret_cast<const char*>(sqlite3_column_text(stmt, 0));
+    if (value == "ok") {
+      check = true;
+    } else {
+      LOG(DB_ERROR) << "Integrity check error: " << value;
     }
+  } else if (result != SQLITE_DONE) {
+    LOG(DB_ERROR) << "Failed to get integrity check result with error: "
+                  << sqlite3_errstr(result);
   }
 
   result = sqlite3_finalize(stmt);

diff --git a/src/cache/file_cache.h b/src/cache/file_cache.h
@@ -19,6 +19,7 @@ FORWARD_TEST(FileCacheTest, LockNonExistentFile);
 FORWARD_TEST(FileCacheTest, RemoveEntry);
 FORWARD_TEST(FileCacheTest, RestoreEntryWithMissingFile);
 FORWARD_TEST(FileCacheTest, UseIndexFromDisk);
+FORWARD_TEST(FileCacheTest, FallbackToInMemoryIndex);
 FORWARD_TEST(FileCacheMigratorTest, Version_0_to_1_Direct);
 FORWARD_TEST(FileCacheMigratorTest, Version_0_to_1_Simple);
 FORWARD_TEST(FileCacheMigratorTest, Version_1_to_2_Direct);
@@ -137,6 +138,7 @@ class FileCache {
   FRIEND_TEST(FileCacheTest, RemoveEntry);
   FRIEND_TEST(FileCacheTest, RestoreEntryWithMissingFile);
   FRIEND_TEST(FileCacheTest, UseIndexFromDisk);
+  FRIEND_TEST(FileCacheTest, FallbackToInMemoryIndex);
   FRIEND_TEST(FileCacheMigratorTest, Version_0_to_1_Direct);
   FRIEND_TEST(FileCacheMigratorTest, Version_0_to_1_Simple);
   FRIEND_TEST(FileCacheMigratorTest, Version_1_to_2_Direct);

diff --git a/src/cache/file_cache_test.cc b/src/cache/file_cache_test.cc
@@ -722,7 +722,51 @@ TEST(FileCacheTest, UseIndexFromDisk) {
 //           in-memory database.
 TEST(FileCacheTest, FallbackToInMemoryIndex) {
   const base::TemporaryDir tmp_dir;
-  // TODO: implement this test.
+  const String path = tmp_dir;
+  const String cache_path = path + "/cache";
+  const String db_path =
+      base::NormalizePath(FLAGS_data + "/issue-75-test.sqlite");
+  const String cache_db_path = cache_path + "/index.sqlite";
+  const Literal obj_content[] = {"22"_l, "333"_l, "4444"_l};
+  const HandledSource code[] = {HandledSource("int main() { return 0; }"_l),
+                                HandledSource("int main() { return 1; }"_l),
+                                HandledSource("int main() { return 2; }"_l)};
+  const CommandLine cl("-c"_l);
+  const Version version("3.5 (revision 100000)"_l);
+
+  ASSERT_TRUE(base::CreateDirectory(cache_path));
+  ASSERT_TRUE(base::File::Link(db_path, cache_db_path, false));
+
+  FileCache cache(cache_path, 138, false, true);
+  // 138 = sizeof(obj_content[0]) + sizeof(obj_content[1]) + 1 + 2 *
+  // <size_of_manifest>. The current typical size of manifest is 66 bytes.
+
+  ASSERT_TRUE(cache.Run(1));
+
+  {
+    FileCache::Entry entry{obj_content[0], String(), String()};
+    cache.Store(code[0], {}, cl, version, entry);
+  }
+
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  {
+    FileCache::Entry entry{obj_content[1], String(), String()};
+    cache.Store(code[1], {}, cl, version, entry);
+  }
+
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  {
+    FileCache::Entry entry{obj_content[2], String(), String()};
+    cache.Store(code[2], {}, cl, version, entry);
+    std::this_thread::sleep_for(std::chrono::seconds(3));
+  }
+
+  FileCache::Entry entry;
+  EXPECT_FALSE(cache.Find(code[0], {}, cl, version, &entry));
+  EXPECT_FALSE(cache.Find(code[1], {}, cl, version, &entry));
+  EXPECT_TRUE(cache.Find(code[2], {}, cl, version, &entry));
 }
 
 }  // namespace cache

diff --git a/src/test/run_all_tests.cc b/src/test/run_all_tests.cc
@@ -45,7 +45,7 @@ int main(int argc, char* argv[]) {
   gflags::ParseCommandLineFlags(&argc, &argv, false);
 
   if (FLAGS_data.empty()) {
-    LOG(WARNING) << "No data path specified - some tests may fail!";
+    LOG(ERROR) << "No data path specified - some tests may fail!";
   }
 
   return RUN_ALL_TESTS();