From 1da64ed5920dd23323e71929a7984465dc2695e8 Mon Sep 17 00:00:00 2001 From: mhead Date: Tue, 13 Aug 2024 00:17:41 +0530 Subject: [PATCH] mv: tests and minor changes --- Cargo.lock | 1 + src/uu/mv/Cargo.toml | 3 + src/uu/mv/src/mv.rs | 347 +++++++++++++++++++++++++++++++++++++-- tests/by-util/test_mv.rs | 6 +- 4 files changed, 339 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index abaf5c3106f..4f98c0c858e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2956,6 +2956,7 @@ dependencies = [ "clap", "fs_extra", "indicatif", + "tempfile", "uucore", ] diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index c484d5a77f3..e943ce7bc57 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -27,6 +27,9 @@ uucore = { workspace = true, features = [ "update-control", ] } +[dev-dependencies] +tempfile = { workspace = true } + [[bin]] name = "mv" path = "src/main.rs" diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index b6ac650c24c..ad24ec87d60 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -284,7 +284,7 @@ fn parse_paths(files: &[OsString], opts: &Options) -> Vec { paths.map(|p| p.to_owned()).collect::>() } } - +/// Handles moving files from a source path to a target path based on the provided options. fn handle_two_paths( source: &Path, target: &Path, @@ -662,7 +662,11 @@ fn rename_with_fallback( fs::remove_dir_all(to)?; } // Calculate total size of directory - let total_size = dir_get_size(from).ok(); + let total_size = if multi_progress.is_some() { + dir_get_size(from).ok() + } else { + None + }; let progress_bar = get_pb(total_size); @@ -753,8 +757,8 @@ fn is_empty_dir(path: &Path) -> bool { /// - `moved_files`: A mutable HashMap containing information about moved files. /// /// Returns: -/// - Result: The total number of bytes moved if successful. -pub fn move_dir( +/// - `Result`: The total number of bytes moved if successful. +fn move_dir( from: &Path, to: &Path, progress_bar: Option<&ProgressBar>, @@ -833,12 +837,7 @@ fn move_file( overwrite: true, ..Default::default() }; - // We check whether a source file with the same inode has been moved before. - // If that's the case, instead of moving it, we create a link to the target. - let result_file_move = if let Some(new_source) = moved_files.get(file_info) { - fs::hard_link(new_source, to)?; - Ok(file_info.file_size()) - } else if let Some(progress_bar) = progress_bar { + let result_file_move = if let Some(progress_bar) = progress_bar { let display_file_name = if let Some(file_name) = from.file_name().and_then(|file_name| file_name.to_str()) { file_name.to_string() @@ -848,14 +847,330 @@ fn move_file( "Invalid file name", )); }; - let _progress_handler = |info: file::TransitProcess| { - let copied_bytes = progress_bar_start_val + info.copied_bytes; - progress_bar.set_position(copied_bytes); - progress_bar.set_message(display_file_name.clone()); - }; - file::move_file_with_progress(from, to, ©_options, _progress_handler) + + // We check whether a source file with the same inode has been moved before. + // If that's the case, instead of moving it, we create a link to the target. + if let Some(new_source) = moved_files.get(file_info) { + let file_size = file_info.file_size(); + fs::hard_link(new_source, to)?; + fs::remove_file(from)?; + progress_bar.set_position(file_size); + progress_bar.set_message(display_file_name); + Ok(file_size) + } else { + let _progress_handler = |info: file::TransitProcess| { + let copied_bytes = progress_bar_start_val + info.copied_bytes; + progress_bar.set_position(copied_bytes); + progress_bar.set_message(display_file_name.clone()); + }; + file::move_file_with_progress(from, to, ©_options, _progress_handler) + } + } else if let Some(new_source) = moved_files.get(file_info) { + let file_size = file_info.file_size(); + fs::hard_link(new_source, to)?; + fs::remove_file(from)?; + Ok(file_size) } else { file::move_file(from, to, ©_options) }; + result_file_move } + +#[cfg(test)] +mod tests { + + use std::{collections::HashMap, fs, path::PathBuf}; + + extern crate fs_extra; + use super::{move_dir, move_file}; + use fs_extra::dir::*; + use indicatif::{ProgressBar, ProgressStyle}; + use tempfile::tempdir; + use uucore::fs::FileInformation; + + // These tests are copied from the `fs_extra`'s repository + #[test] + fn it_move_work() { + for with_progress_bar in [false, true] { + let temp_dir = tempdir().unwrap(); + let mut path_from = PathBuf::from(temp_dir.path()); + let test_name = "sub"; + path_from.push("it_move_work"); + let mut path_to = path_from.clone(); + path_to.push("out"); + path_from.push(&test_name); + + create_all(&path_from, true).unwrap(); + assert!(path_from.exists()); + create_all(&path_to, true).unwrap(); + assert!(path_to.exists()); + + let mut file1_path = path_from.clone(); + file1_path.push("test1.txt"); + let content1 = "content1"; + fs_extra::file::write_all(&file1_path, &content1).unwrap(); + assert!(file1_path.exists()); + + let mut sub_dir_path = path_from.clone(); + sub_dir_path.push("sub"); + create(&sub_dir_path, true).unwrap(); + let mut file2_path = sub_dir_path.clone(); + file2_path.push("test2.txt"); + let content2 = "content2"; + fs_extra::file::write_all(&file2_path, &content2).unwrap(); + assert!(file2_path.exists()); + + let pb = if with_progress_bar { + Some( + ProgressBar::new(16).with_style( + ProgressStyle::with_template( + "{msg}: [{elapsed_precise}] {wide_bar} {bytes:>7}/{total_bytes:7}", + ) + .unwrap(), + ), + ) + } else { + None + }; + + let result = move_dir(&path_from, &path_to, pb.as_ref(), &mut HashMap::new()).unwrap(); + + assert_eq!(16, result); + assert!(path_to.exists()); + assert!(!path_from.exists()); + if let Some(pb) = pb { + assert_eq!(pb.position(), 16) + } + } + } + + #[test] + fn it_move_exist_overwrite() { + for with_progress_bar in [false, true] { + let temp_dir = tempdir().unwrap(); + let mut path_from = PathBuf::from(temp_dir.path()); + let test_name = "sub"; + path_from.push("it_move_exist_overwrite"); + let mut path_to = path_from.clone(); + path_to.push("out"); + path_from.push(&test_name); + let same_file = "test.txt"; + + create_all(&path_from, true).unwrap(); + assert!(path_from.exists()); + create_all(&path_to, true).unwrap(); + assert!(path_to.exists()); + + let mut file1_path = path_from.clone(); + file1_path.push(same_file); + let content1 = "content1"; + fs_extra::file::write_all(&file1_path, &content1).unwrap(); + assert!(file1_path.exists()); + + let mut sub_dir_path = path_from.clone(); + sub_dir_path.push("sub"); + create(&sub_dir_path, true).unwrap(); + let mut file2_path = sub_dir_path.clone(); + file2_path.push("test2.txt"); + let content2 = "content2"; + fs_extra::file::write_all(&file2_path, &content2).unwrap(); + assert!(file2_path.exists()); + + let mut exist_path = path_to.clone(); + exist_path.push(&test_name); + create(&exist_path, true).unwrap(); + assert!(exist_path.exists()); + exist_path.push(same_file); + let exist_content = "exist content"; + assert_ne!(exist_content, content1); + fs_extra::file::write_all(&exist_path, exist_content).unwrap(); + assert!(exist_path.exists()); + + let dir_size = get_size(&path_from).expect("failed to get dir size"); + let pb = if with_progress_bar { + Some( + ProgressBar::new(dir_size).with_style( + ProgressStyle::with_template( + "{msg}: [{elapsed_precise}] {wide_bar} {bytes:>7}/{total_bytes:7}", + ) + .unwrap(), + ), + ) + } else { + None + }; + move_dir(&path_from, &path_to, pb.as_ref(), &mut HashMap::new()).unwrap(); + assert!(exist_path.exists()); + assert!(path_to.exists()); + assert!(!path_from.exists()); + if let Some(pb) = pb { + assert_eq!(pb.position(), dir_size) + } + } + } + + #[test] + fn it_move_inside_work_target_dir_not_exist() { + for with_progress_bar in [false, true] { + let temp_dir = tempdir().unwrap(); + let path_root = PathBuf::from(temp_dir.path()); + let root = path_root.join("it_move_inside_work_target_dir_not_exist"); + let root_dir1 = root.join("dir1"); + let root_dir1_sub = root_dir1.join("sub"); + let root_dir2 = root.join("dir2"); + let file1 = root_dir1.join("file1.txt"); + let file2 = root_dir1_sub.join("file2.txt"); + + create_all(&root_dir1_sub, true).unwrap(); + fs_extra::file::write_all(&file1, "content1").unwrap(); + fs_extra::file::write_all(&file2, "content2").unwrap(); + + if root_dir2.exists() { + remove(&root_dir2).unwrap(); + } + + assert!(root_dir1.exists()); + assert!(root_dir1_sub.exists()); + assert!(!root_dir2.exists()); + assert!(file1.exists()); + assert!(file2.exists()); + let dir_size = get_size(&root_dir1).expect("failed to get dir size"); + let pb = if with_progress_bar { + Some( + ProgressBar::new(dir_size).with_style( + ProgressStyle::with_template( + "{msg}: [{elapsed_precise}] {wide_bar} {bytes:>7}/{total_bytes:7}", + ) + .unwrap(), + ), + ) + } else { + None + }; + + let result = + move_dir(&root_dir1, &root_dir2, pb.as_ref(), &mut HashMap::new()).unwrap(); + + assert_eq!(16, result); + assert!(!root_dir1.exists()); + let root_dir2_sub = root_dir2.join("sub"); + let root_dir2_file1 = root_dir2.join("file1.txt"); + let root_dir2_sub_file2 = root_dir2_sub.join("file2.txt"); + assert!(root_dir2.exists()); + assert!(root_dir2_sub.exists()); + assert!(root_dir2_file1.exists()); + assert!(root_dir2_sub_file2.exists()); + if let Some(pb) = pb { + assert_eq!(pb.position(), dir_size) + } + } + } + #[test] + fn move_file_test() { + for with_progress_bar in [false, true] { + let temp_dir = tempdir().unwrap(); + let temp_dir_path = temp_dir.path(); + + let file1_path = temp_dir_path.join("file"); + let content = "content"; + fs_extra::file::write_all(&file1_path, &content).unwrap(); + assert!(file1_path.exists()); + let file_info = FileInformation::from_path(&file1_path, false) + .expect("couldn't get file information"); + + let path_to = temp_dir_path.join("file_out"); + + let pb = if with_progress_bar { + Some( + ProgressBar::new(7).with_style( + ProgressStyle::with_template( + "{msg}: [{elapsed_precise}] {wide_bar} {bytes:>7}/{total_bytes:7}", + ) + .unwrap(), + ), + ) + } else { + None + }; + + let result = move_file( + &file1_path, + &path_to, + &mut HashMap::new(), + &file_info, + pb.as_ref(), + 0, + ) + .expect("move failed"); + + assert_eq!(7, result); + assert!(path_to.exists()); + assert!(!file1_path.exists()); + if let Some(pb) = pb { + assert_eq!(pb.position(), 7) + } + } + } + + #[test] + fn move_file_hardlink_test() { + for with_progress_bar in [false, true] { + let temp_dir = tempdir().unwrap(); + let temp_dir_path = temp_dir.path(); + + let file = temp_dir_path.join("file"); + let content = "content"; + fs_extra::file::write_all(&file, &content).unwrap(); + assert!(file.exists()); + + let link = temp_dir_path.join("link"); + fs::hard_link(&file, &link).expect("couldn't create hardlink"); + + let path_to = temp_dir_path.join("file_out"); + let link_to = temp_dir_path.join("link_to"); + let mut moved_files = HashMap::new(); + + for (from, to) in [(&file, &path_to), (&link, &link_to)] { + let pb = if with_progress_bar { + Some( + ProgressBar::new(7).with_style( + ProgressStyle::with_template( + "{msg}: [{elapsed_precise}] {wide_bar} {bytes:>7}/{total_bytes:7}", + ) + .unwrap(), + ), + ) + } else { + None + }; + + let file_info = FileInformation::from_path(&from, false) + .expect("couldn't get file information"); + let result = move_file(&from, &to, &mut moved_files, &file_info, pb.as_ref(), 0) + .expect("move failed"); + moved_files.insert(file_info, to.to_path_buf()); + + assert_eq!(7, result); + assert!(to.exists()); + assert!(!from.exists()); + if let Some(pb) = pb { + assert_eq!(pb.position(), 7) + } + } + #[cfg(unix)] + { + assert_eq!( + path_to + .metadata() + .map(|v| { std::os::unix::fs::MetadataExt::ino(&v) }) + .ok(), + link_to + .metadata() + .map(|v| { std::os::unix::fs::MetadataExt::ino(&v) }) + .ok(), + ); + } + } + } +} diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 906cb96cf43..f982b3f6008 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1654,11 +1654,13 @@ mod inter_partition_copying { // Ensure that the folder structure is preserved, files are copied, and their contents remain intact. assert_eq!( - read_to_string(other_fs_tempdir.path().join("a/b/d"),).expect("Unable to read other_fs_file"), + read_to_string(other_fs_tempdir.path().join("a/b/d"),) + .expect("Unable to read other_fs_file"), "d" ); assert_eq!( - read_to_string(other_fs_tempdir.path().join("a/b/c/e"),).expect("Unable to read other_fs_file"), + read_to_string(other_fs_tempdir.path().join("a/b/c/e"),) + .expect("Unable to read other_fs_file"), "e" ); }