refactor(test): replace #[cfg] with #[cfg_attr(..., ignore)] for test skipping

Use `#[cfg_attr(..., ignore = "reason")]` instead of `#[cfg(not(...))]` on
tests that can still compile under the skipped condition. This keeps the
tests compiled on all configurations, catching type errors early while
only skipping execution at runtime.

Changes:
- cli_errors::fs_errors: replace `#[cfg(not(pdu_test_skip_fs_errors))]`
  with `#[cfg_attr(pdu_test_skip_fs_errors, ignore)]`; remove the same
  gate from its imports and helper function
- test_remove_overlapping_paths: remove `#[cfg(unix)]` from module
  declaration; add `#[cfg_attr(not(unix), ignore)]` to each test
- Add "Conditional Test Skipping" section to CONTRIBUTING.md
- Add guideline bullet to AI instruction templates

https://claude.ai/code/session_0184ceZayyCwvK7PEF9iB7K3

Author: claude

.github/copilot-instructions.md

@@ -14,6 +14,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
 - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated
 - Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors
 - Minimize `unwrap()` in non-test code — use proper error handling
+- Prefer `#[cfg_attr(..., ignore = "reason")]` over `#[cfg(...)]` to skip tests — use `#[cfg]` on tests only when the code cannot compile under the condition (e.g., references types/functions that don't exist on other platforms)
 - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
 - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `RUSTFLAGS` and `--cfg pdu_test_skip_*`, follow the hint and rerun with the suggested flags.
 - **ALWAYS run the full test suite** (`FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh`) before committing, regardless of how trivial the change seems — this includes documentation-only changes, comment edits, config changes, and refactors. The test suite checks formatting, linting, building, tests, and docs across multiple feature combinations; any type of change can break any of these checks.

AGENTS.md

@@ -14,6 +14,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
 - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated
 - Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors
 - Minimize `unwrap()` in non-test code — use proper error handling
+- Prefer `#[cfg_attr(..., ignore = "reason")]` over `#[cfg(...)]` to skip tests — use `#[cfg]` on tests only when the code cannot compile under the condition (e.g., references types/functions that don't exist on other platforms)
 - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
 - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `RUSTFLAGS` and `--cfg pdu_test_skip_*`, follow the hint and rerun with the suggested flags.
 - **ALWAYS run the full test suite** (`FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh`) before committing, regardless of how trivial the change seems — this includes documentation-only changes, comment edits, config changes, and refactors. The test suite checks formatting, linting, building, tests, and docs across multiple feature combinations; any type of change can break any of these checks.

CLAUDE.md

@@ -14,6 +14,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
 - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated
 - Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors
 - Minimize `unwrap()` in non-test code — use proper error handling
+- Prefer `#[cfg_attr(..., ignore = "reason")]` over `#[cfg(...)]` to skip tests — use `#[cfg]` on tests only when the code cannot compile under the condition (e.g., references types/functions that don't exist on other platforms)
 - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
 - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `RUSTFLAGS` and `--cfg pdu_test_skip_*`, follow the hint and rerun with the suggested flags.
 - **ALWAYS run the full test suite** (`FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh`) before committing, regardless of how trivial the change seems — this includes documentation-only changes, comment edits, config changes, and refactors. The test suite checks formatting, linting, building, tests, and docs across multiple feature combinations; any type of change can break any of these checks.

CONTRIBUTING.md

@@ -213,6 +213,36 @@ 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.
+
+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.
+
+```rust
+// Good — test compiles everywhere, skipped at runtime on non-unix
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
+#[test]
+fn unix_path_logic() { /* uses hardcoded unix paths but no unix-only types */ }
+
+// Good — test CANNOT compile on non-unix (uses unix-only types)
+#[cfg(unix)]
+#[test]
+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]
+fn fs_errors() { /* ... */ }
+
+// Bad — excludes the test from compilation entirely when it could still compile
+#[cfg(not(pdu_test_skip_fs_errors))]
+#[test]
+fn fs_errors() { /* ... */ }
+```
+
 ### Using `pipe-trait`
 
 This codebase uses the [`pipe-trait`](https://docs.rs/pipe-trait) crate extensively. The `Pipe` trait enables method-chaining through unary functions, keeping code in a natural left-to-right reading order. Import it as `use pipe_trait::Pipe;`.

src/app/overlapping_arguments.rs

@@ -121,6 +121,5 @@ pub fn remove_items_from_vec_by_indices<Item>(vec: &mut Vec<Item>, indices: &Has
 
 #[cfg(test)]
 mod test_remove_items_from_vec_by_indices;
-#[cfg(unix)]
 #[cfg(test)]
 mod test_remove_overlapping_paths;

src/app/overlapping_arguments/test_remove_overlapping_paths.rs

@@ -69,6 +69,7 @@ impl Api for MockedApi {
     }
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn remove_nothing() {
     let original = vec!["foo", "bar", "abc/def", "0/1/2"];
@@ -78,6 +79,7 @@ fn remove_nothing() {
     assert_eq!(actual, expected);
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn remove_duplicated_arguments() {
     let original = dbg!(vec![
@@ -109,6 +111,7 @@ fn remove_duplicated_arguments() {
     assert_eq!(actual, expected);
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn remove_overlapping_sub_paths() {
     let original = vec![
@@ -126,6 +129,7 @@ fn remove_overlapping_sub_paths() {
     assert_eq!(actual, expected);
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn remove_all_except_current_dir() {
     let original = dbg!(vec!["foo", "bar", ".", "abc/def", "0/1/2"]);
@@ -161,6 +165,7 @@ fn remove_all_except_current_dir() {
     assert_eq!(actual, expected);
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn remove_all_except_parent_dir() {
     let original = dbg!(vec!["foo", "bar", "..", "abc/def", ".", "0/1/2"]);
@@ -184,6 +189,7 @@ fn remove_all_except_parent_dir() {
     assert_eq!(actual, expected);
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn remove_overlapping_real_paths() {
     let original = dbg!(vec![
@@ -226,6 +232,7 @@ fn remove_overlapping_real_paths() {
     assert_eq!(actual, expected);
 }
 
+#[cfg_attr(not(unix), ignore = "test uses hardcoded unix paths")]
 #[test]
 fn do_not_remove_symlinks() {
     let original = dbg!(vec![

template/ai-instructions/shared.md

@@ -14,6 +14,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
 - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated
 - Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors
 - Minimize `unwrap()` in non-test code — use proper error handling
+- Prefer `#[cfg_attr(..., ignore = "reason")]` over `#[cfg(...)]` to skip tests — use `#[cfg]` on tests only when the code cannot compile under the condition (e.g., references types/functions that don't exist on other platforms)
 - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
 - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `RUSTFLAGS` and `--cfg pdu_test_skip_*`, follow the hint and rerun with the suggested flags.
 - **ALWAYS run the full test suite** (`FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh`) before committing, regardless of how trivial the change seems — this includes documentation-only changes, comment edits, config changes, and refactors. The test suite checks formatting, linting, building, tests, and docs across multiple feature combinations; any type of change can break any of these checks.

tests/cli_errors.rs

@@ -10,10 +10,8 @@ use std::process::{Command, Output, Stdio};
 use text_block_macros::text_block;
 
 #[cfg(unix)]
-#[cfg(not(pdu_test_skip_fs_errors))]
 use maplit::btreeset;
 #[cfg(unix)]
-#[cfg(not(pdu_test_skip_fs_errors))]
 use parallel_disk_usage::{
     bytes_format::BytesFormat,
     data_tree::DataTree,
@@ -25,7 +23,6 @@ use parallel_disk_usage::{
     visualizer::{BarAlignment, ColumnWidthDistribution, Direction, Visualizer},
 };
 #[cfg(unix)]
-#[cfg(not(pdu_test_skip_fs_errors))]
 use std::{collections::BTreeSet, path::Path};
 
 fn stdio(command: Command) -> Command {
@@ -36,7 +33,6 @@ fn stdio(command: Command) -> Command {
 }
 
 #[cfg(unix)]
-#[cfg(not(pdu_test_skip_fs_errors))]
 fn fs_permission(path: impl AsRef<Path>, permission: &'static str, recursive: bool) {
     let Output { status, stderr, .. } = Command::new("chmod")
         .pipe(|cmd| if recursive { cmd.with_arg("-R") } else { cmd })
@@ -110,14 +106,14 @@ fn max_depth_0() {
 }
 
 #[cfg(unix)]
-#[cfg(not(pdu_test_skip_fs_errors))]
+#[cfg_attr(pdu_test_skip_fs_errors, ignore = "pdu_test_skip_fs_errors is set")]
 #[test]
 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 skip this test.",
+            "hint: Either run this test as a non-root user or set `RUSTFLAGS='--cfg pdu_test_skip_fs_errors'` to ignore this test.",
         );
     }