refactor(test): address review feedback on ignore attributes

- Change ignore reason to "only one path separator style is tested"
- Reorder attributes: place #[cfg_attr(..., ignore)] below #[test]
- Revert hint message in fs_errors back to "skip"
- Soften "Always" to "Prefer" for reason strings in CONTRIBUTING.md
- Rephrase CONTRIBUTING.md to mention tests are "still compiled"

https://claude.ai/code/session_0184ceZayyCwvK7PEF9iB7K3

Author: claude

CONTRIBUTING.md

@@ -215,16 +215,16 @@ pub enum RuntimeError {
 
 ### Conditional Test Skipping: `#[cfg]` vs `#[cfg_attr(..., ignore)]`
 
-When a test cannot run under certain conditions (e.g., wrong platform, running as root), prefer `#[cfg_attr(..., ignore)]` over `#[cfg(...)]` to skip it. `#[ignore]` keeps the test compiled on all configurations, catching type errors and regressions early — the test is simply skipped at runtime.
+When a test cannot run under certain conditions (e.g., wrong platform, running as root), prefer `#[cfg_attr(..., ignore)]` over `#[cfg(...)]` to skip it. This way the test is still compiled on all configurations — catching type errors and regressions early — but simply skipped at runtime.
 
 Use `#[cfg]` on tests **only** when the code cannot compile under the condition — for example, when the test references types, functions, or trait methods that are gated behind `#[cfg]` and do not exist on other platforms or feature sets.
 
-Always include a reason string in the `ignore` attribute to explain why the test is skipped.
+Prefer including a reason string in the `ignore` attribute to explain why the test is skipped.
 
 ```rust
 // Good — test compiles everywhere, skipped at runtime on non-unix
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn unix_path_logic() { /* uses hardcoded unix paths but no unix-only types */ }
 
 // Good — test CANNOT compile on non-unix (uses unix-only types)
@@ -233,8 +233,8 @@ fn unix_path_logic() { /* uses hardcoded unix paths but no unix-only types */ }
 fn block_size() { /* uses GetBlockSize which only exists on unix */ }
 
 // Good — test compiles with the flag, skipped at runtime
-#[cfg_attr(pdu_test_skip_fs_errors, ignore = "pdu_test_skip_fs_errors is set")]
 #[test]
+#[cfg_attr(pdu_test_skip_fs_errors, ignore = "pdu_test_skip_fs_errors is set")]
 fn fs_errors() { /* ... */ }
 
 // Bad — excludes the test from compilation entirely when it could still compile

src/app/overlapping_arguments/test_remove_overlapping_paths.rs

@@ -69,8 +69,8 @@ impl Api for MockedApi {
     }
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_nothing() {
     let original = vec!["foo", "bar", "abc/def", "0/1/2"];
     let mut actual = original.clone();
@@ -79,8 +79,8 @@ fn remove_nothing() {
     assert_eq!(actual, expected);
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_duplicated_arguments() {
     let original = dbg!(vec![
         "foo",
@@ -111,8 +111,8 @@ fn remove_duplicated_arguments() {
     assert_eq!(actual, expected);
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_overlapping_sub_paths() {
     let original = vec![
         "foo/child",
@@ -129,8 +129,8 @@ fn remove_overlapping_sub_paths() {
     assert_eq!(actual, expected);
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_all_except_current_dir() {
     let original = dbg!(vec!["foo", "bar", ".", "abc/def", "0/1/2"]);
     let mut actual = original.clone();
@@ -165,8 +165,8 @@ fn remove_all_except_current_dir() {
     assert_eq!(actual, expected);
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_all_except_parent_dir() {
     let original = dbg!(vec!["foo", "bar", "..", "abc/def", ".", "0/1/2"]);
     let mut actual = original.clone();
@@ -189,8 +189,8 @@ fn remove_all_except_parent_dir() {
     assert_eq!(actual, expected);
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_overlapping_real_paths() {
     let original = dbg!(vec![
         "foo",
@@ -232,8 +232,8 @@ fn remove_overlapping_real_paths() {
     assert_eq!(actual, expected);
 }
 
-#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn do_not_remove_symlinks() {
     let original = dbg!(vec![
         "foo",

tests/cli_errors.rs

@@ -106,14 +106,14 @@ fn max_depth_0() {
 }
 
 #[cfg(unix)]
-#[cfg_attr(pdu_test_skip_fs_errors, ignore = "pdu_test_skip_fs_errors is set")]
 #[test]
+#[cfg_attr(pdu_test_skip_fs_errors, ignore = "pdu_test_skip_fs_errors is set")]
 fn fs_errors() {
     if unsafe { libc::geteuid() } == 0 {
         panic!(
             "{}\n{}",
             "error: This test must not be run as root because running with elevated privileges would affect its accuracy.",
-            "hint: Either run this test as a non-root user or set `RUSTFLAGS='--cfg pdu_test_skip_fs_errors'` to ignore this test.",
+            "hint: Either run this test as a non-root user or set `RUSTFLAGS='--cfg pdu_test_skip_fs_errors'` to skip this test.",
         );
     }