refactor(test): prefer cfg_attr(ignore) over cfg for conditional test skipping (#369)

* 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

* 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

* docs(guide/dev): use generic `pdu_test_skip_*` name in example

https://claude.ai/code/session_0184ceZayyCwvK7PEF9iB7K3

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: KSXGitHub

.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. 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.
+
+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
+#[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)
+#[cfg(unix)]
+#[test]
+fn block_size() { /* uses GetBlockSize which only exists on unix */ }
+
+// Good — test compiles with the flag, skipped at runtime
+#[test]
+#[cfg_attr(pdu_test_skip_some_test, ignore = "pdu_test_skip_some_test is set")]
+fn some_test() { /* ... */ }
+
+// Bad — excludes the test from compilation entirely when it could still compile
+#[cfg(not(pdu_test_skip_some_test))]
+#[test]
+fn some_test() { /* ... */ }
+```
+
 ### 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

@@ -70,6 +70,7 @@ impl Api for MockedApi {
 }
 
 #[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,6 +80,7 @@ fn remove_nothing() {
 }
 
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_duplicated_arguments() {
     let original = dbg!(vec![
         "foo",
@@ -110,6 +112,7 @@ fn remove_duplicated_arguments() {
 }
 
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_overlapping_sub_paths() {
     let original = vec![
         "foo/child",
@@ -127,6 +130,7 @@ fn remove_overlapping_sub_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();
@@ -162,6 +166,7 @@ fn remove_all_except_current_dir() {
 }
 
 #[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();
@@ -185,6 +190,7 @@ fn remove_all_except_parent_dir() {
 }
 
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn remove_overlapping_real_paths() {
     let original = dbg!(vec![
         "foo",
@@ -227,6 +233,7 @@ fn remove_overlapping_real_paths() {
 }
 
 #[test]
+#[cfg_attr(not(unix), ignore = "only one path separator style is tested")]
 fn do_not_remove_symlinks() {
     let original = dbg!(vec![
         "foo",

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,8 +106,8 @@ fn max_depth_0() {
 }
 
 #[cfg(unix)]
-#[cfg(not(pdu_test_skip_fs_errors))]
 #[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!(