test: check base's read permission in module config

Signed-off-by: Gaoyang Zhang <[email protected]>
blurgyy · Feb 8, 2022 · 69cf18d · 69cf18d
1 parent 8247d78
commit 69cf18d
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 79 deletions.
diff --git a/dt-core/src/config.rs b/dt-core/src/config.rs
@@ -798,6 +798,8 @@ impl LocalGroup {
     ///   3. Wrong type of existing [`target`] path
     ///   4. Path to [`target`] contains readonly parent directory
     ///
+    ///   5. Base is unreadable
+    ///
     /// [group name]: LocalGroup::name
     /// [`base`]: LocalGroup::base
     /// [`target`]: LocalGroup::target
@@ -872,6 +874,15 @@ impl LocalGroup {
         // 1-4
         self._validate_with_fs_query()?;
 
+        // 5. Base is unreadable
+        if self.base.exists() {
+            // Check read permission of `base`
+            if let Err(e) = std::fs::read_dir(&self.base) {
+                log::error!("Could not read base '{}'", self.base.display());
+                return Err(e.into());
+            }
+        }
+
         Ok(())
     }
 }
@@ -1549,6 +1560,65 @@ target = "{}"
             ))
         }
     }
+
+    #[test]
+    fn base_unreadable() -> Result<(), Report> {
+        let base = prepare_file(
+            get_testroot().join("base_unreadable").join("base-but-file"),
+            0o311,
+        )?;
+        if let Err(err) = DTConfig::from_str(&format!(
+            r#"
+[[local]]
+name = "base unreadable (not a directory)"
+base = "{}"
+sources = []
+target = ".""#,
+            base.display(),
+        )) {
+            assert_eq!(
+                err,
+                AppError::IoError("Not a directory (os error 20)".to_owned(),),
+                "{}",
+                err,
+            );
+        } else {
+            return Err(eyre!(
+                "This config should not be loaded because base is not a directory",
+            ));
+        }
+
+        let base = prepare_directory(
+            get_testroot()
+                .join("base_unreadable")
+                .join("base-unreadable"),
+            0o311,
+        )?;
+        if let Err(err) = DTConfig::from_str(&format!(
+            r#"
+[[local]]
+name = "base unreadable (permission denied)"
+base = "{}"
+sources = []
+target = ".""#,
+            base.display(),
+        )) {
+            assert_eq!(
+                err,
+                AppError::IoError(
+                    "Permission denied (os error 13)".to_owned(),
+                ),
+                "{}",
+                err,
+            );
+        } else {
+            return Err(eyre!(
+                "This config should not be loaded because insufficient permissions to base",
+            ));
+        }
+
+        Ok(())
+    }
 }
 
 // Author: Blurgy <[email protected]>

diff --git a/dt-core/src/syncing.rs b/dt-core/src/syncing.rs
@@ -56,18 +56,6 @@ fn expand(config: DTConfig) -> Result<DTConfig> {
             next.base = host_specific_base;
         }
 
-        // Above process does not guarantee the `base` to exist, since a
-        // warning will be emitted later in the expanding process (see
-        // function `expand_recursive()`), just don't attempt to read
-        // non-existent `base` here by first checking its existence.
-        if next.base.exists() {
-            // Check read permission of `base`
-            if let Err(e) = std::fs::read_dir(&next.base) {
-                log::error!("Could not read base '{}'", next.base.display());
-                return Err(e.into());
-            }
-        }
-
         // Check for host-specific `sources`
         let sources: Vec<PathBuf> = original
             .sources
@@ -375,73 +363,6 @@ mod tests {
             get_testroot, prepare_directory, prepare_file,
         };
 
-        #[test]
-        fn base_unreadable() -> Result<(), Report> {
-            let base = prepare_file(
-                get_testroot().join("base_unreadable").join("base-but-file"),
-                0o311,
-            )?;
-            if let Err(err) = expand(
-                DTConfig::from_str(&format!(
-                    r#"
-[[local]]
-name = "base unreadable (not a directory)"
-base = "{}"
-sources = []
-target = ".""#,
-                    base.display(),
-                ))
-                .unwrap(),
-            ) {
-                assert_eq!(
-                    err,
-                    AppError::IoError(
-                        "Not a directory (os error 20)".to_owned(),
-                    ),
-                    "{}",
-                    err,
-                );
-            } else {
-                return Err(eyre!(
-                "This config should not be loaded because base is not a directory",
-            ));
-            }
-
-            let base = prepare_directory(
-                get_testroot()
-                    .join("base_unreadable")
-                    .join("base-unreadable"),
-                0o311,
-            )?;
-            if let Err(err) = expand(
-                DTConfig::from_str(&format!(
-                    r#"
-[[local]]
-name = "base unreadable (permission denied)"
-base = "{}"
-sources = []
-target = ".""#,
-                    base.display(),
-                ))
-                .unwrap(),
-            ) {
-                assert_eq!(
-                    err,
-                    AppError::IoError(
-                        "Permission denied (os error 13)".to_owned(),
-                    ),
-                    "{}",
-                    err,
-                );
-            } else {
-                return Err(eyre!(
-                "This config should not be loaded because insufficient permissions to base",
-            ));
-            }
-
-            Ok(())
-        }
-
         #[test]
         fn unreadable_source() -> Result<(), Report> {
             // setup