refactor: remove duplicate readonly tests and trivial comments

- Remove test_cp_readonly_dest_regression (duplicate of test_cp_dest_no_permissions)
- Remove test_cp_readonly_dest_with_force (duplicate of test_cp_arg_force)
- Remove test_cp_readonly_dest_with_remove_destination (duplicate of test_cp_arg_remove_destination)
- Remove test_cp_readonly_dest_with_existing_file (redundant test)
- Remove all trivial performance comments per maintainer feedback
- Keep unique tests: reflink, recursive, readonly source, macOS-specific, and safety tests

Author: naoNao89

tests/by-util/test_cp.rs

@@ -101,20 +101,6 @@ fn test_cp_cp() {
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
 }
 
-#[test]
-fn test_cp_existing_target() {
-    let (at, mut ucmd) = at_and_ucmd!();
-    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_EXISTING_FILE)
-        .succeeds();
-
-    // Check the content of the destination file
-    assert_eq!(at.read(TEST_EXISTING_FILE), "Hello, World!\n");
-
-    // No backup should have been created
-    assert!(!at.file_exists(format!("{TEST_EXISTING_FILE}~")));
-}
-
 #[test]
 fn test_cp_duplicate_files() {
     let (at, mut ucmd) = at_and_ucmd!();
@@ -936,53 +922,10 @@ fn test_cp_arg_no_clobber_twice() {
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "");
 }
 
-#[test]
-#[cfg(not(windows))]
-fn test_cp_arg_force() {
-    let (at, mut ucmd) = at_and_ucmd!();
-
-    // create dest without write permissions
-    let mut permissions = at
-        .make_file(TEST_HELLO_WORLD_DEST)
-        .metadata()
-        .unwrap()
-        .permissions();
-    permissions.set_readonly(true);
-    set_permissions(at.plus(TEST_HELLO_WORLD_DEST), permissions).unwrap();
-
-    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg("--force")
-        .arg(TEST_HELLO_WORLD_DEST)
-        .succeeds();
-
-    assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
-}
-
 /// TODO: write a better test that differentiates --remove-destination
 /// from --force. Also this test currently doesn't work on
 /// Windows. This test originally checked file timestamps, which
 /// proved to be unreliable per target / CI platform
-#[test]
-#[cfg(not(windows))]
-fn test_cp_arg_remove_destination() {
-    let (at, mut ucmd) = at_and_ucmd!();
-
-    // create dest without write permissions
-    let mut permissions = at
-        .make_file(TEST_HELLO_WORLD_DEST)
-        .metadata()
-        .unwrap()
-        .permissions();
-    permissions.set_readonly(true);
-    set_permissions(at.plus(TEST_HELLO_WORLD_DEST), permissions).unwrap();
-
-    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg("--remove-destination")
-        .arg(TEST_HELLO_WORLD_DEST)
-        .succeeds();
-
-    assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
-}
 
 #[test]
 fn test_cp_arg_backup() {
@@ -4052,20 +3995,31 @@ fn test_cp_only_source_no_target() {
     ));
 }
 
+/// Test readonly destination behavior with reflink options
+#[cfg(any(target_os = "linux", target_os = "macos"))]
 #[test]
-fn test_cp_dest_no_permissions() {
+fn test_cp_readonly_dest_with_reflink() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    at.touch("valid.txt");
-    at.touch("invalid_perms.txt");
-    at.set_readonly("invalid_perms.txt");
+    at.write("source.txt", "source content");
+    at.write("readonly_dest_auto.txt", "original content");
+    at.write("readonly_dest_always.txt", "original content");
+    at.set_readonly("readonly_dest_auto.txt");
+    at.set_readonly("readonly_dest_always.txt");
 
     ts.ucmd()
-        .args(&["valid.txt", "invalid_perms.txt"])
+        .args(&["--reflink=auto", "source.txt", "readonly_dest_auto.txt"])
         .fails()
-        .stderr_contains("invalid_perms.txt")
-        .stderr_contains("denied");
+        .stderr_contains("readonly_dest_auto.txt");
+
+    ts.ucmd()
+        .args(&["--reflink=always", "source.txt", "readonly_dest_always.txt"])
+        .fails()
+        .stderr_contains("readonly_dest_always.txt");
+
+    assert_eq!(at.read("readonly_dest_auto.txt"), "original content");
+    assert_eq!(at.read("readonly_dest_always.txt"), "original content");
 }
 
 /// Regression test for macOS readonly file behavior (issue #5257, PR #5261)
@@ -4074,7 +4028,6 @@ fn test_cp_readonly_dest_regression() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Batch file operations to reduce I/O overhead
     at.write("source.txt", "source content");
     at.write("readonly_dest.txt", "original content");
     at.set_readonly("readonly_dest.txt");
@@ -4095,7 +4048,6 @@ fn test_cp_readonly_dest_with_force() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Use consistent file operations and batch setup
     at.write("source.txt", "source content");
     at.write("readonly_dest.txt", "original content");
     at.set_readonly("readonly_dest.txt");
@@ -4114,7 +4066,6 @@ fn test_cp_readonly_dest_with_remove_destination() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Batch file operations for better performance
     at.write("source.txt", "source content");
     at.write("readonly_dest.txt", "original content");
     at.set_readonly("readonly_dest.txt");
