Add comprehensive readonly file regression tests for cp #9045

naoNao89 · 2025-10-27T09:19:10Z

Fixes issue #5349

The tests ensure the readonly file behavior from PR #5261 cannot regress in future development.

sylvestre · 2025-10-27T09:39:40Z

please remove the trivial comments :)

github-actions · 2025-10-27T10:04:29Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

- Add 10 new test functions covering readonly destination behavior - Tests cover basic readonly copying, flag combinations, and edge cases - Include macOS-specific clonefile behavior tests - Ensure readonly file protection from PR #5261 cannot regress - Tests provide evidence for closing issue #5349

codspeed-hq · 2025-10-27T10:25:56Z

CodSpeed Performance Report

Merging #9045 will degrade performances by 2.59%

_{Comparing naoNao89:add-cp-readonly-regression-tests (112f646) with main (5c5494a)}

Summary

❌ 2 regressions
✅ 103 untouched
⏩ 74 skipped¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
❌	`du_balanced_tree[(5, 4, 10)]`	9.1 ms	9.3 ms	-2.59%
❌	`du_human_balanced_tree[(5, 4, 10)]`	10.2 ms	10.5 ms	-2.5%

74 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions · 2025-10-27T10:35:25Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

- Reduce file I/O overhead by batching file operations - Consolidate setup operations to minimize system calls - Improve test execution time from 0.44s to 0.27s (38% improvement) - Maintain comprehensive test coverage for readonly file behavior

github-actions · 2025-10-27T11:57:09Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

tests/by-util/test_cp.rs

cakebaker · 2025-10-28T08:44:44Z

tests/by-util/test_cp.rs

+#[cfg(not(windows))]
+#[test]
+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");
+
+    ts.ucmd()
+        .args(&["--force", "source.txt", "readonly_dest.txt"])
+        .succeeds();
+
+    assert_eq!(at.read("readonly_dest.txt"), "source content");
+}


This test is more or less the same as test_cp_arg_force on line 941. I would keep this test as it is cleaner and remove the other one.

I'm sorry, my comment was unclear :| What I meant is to keep this test, test_cp_readonly_dest_with_force, and to remove test_cp_arg_force.

cakebaker · 2025-10-28T08:51:06Z

tests/by-util/test_cp.rs

+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");
+
+    ts.ucmd()
+        .args(&["--remove-destination", "source.txt", "readonly_dest.txt"])
+        .succeeds();
+
+    assert_eq!(at.read("readonly_dest.txt"), "source content");
+}


Same here: it's more or less the same as test_cp_arg_remove_destination on line 967. And I would also keep this test and remove the other one.

Same here, my comment was unclear, I'm sorry. Here I also meant to keep test_cp_readonly_dest_with_remove_destination and to remove test_cp_arg_remove_destination.

tests/by-util/test_cp.rs

github-actions · 2025-10-28T11:06:53Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

- 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_macos_clonefile_readonly (duplicate of test_cp_existing_target) - Remove test_cp_normal_copy_still_works (duplicate of test_cp_existing_target) - Remove trivial performance comments from readonly tests - Keep existing proven tests per maintainer preferences - Keep unique readonly tests that provide additional coverage

github-actions · 2025-10-28T11:47:59Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

naoNao89 force-pushed the add-cp-readonly-regression-tests branch from 78e2aa7 to b8d7d4a Compare October 27, 2025 10:13