* corrects the signature of RmDir in the header and fixes handling of case where directory might not exist to begin with so that it doesn't error out if that's the case

* adds unit test for untested functions (`Stem`, `MkDir`, `Touch`, `Size`, `LoadBinaryFile`, `ListDir`, `Filename`, `Parent`, and `RmDir`.)

LiteRT-PiperOrigin-RevId: 826643032

Author: turbotoribio

litert/core/filesystem.cc

@@ -19,6 +19,7 @@
 #include <filesystem>  // NOLINT
 #include <fstream>
 #include <string>
+#include <system_error> // NOLINT
 #include <vector>
 
 #include "absl/strings/str_format.h"  // from @com_google_absl
@@ -160,20 +161,29 @@ Expected<std::string> Parent(absl::string_view path) {
 }
 
 Expected<void> RmDir(std::string path_to_remove) {
-  // remove_all recursively deletes the directory and all its contents.
-  std::uintmax_t count = std::filesystem::remove_all(path_to_remove);
-
-  if (count > 0 && IsDir(path_to_remove)) {
-    LITERT_LOG(LITERT_INFO,
-               "Successfully removed directory and its contents: %s (%ju "
-               "items deleted)",
-               path_to_remove.c_str(), count);
+  std::error_code error_code;
+  std::uintmax_t count =
+      std::filesystem::remove_all(path_to_remove, error_code);
+  if (error_code) {
+    return Error(kLiteRtStatusErrorFileIO,
+                 absl::StrFormat("Could not remove: %s, error: %s",
+                                 path_to_remove.c_str(), error_code.message()));
+  }
+
+  if (!Exists(path_to_remove)) {
+    if (count > 0) {
+      LITERT_LOG(LITERT_INFO,
+                 "Successfully removed directory and its contents: %s (%ju "
+                 "items deleted)",
+                 path_to_remove.c_str(), count);
+    }
+    // If count == 0 and it doesn't exist, it means it never existed. Still Ok.
+    return {};
   } else {
     return Error(kLiteRtStatusErrorFileIO,
-                 absl::StrFormat("Could not remove or path did not exist: %s",
+                 absl::StrFormat("Could not fully remove: %s",
                                  path_to_remove.c_str()));
   }
-  return {};
 }
 
 }  // namespace litert::internal

litert/core/filesystem.h

@@ -62,8 +62,9 @@ Expected<void> MkDir(absl::string_view path);
 // Get the parent directory of the given path.
 Expected<std::string> Parent(absl::string_view path);
 
-Expected<void> RmDir();
+Expected<void> RmDir(std::string path_to_remove);
 
 }  // namespace litert::internal
 
 #endif  // ODML_LITERT_LITERT_CORE_FILESYSTEM_H_
+

litert/core/filesystem_test.cc

@@ -1,3 +1,4 @@
+
 // Copyright 2024 Google LLC.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -14,19 +15,27 @@
 
 #include "litert/core/filesystem.h"
 
