From 06705661609682d4985d87d0c1e3dacecc7f2793 Mon Sep 17 00:00:00 2001 From: mhead Date: Sun, 22 Sep 2024 15:48:47 +0530 Subject: [PATCH] mv: tests and minor changes --- src/uu/mv/src/mv.rs | 471 ++++++++++++++++++++------------------- tests/by-util/test_mv.rs | 193 +++++++++++++++- 2 files changed, 433 insertions(+), 231 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 7b24e7e6e5..9749bc96b7 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -11,6 +11,7 @@ use clap::builder::ValueParser; use clap::{crate_version, error::ErrorKind, Arg, ArgAction, ArgMatches, Command}; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use std::collections::HashSet; +use std::env; use std::ffi::OsString; use std::fs::{self, FileType}; use std::io; @@ -19,7 +20,6 @@ use std::os::unix; #[cfg(windows)] use std::os::windows; use std::path::{Path, PathBuf, MAIN_SEPARATOR}; -use std::env; #[cfg(unix)] use unix::fs::FileTypeExt; use uucore::backup_control::{self, source_is_target_backup}; @@ -113,6 +113,7 @@ impl<'a> VerboseContext<'a> { fn new(backup: Option<&'a Path>, pb: Option<&'a MultiProgress>) -> VerboseContext<'a> { VerboseContext { backup, pb } } + fn print_move_file(&self, from: &Path, to: &Path) { let message = match self.backup.as_ref() { Some(path) => format!( @@ -131,6 +132,7 @@ impl<'a> VerboseContext<'a> { None => println!("{message}"), }; } + fn print_copy_file(&self, from: &Path, to: &Path, with_backup_message: bool) { let message = match self.backup.as_ref() { Some(path) if with_backup_message => format!( @@ -772,7 +774,9 @@ fn rename_with_fallback( .and_then(|_| { let res = fs::remove_file(from); if let Some(vc) = vebose_context { - vc.remove_file(from); + if res.is_ok() { + vc.remove_file(from); + } } res })?; @@ -919,19 +923,26 @@ fn move_dir( if !error_occurred { while !moved_entries.is_empty() { let (moved_path, file_type) = moved_entries.pop().unwrap(); - if file_type.is_dir() { - remove(&moved_path)?; - if let Some(vc) = vebose_context.as_ref() { - vc.remove_folder(&moved_path); - } + let res = if file_type.is_dir() { + remove(&moved_path) + } else { + file::remove(&moved_path) + }; + if let Err(err) = res { + error_occurred = true; + show_error!("cannot remove {}: {}", moved_path.quote(), err); } else { - file::remove(&moved_path)?; if let Some(vc) = vebose_context.as_ref() { - vc.remove_file(&moved_path); + if file_type.is_dir() { + vc.remove_folder(&moved_path); + } else { + vc.remove_file(&moved_path); + } } } } - } else { + } + if error_occurred { return Err(MvError::NotAllFilesMoved); } Ok(result) @@ -981,223 +992,223 @@ fn copy_file( } } -// #[cfg(test)] -// mod tests { -// use std::path::PathBuf; -// extern crate fs_extra; -// use super::{copy_file, move_dir}; -// use fs_extra::dir::*; -// use indicatif::{ProgressBar, ProgressStyle}; -// use tempfile::tempdir; - -// // 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()).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()).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()).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 copy_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 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 = copy_file(&file1_path, &path_to, pb.as_ref(), 0).expect("move failed"); - -// assert_eq!(7, result); -// assert!(path_to.exists()); -// if let Some(pb) = pb { -// assert_eq!(pb.position(), 7); -// } -// } -// } -// } +#[cfg(test)] +mod tests { + use std::path::PathBuf; + extern crate fs_extra; + use super::{copy_file, move_dir}; + use fs_extra::dir::*; + use indicatif::{ProgressBar, ProgressStyle}; + use tempfile::tempdir; + + // 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(), None).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(), None).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(), None).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 copy_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 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 = copy_file(&file1_path, &path_to, pb.as_ref(), 0).expect("move failed"); + + assert_eq!(7, result); + assert!(path_to.exists()); + if let Some(pb) = pb { + assert_eq!(pb.position(), 7); + } + } + } +} diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 8ed6689d4c..bca4659ce5 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1628,8 +1628,9 @@ fn test_acl() { mod inter_partition_copying { use crate::common::util::TestScenario; use std::fs::{read_to_string, set_permissions, write}; - use std::os::unix::fs::{symlink, PermissionsExt}; + use std::os::unix::fs::{symlink, FileTypeExt, PermissionsExt}; use tempfile::TempDir; + use uucore::display::Quotable; // Ensure that the copying code used in an inter-partition move preserve dir structure. #[test] fn test_inter_partition_copying_folder() { @@ -1751,6 +1752,196 @@ mod inter_partition_copying { .stderr_contains("inter-device move failed:") .stderr_contains("Permission denied"); } + + #[cfg(unix)] + #[test] + fn test_inter_partition_copying_folder_with_fifo() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("a/b/c"); + at.write("a/b/d", "d"); + at.write("a/b/c/e", "e"); + at.mkfifo("a/b/c/f"); + + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // mv to other partition + scene + .ucmd() + .arg("a") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .succeeds(); + + // make sure that a got removed. + assert!(!at.dir_exists("a")); + + // 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"), + "d" + ); + assert_eq!( + read_to_string(other_fs_tempdir.path().join("a/b/c/e"),) + .expect("Unable to read other_fs_file"), + "e" + ); + assert!(other_fs_tempdir + .path() + .join("a/b/c/f") + .metadata() + .expect("") + .file_type() + .is_fifo()) + } + + #[test] + fn test_inter_partition_copying_folder_with_verbose() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("a/b/c"); + at.write("a/b/d", "d"); + at.write("a/b/c/e", "e"); + + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // mv to other partition + scene + .ucmd() + .arg("-v") + .arg("a") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .succeeds() + .stdout_contains(format!( + "created directory {}", + other_fs_tempdir.path().join("a").to_string_lossy().quote() + )) + .stdout_contains(format!( + "created directory {}", + other_fs_tempdir + .path() + .join("a/b/c") + .to_string_lossy() + .quote() + )) + .stdout_contains(format!( + "copied 'a/b/d' -> {}", + other_fs_tempdir + .path() + .join("a/b/d") + .to_string_lossy() + .quote() + )) + .stdout_contains("removed 'a/b/c/e'") + .stdout_contains("removed directory 'a/b'"); + } + #[test] + fn test_inter_partition_copying_file_with_verbose() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write("a", "file_contents"); + + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // mv to other partition + scene + .ucmd() + .arg("-v") + .arg("a") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .succeeds() + .stdout_contains(format!( + "copied 'a' -> {}", + other_fs_tempdir.path().join("a").to_string_lossy().quote() + )) + .stdout_contains("removed 'a'"); + + at.write("a", "file_contents"); + + // mv to other partition + scene + .ucmd() + .arg("-vb") + .arg("a") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .succeeds() + .stdout_contains(format!( + "copied 'a' -> {} (backup: {})", + other_fs_tempdir.path().join("a").to_string_lossy().quote(), + other_fs_tempdir.path().join("a~").to_string_lossy().quote() + )) + .stdout_contains("removed 'a'"); + } + + #[test] + fn test_inter_partition_copying_folder_without_read_permission_to_subfolders() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("a/b/c"); + at.write("a/b/d", "d"); + at.write("a/b/c/e", "e"); + at.mkdir_all("a/b/f"); + at.write("a/b/f/g", "g"); + at.write("a/b/h", "h"); + at.set_mode("a/b/f", 0); + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // mv to other partition + scene + .ucmd() + .arg("-v") + .arg("a") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .fails() + // check erorr occured + .stderr_contains(format!("cannot access 'a/b/f': Permission denied")); + + // make sure mv kept on going after error + assert_eq!( + read_to_string(other_fs_tempdir.path().join("a/b/h"),) + .expect("Unable to read other_fs_file"), + "h" + ); + + // make sure mv didn't remove src because an error occured + at.dir_exists("a"); + at.file_exists("a/b/h"); + } + + #[test] + fn test_inter_partition_copying_folder_without_write_permission_to_subfolders() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir_all("a/b/c"); + at.write("a/b/d", "d"); + at.write("a/b/c/e", "e"); + at.mkdir_all("a/b/f"); + at.write("a/b/f/g", "g"); + at.write("a/b/h", "h"); + at.set_mode("a/b/f",0o555 ); + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // mv to other partition + scene + .ucmd() + .arg("-v") + .arg("a") + .arg(other_fs_tempdir.path().to_str().unwrap()) + .fails() + // check erorr occured + .stderr_contains(format!("mv: cannot remove 'a': Permission denied")); + + } } #[cfg(unix)]