Minor code cleanup

Added more tests
simgrid · Oct 11, 2024 · 3c9db84 · 3c9db84
1 parent c6c1994
commit 3c9db84
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 12 deletions.
diff --git a/include/fsmod/FileSystem.hpp b/include/fsmod/FileSystem.hpp
@@ -28,7 +28,7 @@ namespace simgrid::fsmod {
         static xbt::Extension<kernel::routing::NetZoneImpl, FileSystemNetZoneImplExtension> EXTENSION_ID;
 
         void register_file_system(const std::shared_ptr<FileSystem>& fs);
-        const std::map<std::string, std::shared_ptr<FileSystem>, std::less<>>& get_all_file_systems() const { return file_systems_;}
+        [[nodiscard]] const std::map<std::string, std::shared_ptr<FileSystem>, std::less<>>& get_all_file_systems() const { return file_systems_;}
     };
 
     /**
@@ -44,10 +44,10 @@ namespace simgrid::fsmod {
 
         static std::shared_ptr<FileSystem> create(const std::string &name, int max_num_open_files = 1024);
         static const std::map<std::string, std::shared_ptr<FileSystem>, std::less<>>&
-            get_file_systems_by_actor(s4u::ActorPtr actor);
+            get_file_systems_by_actor(const s4u::ActorPtr& actor);
         static const std::map<std::string, std::shared_ptr<FileSystem>, std::less<>>&
             get_file_systems_by_netzone(const s4u::NetZone* netzone);
-        static void register_file_system(const s4u::NetZone* netzone, std::shared_ptr<FileSystem> fs);
+        static void register_file_system(const s4u::NetZone* netzone, const std::shared_ptr<FileSystem>& fs);
 
         /**
          * @brief Retrieves the file system's name

diff --git a/include/fsmod/Partition.hpp b/include/fsmod/Partition.hpp
@@ -97,7 +97,7 @@ namespace simgrid::fsmod {
 
 
         std::string name_;
-        FileSystem *file_system_;FileSystem *file_system;
+        FileSystem *file_system_;
         std::shared_ptr<Storage> storage_;
         sg_size_t size_ = 0;
         sg_size_t free_space_ = 0;
@@ -110,7 +110,7 @@ namespace simgrid::fsmod {
 
         void create_new_directory(const std::string& dir_path);
         [[nodiscard]] bool directory_exists(const std::string& dir_path) const { return content_.find(dir_path) != content_.end(); }
-        std::set<std::string, std::less<>> list_files_in_directory(const std::string &dir_path) const;
+        [[nodiscard]] std::set<std::string, std::less<>> list_files_in_directory(const std::string &dir_path) const;
         void delete_directory(const std::string &dir_path);
 
         [[nodiscard]] FileMetadata* get_file_metadata(const std::string& dir_path, const std::string& file_name) const;

diff --git a/src/FileMetadata.cpp b/src/FileMetadata.cpp
@@ -8,7 +8,6 @@
 
 #include "fsmod/Partition.hpp"
 #include "fsmod/FileMetadata.hpp"
-#include "fsmod/FileSystemException.hpp"
 
 namespace simgrid::fsmod {
 

diff --git a/src/FileSystem.cpp b/src/FileSystem.cpp
@@ -122,7 +122,7 @@ namespace simgrid::fsmod {
     * @param netzone The SimGrid NetZone
     * @param fs The FSMOD file system
     */