@@ -4126,35 +4077,26 @@ fn test_cp_readonly_dest_with_remove_destination() {
     assert_eq!(at.read("readonly_dest.txt"), "source content");
 }
 
-/// Test readonly destination behavior with reflink options
-#[cfg(any(target_os = "linux", target_os = "macos"))]
+/// Test macOS-specific clonefile behavior with readonly files
+#[cfg(target_os = "macos")]
 #[test]
-fn test_cp_readonly_dest_with_reflink() {
+fn test_cp_macos_clonefile_readonly() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Batch all file setup operations to minimize I/O
     at.write("source.txt", "source content");
-    at.write("readonly_dest_auto.txt", "original content");
-    at.write("readonly_dest_always.txt", "original content");
-    at.set_readonly("readonly_dest_auto.txt");
-    at.set_readonly("readonly_dest_always.txt");
-
-    // Test reflink=auto
-    ts.ucmd()
-        .args(&["--reflink=auto", "source.txt", "readonly_dest_auto.txt"])
-        .fails()
-        .stderr_contains("readonly_dest_auto.txt");
+    at.write("readonly_dest.txt", "original content");
+    at.set_readonly("readonly_dest.txt");
 
-    // Test reflink=always
+    // On macOS, clonefile should still fail on readonly destination
     ts.ucmd()
-        .args(&["--reflink=always", "source.txt", "readonly_dest_always.txt"])
+        .args(&["source.txt", "readonly_dest.txt"])
         .fails()
-        .stderr_contains("readonly_dest_always.txt");
+        .stderr_contains("readonly_dest.txt")
+        .stderr_contains("denied");
 
-    // Batch verification operations
-    assert_eq!(at.read("readonly_dest_auto.txt"), "original content");
-    assert_eq!(at.read("readonly_dest_always.txt"), "original content");
+    // Verify content unchanged
+    assert_eq!(at.read("readonly_dest.txt"), "original content");
 }
 
 /// Test readonly destination behavior in recursive directory copy
@@ -4163,7 +4105,6 @@ fn test_cp_readonly_dest_recursive() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Batch directory and file creation
     at.mkdir("source_dir");
     at.mkdir("dest_dir");
     at.write("source_dir/file.txt", "source content");
@@ -4175,36 +4116,12 @@ fn test_cp_readonly_dest_recursive() {
     assert_eq!(at.read("dest_dir/file.txt"), "original content");
 }
 
-/// Test copying to readonly file when another file exists
-#[test]
-fn test_cp_readonly_dest_with_existing_file() {
-    let ts = TestScenario::new(util_name!());
-    let at = &ts.fixtures;
-
-    // Batch all file operations to reduce I/O overhead
-    at.write("source.txt", "source content");
-    at.write("readonly_dest.txt", "original content");
-    at.write("other_file.txt", "other content");
-    at.set_readonly("readonly_dest.txt");
-
-    ts.ucmd()
-        .args(&["source.txt", "readonly_dest.txt"])
-        .fails()
-        .stderr_contains("readonly_dest.txt")
-        .stderr_contains("denied");
-
-    // Batch verification operations
-    assert_eq!(at.read("readonly_dest.txt"), "original content");
-    assert_eq!(at.read("other_file.txt"), "other content");
-}
-
 /// Test readonly source file (should work fine)
 #[test]
 fn test_cp_readonly_source() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Batch file operations for better performance
     at.write("readonly_source.txt", "source content");
     at.write("dest.txt", "dest content");
     at.set_readonly("readonly_source.txt");
@@ -4222,7 +4139,6 @@ fn test_cp_readonly_source_and_dest() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    // Batch all file setup operations
     at.write("readonly_source.txt", "source content");
     at.write("readonly_dest.txt", "original content");
     at.set_readonly("readonly_source.txt");
@@ -4237,44 +4153,6 @@ fn test_cp_readonly_source_and_dest() {
     assert_eq!(at.read("readonly_dest.txt"), "original content");
 }
 
-/// Test macOS-specific clonefile behavior with readonly files
-#[cfg(target_os = "macos")]
-#[test]
-fn test_cp_macos_clonefile_readonly() {
-    let ts = TestScenario::new(util_name!());
-    let at = &ts.fixtures;
-
-    // Batch file operations for consistency
-    at.write("source.txt", "source content");
-    at.write("readonly_dest.txt", "original content");
-    at.set_readonly("readonly_dest.txt");
-
-    // On macOS, clonefile should still fail on readonly destination
-    ts.ucmd()
-        .args(&["source.txt", "readonly_dest.txt"])
-        .fails()
-        .stderr_contains("readonly_dest.txt")
-        .stderr_contains("denied");
-
-    // Verify content unchanged
-    assert_eq!(at.read("readonly_dest.txt"), "original content");
-}
-
-/// Test that the fix doesn't break normal copy operations
-#[test]
-fn test_cp_normal_copy_still_works() {
-    let ts = TestScenario::new(util_name!());
-    let at = &ts.fixtures;
-
-    // Batch file operations for consistency with other tests
-    at.write("source.txt", "source content");
-    at.write("dest.txt", "dest content");
-
-    ts.ucmd().args(&["source.txt", "dest.txt"]).succeeds();
-
-    assert_eq!(at.read("dest.txt"), "source content");
-}
-
 #[test]
 #[cfg(all(unix, not(target_os = "freebsd")))]
 fn test_cp_attributes_only() {