From 907a679e8b40145bdd056bb6c359fb3f782a1dd7 Mon Sep 17 00:00:00 2001 From: mhead Date: Thu, 18 Jul 2024 22:59:59 +0530 Subject: [PATCH 1/3] mv: inter partition copying test code cleanup --- tests/by-util/test_mv.rs | 199 ++++++++++++++++++++------------------- 1 file changed, 100 insertions(+), 99 deletions(-) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index db9e27210ad..d77f3764af0 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1623,105 +1623,106 @@ fn test_acl() { // mv: try to overwrite 'b', overriding mode 0444 (r--r--r--)? y // 'a' -> 'b' -// Ensure that the copying code used in an inter-partition move unlinks the destination symlink. #[cfg(target_os = "linux")] -#[test] -fn test_mv_unlinks_dest_symlink() { - let scene = TestScenario::new(util_name!()); - let at = &scene.fixtures; - - // create a file in the current partition. - at.write("src", "src contents"); - - // create a folder in another partition. - let other_fs_tempdir = - tempfile::TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); - - // create a file inside that folder. - let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); - let mut file = - std::fs::File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); - std::io::Write::write_all(&mut file, b"other fs file contents") - .expect("Unable to write to other_fs_file"); - - // create a symlink to the file inside the same directory. - let symlink_path = other_fs_tempdir.path().join("symlink_to_file"); - std::os::unix::fs::symlink(&other_fs_file_path, &symlink_path) - .expect("Unable to create symlink_to_file"); - - // mv src to symlink in another partition - scene - .ucmd() - .arg("src") - .arg(symlink_path.to_str().unwrap()) - .succeeds(); - - // make sure that src got removed. - assert!(!at.file_exists("src")); - - // make sure symlink got unlinked - assert!(!symlink_path.is_symlink()); - - // make sure that file contents in other_fs_file didn't change. - let mut new_contents = String::new(); - std::io::Read::read_to_string( - &mut std::fs::File::open(&other_fs_file_path).expect("Unable to open other_fs_file"), - &mut new_contents, - ) - .expect("Unable to read other_fs_file"); - assert_eq!(new_contents, "other fs file contents"); - - // make sure that src file contents got copied into new file created in symlink_path . - let mut new_contents = String::new(); - std::io::Read::read_to_string( - &mut std::fs::File::open(&symlink_path).expect("Unable to open file"), - &mut new_contents, - ) - .expect("Unable to read file"); - assert_eq!(new_contents, "src contents"); -} - -// In an inter-partition move if unlinking the destination symlink fails, ensure -// that it would output the proper error message. -#[cfg(target_os = "linux")] -#[test] -fn test_mv_unlinks_dest_symlink_error_message() { - let scene = TestScenario::new(util_name!()); - let at = &scene.fixtures; +mod inter_partition_copying { + use crate::common::util::TestScenario; + + use std::fs::{set_permissions, File}; + use std::io::{Read, Write}; + use std::os::unix::fs::{symlink, PermissionsExt}; + use tempfile::TempDir; + + // Ensure that the copying code used in an inter-partition move unlinks the destination symlink. + #[test] + pub(crate) fn test_mv_unlinks_dest_symlink() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // create a file in the current partition. + at.write("src", "src contents"); + + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // create a file inside that folder. + let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); + let mut file = File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); + Write::write_all(&mut file, b"other fs file contents") + .expect("Unable to write to other_fs_file"); + + // create a symlink to the file inside the same directory. + let symlink_path = other_fs_tempdir.path().join("symlink_to_file"); + symlink(&other_fs_file_path, &symlink_path).expect("Unable to create symlink_to_file"); + + // mv src to symlink in another partition + scene + .ucmd() + .arg("src") + .arg(symlink_path.to_str().unwrap()) + .succeeds(); + + // make sure that src got removed. + assert!(!at.file_exists("src")); + + // make sure symlink got unlinked + assert!(!symlink_path.is_symlink()); + + // make sure that file contents in other_fs_file didn't change. + let mut new_contents = String::new(); + Read::read_to_string( + &mut File::open(&other_fs_file_path).expect("Unable to open other_fs_file"), + &mut new_contents, + ) + .expect("Unable to read other_fs_file"); + assert_eq!(new_contents, "other fs file contents"); + + // make sure that src file contents got copied into new file created in symlink_path . + let mut new_contents = String::new(); + Read::read_to_string( + &mut File::open(&symlink_path).expect("Unable to open file"), + &mut new_contents, + ) + .expect("Unable to read file"); + assert_eq!(new_contents, "src contents"); + } - // create a file in the current partition. - at.write("src", "src contents"); - - // create a folder in another partition. - let other_fs_tempdir = - tempfile::TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); - - // create a file inside that folder. - let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); - let mut file = - std::fs::File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); - std::io::Write::write_all(&mut file, b"other fs file contents") - .expect("Unable to write to other_fs_file"); - - // create a symlink to the file inside the same directory. - let symlink_path = other_fs_tempdir.path().join("symlink_to_file"); - std::os::unix::fs::symlink(&other_fs_file_path, &symlink_path) - .expect("Unable to create symlink_to_file"); - - // disable write for the target folder so that when mv tries to remove the - // the destination symlink inside the target directory it would fail. - std::fs::set_permissions( - other_fs_tempdir.path(), - std::os::unix::fs::PermissionsExt::from_mode(0o555), - ) - .expect("Unable to set permissions for temp directory"); - - // mv src to symlink in another partition - scene - .ucmd() - .arg("src") - .arg(symlink_path.to_str().unwrap()) - .fails() - .stderr_contains("inter-device move failed:") - .stderr_contains("Permission denied"); + // In an inter-partition move if unlinking the destination symlink fails, ensure + // that it would output the proper error message. + #[test] + pub(crate) fn test_mv_unlinks_dest_symlink_error_message() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // create a file in the current partition. + at.write("src", "src contents"); + + // create a folder in another partition. + let other_fs_tempdir = + TempDir::new_in("/dev/shm/").expect("Unable to create temp directory"); + + // create a file inside that folder. + let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); + let mut file = File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); + Write::write_all(&mut file, b"other fs file contents") + .expect("Unable to write to other_fs_file"); + + // create a symlink to the file inside the same directory. + let symlink_path = other_fs_tempdir.path().join("symlink_to_file"); + symlink(&other_fs_file_path, &symlink_path).expect("Unable to create symlink_to_file"); + + // disable write for the target folder so that when mv tries to remove the + // the destination symlink inside the target directory it would fail. + set_permissions(other_fs_tempdir.path(), PermissionsExt::from_mode(0o555)) + .expect("Unable to set permissions for temp directory"); + + // mv src to symlink in another partition + scene + .ucmd() + .arg("src") + .arg(symlink_path.to_str().unwrap()) + .fails() + .stderr_contains("inter-device move failed:") + .stderr_contains("Permission denied"); + } } From 4e9c01f1a055403386bac22f22c4a629fa2675b0 Mon Sep 17 00:00:00 2001 From: mhead Date: Fri, 19 Jul 2024 07:40:46 +0530 Subject: [PATCH 2/3] mv: inter partition copying use of read_to_string and write --- tests/by-util/test_mv.rs | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index d77f3764af0..e19f5845a37 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1626,9 +1626,7 @@ fn test_acl() { #[cfg(target_os = "linux")] mod inter_partition_copying { use crate::common::util::TestScenario; - - use std::fs::{set_permissions, File}; - use std::io::{Read, Write}; + use std::fs::{read_to_string, set_permissions, write}; use std::os::unix::fs::{symlink, PermissionsExt}; use tempfile::TempDir; @@ -1647,8 +1645,7 @@ mod inter_partition_copying { // create a file inside that folder. let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); - let mut file = File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); - Write::write_all(&mut file, b"other fs file contents") + write(&other_fs_file_path, "other fs file contents") .expect("Unable to write to other_fs_file"); // create a symlink to the file inside the same directory. @@ -1669,22 +1666,16 @@ mod inter_partition_copying { assert!(!symlink_path.is_symlink()); // make sure that file contents in other_fs_file didn't change. - let mut new_contents = String::new(); - Read::read_to_string( - &mut File::open(&other_fs_file_path).expect("Unable to open other_fs_file"), - &mut new_contents, - ) - .expect("Unable to read other_fs_file"); - assert_eq!(new_contents, "other fs file contents"); - - // make sure that src file contents got copied into new file created in symlink_path . - let mut new_contents = String::new(); - Read::read_to_string( - &mut File::open(&symlink_path).expect("Unable to open file"), - &mut new_contents, - ) - .expect("Unable to read file"); - assert_eq!(new_contents, "src contents"); + assert_eq!( + read_to_string(&other_fs_file_path,).expect("Unable to read other_fs_file"), + "other fs file contents" + ); + + // make sure that src file contents got copied into new file created in symlink_path + assert_eq!( + read_to_string(&symlink_path,).expect("Unable to read other_fs_file"), + "src contents" + ); } // In an inter-partition move if unlinking the destination symlink fails, ensure @@ -1703,8 +1694,7 @@ mod inter_partition_copying { // create a file inside that folder. let other_fs_file_path = other_fs_tempdir.path().join("other_fs_file"); - let mut file = File::create(&other_fs_file_path).expect("Unable to create other_fs_file"); - Write::write_all(&mut file, b"other fs file contents") + write(&other_fs_file_path, "other fs file contents") .expect("Unable to write to other_fs_file"); // create a symlink to the file inside the same directory. From f537fc3e73e14b4f8db2929dd9160562e9c89756 Mon Sep 17 00:00:00 2001 From: sreehari prasad <52113972+matrixhead@users.noreply.github.com> Date: Fri, 19 Jul 2024 22:02:42 +0530 Subject: [PATCH 3/3] Update tests/by-util/test_mv.rs Co-authored-by: Ben Wiederhake --- tests/by-util/test_mv.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index e19f5845a37..82741d5f002 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -1673,7 +1673,7 @@ mod inter_partition_copying { // make sure that src file contents got copied into new file created in symlink_path assert_eq!( - read_to_string(&symlink_path,).expect("Unable to read other_fs_file"), + read_to_string(&symlink_path).expect("Unable to read other_fs_file"), "src contents" ); }