Add actions for building and testing (#8)

* Add a Github action for building and testing

On Windows, -- -//third_party/... doesn't seem to work, so add all test directories manually. Also run the tests_*. We run only fastbuild tests here, since the opt tests will be run in the release workflow.

Also fix a number of compilation and test issues found along the way.

.github/workflows/build_and_test.yml

@@ -0,0 +1,65 @@
+name: Build & Test
+
+# Run when something is pushed to main or when there's action on a pull request.
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+# Cancel running workflow if a pull request is modified. Note that head_ref is
+# undefined for pushes to main. Use run_id as fallback. This is unique for each
+# run, so runs for pushes to main are never cancelled.
+concurrency: 
+  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  Build-And-Test-Linux:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Initialize submodules
+        run: git submodule update --init --recursive
+
+      - name: Build (fastbuild)
+        run: |
+          bazel build --config=linux -- //... -//third_party/...
+
+      # Skip file_finder_test: The test works when file_finder_test is run
+      # directly, but not through bazel test. The reason is, bazel test
+      # creates symlinks of test files, but the finder ignores symlinks.
+      # Also run tests sequentially since some tests write to a common tmp dir.
+      - name: Test (fastbuild)
+        run: |
+          bazel test --config=linux --test_output=errors --local_test_jobs=1 -- //... -//third_party/... -//cdc_rsync_server:file_finder_test
+
+  Build-And-Test-Windows:
+    runs-on: windows-2019
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Initialize submodules
+        run: git submodule update --init --recursive
+
+      - name: Build
+        run: |
+          bazel build --config=windows //cdc_rsync //cdc_stream //asset_stream_manager //tests_common //tests_asset_streaming_30 //tests_cdc_rsync
+
+      - name: Test
+        run: |
+          bazel-bin\tests_common\tests_common.exe
+          bazel-bin\tests_asset_streaming_30\tests_asset_streaming_30.exe
+          bazel-bin\tests_cdc_rsync\tests_cdc_rsync.exe
+          bazel test --config=windows --test_output=errors --local_test_jobs=1 `
+            //asset_stream_manager/... `
+            //cdc_fuse_fs/... `
+            //cdc_rsync/... `
+            //cdc_rsync/base/... `
+            //cdc_rsync_server/... `
+            //common/... `
+            //data_store/... `
+            //fastcdc/... `
+            //manifest/... `
+            //metrics/...

.gitignore

@@ -11,3 +11,4 @@ dependencies
 *.includes
 .qtc_clangd
 bazel-*
+user.bazelrc

cdc_fuse_fs/cdc_fuse_fs.cc

@@ -1648,7 +1648,7 @@ absl::Status Run(DataStoreReader* data_store_reader, bool consistency_check) {
   ctx->data_store_reader = data_store_reader;
   InitializeRootManifest();
 #ifndef USE_MOCK_LIBFUSE
-  RETURN_IF_ERROR(ctx->config_stream_client_->StartListeningToManifestUpdates(
+  RETURN_IF_ERROR(ctx->config_stream_client->StartListeningToManifestUpdates(
                       [](const ContentIdProto& id) { return SetManifest(id); }),
                   "Failed to listen to manifest updates");
 

common/path_test.cc

@@ -29,6 +29,14 @@
 #include "common/util.h"
 #include "gtest/gtest.h"
 
+#if PLATFORM_WINDOWS
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN 1
+#endif
+#include <windows.h>  // GetLongPathName
+#undef ReplaceFile
+#endif
+
 namespace cdc_ft {
 namespace {
 
@@ -53,6 +61,22 @@ constexpr char kTestFileName[] = "test.txt";
 constexpr char kUnicodeTestFileName[] = u8"unicode test \U0001F964.txt";
 constexpr char kSubDirFileName[] = "subdir_file.txt";
 
+// Converts e.g. C:\Progra~1 to C:\Program Files on Windows.
+std::string GetLongPath(std::string path) {
+#if PLATFORM_WINDOWS
+  std::wstring wpath = Util::Utf8ToWideStr(path);
+  std::wstring long_wpath(4096, 0);
+  size_t buf_len =
+      GetLongPathName(wpath.c_str(), const_cast<wchar_t*>(long_wpath.c_str()),
+                      long_wpath.size() * sizeof(wchar_t));
+  EXPECT_GT(buf_len, 0);
+  long_wpath.resize(buf_len);
+  return Util::WideToUtf8Str(long_wpath);
+#else
+  return path;
+#endif
+}
+
 class PathTest : public ::testing::Test {
  public:
   PathTest() {
@@ -159,7 +183,10 @@ class PathTest : public ::testing::Test {
 
   std::vector<File> SearchFiles(const std::string& pattern, bool recursive);
 
-  std::string base_dir_ = path::Join(path::GetTempDir(), kBaseDirName);
+  // Some tests get confused if the temp dir is a short filename
+  // (e.g. RUNNER~1 instead of runneradmin - I'm looking at you, Github!).
+  const std::string tmp_dir_ = GetLongPath(path::GetTempDir());
+  std::string base_dir_ = path::Join(tmp_dir_, kBaseDirName);
   std::string search_dir_ = path::Join(base_dir_, kSearchDirName);
   std::string subdir_ = path::Join(search_dir_, kSubDirName);
   const std::string subdir_fulldirpath_ = subdir_;
@@ -303,9 +330,11 @@ TEST_F(PathTest, GetFullPath) {
             path::Join(cwd, u8"\U0001F964", "foo"));
 
 #if PLATFORM_WINDOWS
-  // These test cases assume that C: is the current drive.
+  std::string current_drive = path::GetDrivePrefix(cwd);
-  EXPECT_EQ(path::GetFullPath("C:"), cwd);
+  ASSERT_NE(current_drive, std::string());
-  EXPECT_EQ(path::GetFullPath("C:foo"), path::Join(cwd, "foo"));
+  ASSERT_EQ(current_drive.size(), 2) << current_drive;
+  EXPECT_EQ(path::GetFullPath(current_drive), cwd);
+  EXPECT_EQ(path::GetFullPath(current_drive + "foo"), path::Join(cwd, "foo"));
   EXPECT_EQ(path::GetFullPath("V:foo"), "V:\\foo");
   EXPECT_EQ(path::GetFullPath("C:\\"), "C:\\");
   EXPECT_EQ(path::GetFullPath("C:\\foo"), "C:\\foo");
@@ -322,9 +351,10 @@ TEST_F(PathTest, GetFullPath) {
   // other Windows APIs are trimming such spaces, most notably GetFullPathNameW.
   EXPECT_EQ(path::GetFullPath("trailing space "),
             path::Join(cwd, "trailing space "));
-  EXPECT_EQ(path::GetFullPath("C:trailing space "),
+  EXPECT_EQ(path::GetFullPath(current_drive + "trailing space "),
             path::Join(cwd, "trailing space "));
-  EXPECT_EQ(path::GetFullPath("C:\\trailing space "), "C:\\trailing space ");
+  EXPECT_EQ(path::GetFullPath(current_drive + "\\trailing space "),
+            current_drive + "\\trailing space ");
   EXPECT_EQ(path::GetFullPath("V:trailing space "), "V:\\trailing space ");
 #else
   EXPECT_EQ(path::GetFullPath("/"), "/");
@@ -753,6 +783,12 @@ std::vector<PathTest::File> PathTest::SearchFiles(const std::string& pattern,
   };
 
   EXPECT_OK(path::SearchFiles(pattern, recursive, handler));
+
+  // Linux and Windows yield a slightly different order, so sort first.
+  std::sort(files.begin(), files.end(), [](const File& a, const File& b) {
+    return a.filename_ < b.filename_;
+  });
+
   return files;
 }
 
@@ -771,9 +807,11 @@ TEST_F(PathTest, SearchFiles_NonRecursiveTrailingSeparator) {
 
   EXPECT_EQ(files[1].dir_, search_dir_);
   EXPECT_EQ(files[1].Path(), test_filepath_);
+  EXPECT_FALSE(files[1].is_directory_);
 
   EXPECT_EQ(files[2].dir_, search_dir_);
   EXPECT_EQ(files[2].Path(), unicode_test_filepath_);
+  EXPECT_FALSE(files[2].is_directory_);
 #endif
 }
 
@@ -793,9 +831,11 @@ TEST_F(PathTest, SearchFiles_NonRecursiveNoTrailingSeparator) {
 
   EXPECT_EQ(files[1].dir_, search_dir_);
   EXPECT_EQ(files[1].Path(), test_filepath_);
+  EXPECT_FALSE(files[1].is_directory_);
 
   EXPECT_EQ(files[2].dir_, search_dir_);
   EXPECT_EQ(files[2].Path(), unicode_test_filepath_);
+  EXPECT_FALSE(files[2].is_directory_);
 #endif
 }
 
@@ -814,9 +854,11 @@ TEST_F(PathTest, SearchFiles_RecursiveTrailingSeparator) {
 
   EXPECT_EQ(files[2].dir_, search_dir_);
   EXPECT_EQ(files[2].Path(), test_filepath_);
+  EXPECT_FALSE(files[2].is_directory_);
 
   EXPECT_EQ(files[3].dir_, search_dir_);
   EXPECT_EQ(files[3].Path(), unicode_test_filepath_);
+  EXPECT_FALSE(files[3].is_directory_);
 }
 
 #if PLATFORM_WINDOWS
@@ -1215,7 +1257,7 @@ TEST_F(PathTest, Exists) {
 }
 
 TEST_F(PathTest, CreateDir) {
-  std::string dir = path::Join(path::GetTempDir(), "createdir_test");
+  std::string dir = path::Join(tmp_dir_, "createdir_test");
   std::string unicode_dir = path::Join(dir, u8"\U0001F964\U0001F964\U0001F964");
   path::RemoveDirRec(dir).IgnoreError();
   EXPECT_NOT_OK(path::CreateDir(path::Join(dir, "subdir")));
@@ -1232,7 +1274,7 @@ TEST_F(PathTest, CreateDir) {
 }
 
 TEST_F(PathTest, CreateDirRec) {
-  std::string dir = path::Join(path::GetTempDir(), "createdir_test", "subdir");
+  std::string dir = path::Join(tmp_dir_, "createdir_test", "subdir");
   std::string unicode_dir = path::Join(dir, u8"\U0001F964\U0001F964\U0001F964");
   path::RemoveDirRec(dir).IgnoreError();
   EXPECT_OK(path::CreateDirRec(dir));

common/process_test.cc

@@ -230,7 +230,7 @@ TEST_F(ProcessTest, RunUntil) {
   ProcessStartInfo start_info;
   start_info.command = "findstr stop";
   start_info.redirect_stdin = true;
-  std::atomic_bool stop(false);
+  std::atomic_bool stop{false};
   start_info.stdout_handler = [&std_out, &stop](const char* data, size_t) {
     // Check whether someone sent the "stop" command.
     // Note: This runs in a background thread.

common/process_win.cc

@@ -49,7 +49,7 @@ void SetThreadName(const std::string& name) {
   }
 }
 
-std::atomic_int g_pipe_serial_number(0);
+std::atomic_int g_pipe_serial_number{0};
 
 // Creates a pipe suitable for overlapped IO. Regular anonymous pipes in Windows
 // don't support overlapped IO. This method creates a named pipe with a unique

common/semaphore_test.cc

@@ -79,9 +79,9 @@ TEST_F(SemaphoreTest, DoesNotBlockIfInitialCountIsOne) {
 
 TEST_F(SemaphoreTest, SignalManyThreads) {
   Semaphore semaphore(0);
-  std::atomic_int a(0);
+  std::atomic_int a{0};
-  std::atomic_int b(0);
+  std::atomic_int b{0};
-  std::atomic_int c(0);
+  std::atomic_int c{0};
 
   const int N = 16;
   std::vector<std::thread> threads;

common/threadpool.cc

@@ -16,7 +16,7 @@
 
 namespace cdc_ft {
 
-Threadpool::Threadpool(size_t num_threads) : shutdown_(false) {
+Threadpool::Threadpool(size_t num_threads) : shutdown_{false} {
   workers_.reserve(num_threads);
   for (size_t n = 0; n < num_threads; ++n) {
     workers_.emplace_back([this]() { ThreadWorkerMain(); });

common/threadpool_test.cc

@@ -49,7 +49,7 @@ TEST_F(ThreadpoolTest, WaitShutdownWorkWithoutTasks) {
 }
 
 TEST_F(ThreadpoolTest, SingleThreadedRunsToCompletion) {
-  std::atomic_bool task_finished(false);
+  std::atomic_bool task_finished{false};
   auto task_func = [&task_finished](Task::IsCancelledPredicate) {
     task_finished = true;
   };
@@ -69,7 +69,7 @@ TEST_F(ThreadpoolTest, SingleThreadedRunsToCompletion) {
 TEST_F(ThreadpoolTest, MultiThreadedRunsToCompletion) {
   const int num_tasks = 19;
   const int num_threads = 7;
-  std::atomic_int num_completed(0);
+  std::atomic_int num_completed{0};
 
   Threadpool pool(num_threads);
   std::unordered_set<Task*> tasks;
@@ -95,7 +95,7 @@ TEST_F(ThreadpoolTest, MultiThreadedRunsToCompletion) {
 
 TEST_F(ThreadpoolTest, TaskIsCancelledOnShutdown) {
   Semaphore task_started(0);
-  std::atomic_bool task_finished(false);
+  std::atomic_bool task_finished{false};
   auto task_func = [&task_started,
                     &task_finished](Task::IsCancelledPredicate is_cancelled) {
     task_started.Signal();

data_store/data_provider.cc

@@ -40,7 +40,7 @@ DataProvider::DataProvider(
     : prefetch_size_(prefetch_size),
       writer_(std::move(writer)),
       readers_(std::move(readers)),
-      chunks_updated_(true),
+      chunks_updated_{true},
       cleanup_timeout_sec_(cleanup_timeout_sec),
       access_idle_timeout_sec_(access_idle_timeout_sec) {
   if (writer_) {
@@ -74,7 +74,7 @@ size_t DataProvider::PrefetchSize(size_t read_size) const {
 absl::StatusOr<size_t> DataProvider::Get(const ContentIdProto& content_id,
                                          void* data, size_t offset,
                                          size_t size) {
-  last_access_ts_ = steady_clock_->Now();
+  last_access_sec_ = GetSteadyNowSec();
   absl::Mutex* content_mutex = GetContentMutex(content_id);
   absl::StatusOr<size_t> read_bytes;
   if (writer_) {
@@ -127,7 +127,7 @@ absl::StatusOr<size_t> DataProvider::Get(const ContentIdProto& content_id,
 }
 
 absl::Status DataProvider::Get(ChunkTransferList* chunks) {
-  last_access_ts_ = steady_clock_->Now();
+  last_access_sec_ = GetSteadyNowSec();
   // Try to fetch chunks from the cache first.
   RETURN_IF_ERROR(GetFromWriter(chunks, /*lock_required=*/true));
   if (chunks->ReadDone()) return absl::OkStatus();
@@ -175,7 +175,7 @@ absl::Status DataProvider::Get(ChunkTransferList* chunks) {
 }
 
 absl::Status DataProvider::Get(const ContentIdProto& content_id, Buffer* data) {
-  last_access_ts_ = steady_clock_->Now();
+  last_access_sec_ = GetSteadyNowSec();
   absl::Mutex* content_mutex = GetContentMutex(content_id);
   absl::Status status = absl::OkStatus();
   if (writer_) {
@@ -324,9 +324,7 @@ void DataProvider::CleanupThreadMain() {
                                    next_cleanup_time - steady_clock_->Now())
                                    .count())));
     int64_t time_sec_since_last_access =
-        std::chrono::duration_cast<std::chrono::seconds>(steady_clock_->Now() -
+        GetSteadyNowSec() - last_access_sec_.load();
-                                                         last_access_ts_.load())
-            .count();
     if (chunks_updated_ &&
         time_sec_since_last_access > access_idle_timeout_sec_) {
       WriterMutexLockList locks;
@@ -361,4 +359,11 @@ bool DataProvider::WaitForCleanupAndResetForTesting(absl::Duration timeout) {
   is_cleaned_ = false;
   return is_cleaned;
 }
+
+int64_t DataProvider::GetSteadyNowSec() {
+  return std::chrono::duration_cast<std::chrono::seconds>(steady_clock_->Now() -
+                                                          first_now_)
+      .count();
+}
+
 }  // namespace cdc_ft

data_store/data_provider.h

@@ -105,6 +105,9 @@ class DataProvider : public DataStoreReader {
   // Periodically cleans up data in |writer_|.
   void CleanupThreadMain() ABSL_LOCKS_EXCLUDED(shutdown_mutex_, cleaned_mutex_);
 
+  // Returns the current time of |steady_clock_| in seconds.
+  int64_t GetSteadyNowSec();
+
   static constexpr unsigned int kNumberOfMutexes = 256;
 
   // How much additional data to prefetch when a max. FUSE read is encountered.
@@ -124,15 +127,18 @@ class DataProvider : public DataStoreReader {
   // Indicates whether the shutdown was triggered.
   bool shutdown_ ABSL_GUARDED_BY(shutdown_mutex_) = false;
 
-  // The last access time.
+  // The last access time in seconds since construction. Note that for some
-  std::atomic<std::chrono::time_point<std::chrono::steady_clock>>
+  // compilers we can't use std::chrono::time_point with atomics, so keep the
-      last_access_ts_;
+  // time in seconds.
+  std::atomic<int64_t> last_access_sec_;
 
   // Identifies if new data was added to the cache since the last cleanup.
   std::atomic<bool> chunks_updated_;
 
   // Clock to track the last access time.
   SteadyClock* steady_clock_ = DefaultSteadyClock::GetInstance();
+  const std::chrono::time_point<std::chrono::steady_clock> first_now_ =
+      steady_clock_->Now();
 
   // Cleanup interval.
   uint32_t cleanup_timeout_sec_ = kCleanupTimeoutSec;

fastcdc/fastcdc.h

@@ -17,6 +17,8 @@
 #ifndef FASTCDC_FASTCDC_H_
 #define FASTCDC_FASTCDC_H_
 
+#include <string.h>
+
 #include <algorithm>
 #include <cassert>
 #include <cstdio>

manifest/BUILD

@@ -119,6 +119,8 @@ cc_library(
         "manifest_updater.h",
         "pending_assets_queue.h",
     ],
+    # Tests don't work under Linux, but we only need it on Windows, anyway.
+    target_compatible_with = ["@platforms//os:windows"],
     deps = [
         ":file_chunk_map",
         ":manifest_builder",