-    void FileSystem::register_file_system(const s4u::NetZone* netzone, std::shared_ptr<FileSystem> fs) {
+    void FileSystem::register_file_system(const s4u::NetZone* netzone, const std::shared_ptr<FileSystem>& fs) {
         if (not FileSystemNetZoneImplExtension::EXTENSION_ID.valid()) {
             // This is the first time we register a FileSystem, register the NetZoneImpls extension properly
             FileSystemNetZoneImplExtension::EXTENSION_ID =
@@ -145,7 +145,7 @@ namespace simgrid::fsmod {
      * @return A file system map, using names as keys
      */
     const std::map<std::string, std::shared_ptr<FileSystem>, std::less<>>&
-    FileSystem::get_file_systems_by_actor(s4u::ActorPtr actor) {
+    FileSystem::get_file_systems_by_actor(const s4u::ActorPtr& actor) {
         const kernel::routing::NetZoneImpl* netzone_impl;
         if (not actor || simgrid::s4u::Actor::is_maestro()) {
             netzone_impl = s4u::Engine::get_instance()->get_netzone_root()->get_impl();
@@ -271,7 +271,7 @@ namespace simgrid::fsmod {
 
 
     /**
-     * @brief Set the evictability of a file so that it can or cannot be evicted
+     * @brief Set the evictable status of a file so that it can or cannot be evicted
      *        if stored on a partition that implements caching
      * @param full_path: an absolute file path
      * @param evictable: true if the file should be evictable, false if not

diff --git a/src/JBODStorage.cpp b/src/JBODStorage.cpp
@@ -50,7 +50,7 @@ namespace simgrid::fsmod {
 
     s4u::IoPtr JBODStorage::read_async(sg_size_t size) {
         // Determine what to read from each disk
-        sg_size_t read_size = 0;
+        sg_size_t read_size;
         std::vector<s4u::Disk*> targets;
         std::stringstream debug_msg;
 
@@ -138,7 +138,7 @@ namespace simgrid::fsmod {
 
         // Determine what to write on each individual disk according to RAID level and which disk will store the
         // parity block
-        sg_size_t write_size = 0;
+        sg_size_t write_size;
         switch(raid_level_) {
             case RAID::RAID0:
                 write_size = size / num_disks_;
@@ -162,7 +162,7 @@ namespace simgrid::fsmod {
         }
 
         // Compute the parity block (if any)
-        s4u::ExecPtr parity_block_comp = nullptr;
+        s4u::ExecPtr parity_block_comp;
 
         if (raid_level_ == RAID::RAID4 || raid_level_ == RAID::RAID5 || raid_level_ == RAID::RAID6) {
             // Assume 1 flop per byte to write per parity block and two for RAID6.

diff --git a/test/caching_test.cpp b/test/caching_test.cpp
@@ -88,6 +88,39 @@ TEST_F(CachingTest, FIFOBasics)  {
     });
 }
 
+TEST_F(CachingTest, FIFOBasicsWithUnevictableFiles)  {
+    DO_TEST_WITH_FORK([this]() {
+        this->setup_platform();
+        // Create one actor (for this test we could likely do it all in the maestro but what the hell)
+        sg4::Actor::create("TestActor", host_, [this]() {
+            std::shared_ptr<sgfs::File> file;
+            XBT_INFO("Create a 20MB file at /dev/fifo/20mb.txt");
+            ASSERT_NO_THROW(fs_->create_file("/dev/fifo/20mb.txt", "20MB"));
+            XBT_INFO("Making the 20MB file at /dev/fifo/20mb.txt unevictable");
+            ASSERT_THROW(fs_->make_file_evictable("/dev/fifo/bogus.txt", false), sgfs::FileNotFoundException);
+            ASSERT_NO_THROW(fs_->make_file_evictable("/dev/fifo/20mb.txt", false));
+            XBT_INFO("Create a 60MB file at /dev/a/60mb.txt");
+            ASSERT_NO_THROW(fs_->create_file("/dev/fifo/60mb.txt", "60MB"));
+            XBT_INFO("Create a 30MB file at /dev/fifo/60mb.txt");
+            ASSERT_NO_THROW(fs_->create_file("/dev/fifo/30mb.txt", "30MB"));
+            XBT_INFO("Check that files are as they should be");
+            ASSERT_TRUE(fs_->file_exists("/dev/fifo/20mb.txt"));
+            ASSERT_FALSE(fs_->file_exists("/dev/fifo/60mb.txt"));
+            ASSERT_TRUE(fs_->file_exists("/dev/fifo/30mb.txt"));
+            ASSERT_NO_THROW(fs_->make_file_evictable("/dev/fifo/20mb.txt", true));
+            ASSERT_NO_THROW(fs_->create_file("/dev/fifo/60mb.txt", "60MB"));
+            ASSERT_FALSE(fs_->file_exists("/dev/fifo/20mb.txt"));
+            ASSERT_TRUE(fs_->file_exists("/dev/fifo/60mb.txt"));
+            ASSERT_TRUE(fs_->file_exists("/dev/fifo/30mb.txt"));
+
+
+        });
+
+        // Run the simulation
+        ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
+    });
+}
+
 TEST_F(CachingTest, FIFODontEvictOpenFiles) {
     DO_TEST_WITH_FORK([this]() {
         this->setup_platform();

diff --git a/test/register_test.cpp b/test/register_test.cpp
@@ -73,6 +73,7 @@ TEST_F(RegisterTest, RetrieveByActor)  {
                     ASSERT_EQ(accessible_file_systems.size(), 2);
                     ASSERT_NO_THROW(fs = accessible_file_systems["my_fs_0"]);
                     ASSERT_EQ(fs->get_name(), "my_fs_0");
+                    ASSERT_EQ(strcmp(fs->get_cname(), "my_fs_0"), 0);
                     ASSERT_NO_THROW(fs = accessible_file_systems["my_extra_fs"]);
                     ASSERT_EQ(fs->get_name(), "my_extra_fs");
                 } else {