+#include <fstream>
+#include <ios>
+#include <string>
+
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include "absl/strings/str_format.h"  // from @com_google_absl
 #include "absl/strings/string_view.h"  // from @com_google_absl
 
 namespace litert::internal {
 namespace {
 
+using ::testing::UnorderedElementsAre;
+
 static constexpr absl::string_view kPrefix = "a/prefix";
 static constexpr absl::string_view kInfix = "an/infix";
 static constexpr absl::string_view kSuffix = "suffix.ext";
 static constexpr absl::string_view kPath = "a/prefix.ext";
 static constexpr absl::string_view kStem = "prefix";
 
+
 TEST(FilesystemTest, JoinTwo) {
   const auto path = Join({kPrefix, kSuffix});
   EXPECT_EQ(path, absl::StrFormat("%s/%s", kPrefix, kSuffix));
@@ -37,10 +46,99 @@ TEST(FilesystemTest, JoinMany) {
   EXPECT_EQ(path, absl::StrFormat("%s/%s/%s", kPrefix, kInfix, kSuffix));
 }
 
-TEST(FilesystemTest, Stem){
+TEST(FilesystemTest, Stem) {
   const auto stem = Stem(kPath);
   EXPECT_EQ(stem, kStem);
 }
 
+void WriteFile(absl::string_view path, absl::string_view content) {
+  std::ofstream ofs((std::string(path)), std::ios::binary);
+  ofs << content;
+}
+
+TEST(FilesystemTest, MkDirExistsIsDir) {
+  const std::string dir = Join({::testing::TempDir(), "test_dir"});
+  EXPECT_FALSE(Exists(dir));
+  auto status = MkDir(dir);
+  ASSERT_TRUE(status);
+  EXPECT_TRUE(Exists(dir));
+  EXPECT_TRUE(IsDir(dir));
+  EXPECT_FALSE(IsDir(Join({dir, "foo"})));
+}
+
+TEST(FilesystemTest, TouchExists) {
+  const std::string file = Join({::testing::TempDir(), "test_file"});
+  EXPECT_FALSE(Exists(file));
+  Touch(file);
+  EXPECT_TRUE(Exists(file));
+  EXPECT_FALSE(IsDir(file));
+}
+
+TEST(FilesystemTest, Size) {
+  const std::string file = Join({::testing::TempDir(), "test_file_size"});
+  WriteFile(file, "1234");
+  auto size = Size(file);
+  ASSERT_TRUE(size);
+  EXPECT_EQ(*size, 4);
+}
+
+TEST(FilesystemTest, LoadBinaryFile) {
+  const std::string file = Join({::testing::TempDir(), "test_file_load"});
+  const std::string content = "12345";
+  WriteFile(file, content);
+  auto buffer = LoadBinaryFile(file);
+  ASSERT_TRUE(buffer);
+  EXPECT_EQ(buffer->Size(), 5);
+  EXPECT_EQ(absl::string_view(buffer->StrData(), buffer->Size()), content);
+}
+
+TEST(FilesystemTest, ListDir) {
+  const std::string dir = Join({::testing::TempDir(), "list_dir_test"});
+  auto status = MkDir(dir);
+  ASSERT_TRUE(status);
+  const std::string file1 = Join({dir, "file1.txt"});
+  const std::string file2 = Join({dir, "file2.txt"});
+  Touch(file1);
+  Touch(file2);
+  auto list = ListDir(dir);
+  ASSERT_TRUE(list);
+  EXPECT_THAT(*list, UnorderedElementsAre(file1, file2));
+}
+
+TEST(FilesystemTest, Filename) {
+  const std::string dir = Join({::testing::TempDir(), "filename_test"});
+  auto status = MkDir(dir);
+  ASSERT_TRUE(status);
+  const std::string file = Join({dir, "file1.txt"});
+  Touch(file);
+  auto filename = Filename(file);
+  ASSERT_TRUE(filename);
+  EXPECT_EQ(*filename, "file1.txt");
+}
+
+TEST(FilesystemTest, Parent) {
+  const std::string dir = Join({::testing::TempDir(), "parent_test"});
+  auto status = MkDir(dir);
+  ASSERT_TRUE(status);
+  const std::string file = Join({dir, "file1.txt"});
+  Touch(file);
+  auto parent = Parent(file);
+  ASSERT_TRUE(parent);
+  EXPECT_EQ(*parent, dir);
+}
+
+TEST(FilesystemTest, RmDir) {
+  const std::string dir = Join({::testing::TempDir(), "rm_dir_test"});
+  auto status = MkDir(dir);
+  ASSERT_TRUE(status);
+  const std::string file = Join({dir, "file1.txt"});
+  Touch(file);
+  EXPECT_TRUE(Exists(dir));
+  auto rm_status = RmDir(dir);
+  ASSERT_TRUE(rm_status);
+  EXPECT_FALSE(Exists(dir));
+}
+
 }  // namespace
 }  // namespace litert::internal
+