From 069164371d9c2916a326b5050c3588c87d1a5388 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 24 Mar 2026 03:41:33 +0000 Subject: [PATCH 1/3] refactor(test): replace `pdu_test_skip_fs_errors` cfg with `TEST_SKIP` env var The custom cfg flag required `RUSTFLAGS` which forced full recompilation just to skip a test. The new `TEST_SKIP` env var passes `--skip` to the test binary at runtime, avoiding any recompilation. https://claude.ai/code/session_01UBzLwHNmqEC2SafFKjEqYz --- .github/copilot-instructions.md | 2 +- AGENTS.md | 2 +- CLAUDE.md | 2 +- CONTRIBUTING.md | 16 ++++------------ Cargo.toml | 3 --- README.md | 1 + template/ai-instructions/shared.md | 2 +- test.sh | 11 ++++++++++- tests/cli_errors.rs | 3 +-- 9 files changed, 20 insertions(+), 22 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3cf78ddf..a82456c3 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -16,6 +16,6 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - 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. +- Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `TEST_SKIP`, follow the hint and rerun with the suggested variable. - **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. - Set `PDU_NO_FAIL_FAST=true` to run all checks instead of stopping at the first failure — this lets you see which checks pass and which fail diff --git a/AGENTS.md b/AGENTS.md index 3cf78ddf..a82456c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,6 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - 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. +- Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `TEST_SKIP`, follow the hint and rerun with the suggested variable. - **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. - Set `PDU_NO_FAIL_FAST=true` to run all checks instead of stopping at the first failure — this lets you see which checks pass and which fail diff --git a/CLAUDE.md b/CLAUDE.md index 5aa709ae..f0fc2926 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - 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. +- Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `TEST_SKIP`, follow the hint and rerun with the suggested variable. - **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. - Set `PDU_NO_FAIL_FAST=true` to run all checks instead of stopping at the first failure — this lets you see which checks pass and which fail - `gh` (GitHub CLI) is not installed — do not attempt to use it diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index edbd1049..28b454aa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -215,7 +215,7 @@ 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. +When a test cannot run under certain conditions (e.g., wrong platform), 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. @@ -231,18 +231,10 @@ fn unix_path_logic() { /* uses hardcoded unix paths but no 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() { /* ... */ } ``` +For tests that need to be skipped based on the runtime environment (e.g., running as root), use `TEST_SKIP='test_name'` instead of custom `cfg` flags. This avoids forcing a full recompilation just to skip a 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;`. @@ -380,4 +372,4 @@ FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh > Always run the full test suite before committing, even for seemingly trivial changes such as documentation edits, comment changes, or config updates. Any change can break formatting, linting, building, tests, or doc generation across the different feature combinations. > [!NOTE] -> Some tests may fail with a hint about `RUSTFLAGS` and `--cfg pdu_test_skip_*` flags. Follow the hint and rerun with the suggested flags. +> Some tests may fail with a hint about `TEST_SKIP`. Follow the hint and rerun with the suggested variable. diff --git a/Cargo.toml b/Cargo.toml index 7ba21ff4..ced6e8a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -91,6 +91,3 @@ maplit = "1.0.2" normalize-path = "0.2.1" pretty_assertions = "1.4.1" rand = "0.10.0" - -[lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(pdu_test_skip_fs_errors)'] } diff --git a/README.md b/README.md index 67d3c8fd..12e52aa2 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,7 @@ Environment Variables | `TEST` | `true` or `false` | `true` | Whether to run `cargo test` | | `BUILD_FLAGS` | string | _(empty)_ | Space-separated list of flags for `cargo build` | | `TEST_FLAGS` | string | _(empty)_ | Space-separated list of flags for `cargo test` | +| `TEST_SKIP` | string | _(empty)_ | Space-separated list of test names to skip | diff --git a/template/ai-instructions/shared.md b/template/ai-instructions/shared.md index 3cf78ddf..a82456c3 100644 --- a/template/ai-instructions/shared.md +++ b/template/ai-instructions/shared.md @@ -16,6 +16,6 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - 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. +- Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes. If a test fails with a hint about `TEST_SKIP`, follow the hint and rerun with the suggested variable. - **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. - Set `PDU_NO_FAIL_FAST=true` to run all checks instead of stopping at the first failure — this lets you see which checks pass and which fail diff --git a/test.sh b/test.sh index 3846d798..e0554d02 100755 --- a/test.sh +++ b/test.sh @@ -56,10 +56,19 @@ run_if() ( unit() ( read -ra build_flags <<<"${BUILD_FLAGS:-}" read -ra test_flags <<<"${TEST_FLAGS:-}" + read -ra test_skip <<<"${TEST_SKIP:-}" + skip_args=() + for name in ${test_skip[@]+"${test_skip[@]}"}; do + skip_args+=(--skip "$name") + done run_if "${LINT:-true}" cargo clippy "$@" -- -D warnings run_if "${DOC:-false}" cargo doc "$@" run_if "${BUILD:-true}" cargo build ${build_flags[@]+"${build_flags[@]}"} "$@" - run_if "${TEST:-true}" cargo test ${test_flags[@]+"${test_flags[@]}"} "$@" + if [[ ${#skip_args[@]} -gt 0 ]]; then + run_if "${TEST:-true}" cargo test ${test_flags[@]+"${test_flags[@]}"} "$@" -- "${skip_args[@]}" + else + run_if "${TEST:-true}" cargo test ${test_flags[@]+"${test_flags[@]}"} "$@" + fi ) run_if "${FMT:-true}" cargo fmt -- --check diff --git a/tests/cli_errors.rs b/tests/cli_errors.rs index bb2c089b..39004a31 100644 --- a/tests/cli_errors.rs +++ b/tests/cli_errors.rs @@ -107,13 +107,12 @@ fn max_depth_0() { #[cfg(unix)] #[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 skip this test.", + "hint: Either run this test as a non-root user or set `TEST_SKIP='fs_errors'` to skip this test.", ); } From 295e5ec4157e9a48c014ce7d26d87ecc23660128 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 24 Mar 2026 09:59:18 +0000 Subject: [PATCH 2/3] fix(test): clarify hint to mention ./test.sh for TEST_SKIP The TEST_SKIP variable is only interpreted by test.sh, not by cargo test directly. Update the hint to say "rerun via ./test.sh" so users know exactly how to invoke it. https://claude.ai/code/session_01UBzLwHNmqEC2SafFKjEqYz --- tests/cli_errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli_errors.rs b/tests/cli_errors.rs index 39004a31..f1802d5d 100644 --- a/tests/cli_errors.rs +++ b/tests/cli_errors.rs @@ -112,7 +112,7 @@ fn fs_errors() { 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 `TEST_SKIP='fs_errors'` to skip this test.", + "hint: Either run this test as a non-root user or rerun via `TEST_SKIP='fs_errors' ./test.sh` to skip this test.", ); } From caca715d2f8ed83e1adb95c363c365614e8d5a2c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Mar 2026 01:44:25 +0000 Subject: [PATCH 3/3] docs(contributing): remove redundant TEST_SKIP guideline The paragraph framed TEST_SKIP as a general policy for writing new tests, but there is only one such test. The test's own panic hint and the existing note in the test-running section already provide sufficient guidance. https://claude.ai/code/session_01UBzLwHNmqEC2SafFKjEqYz --- CONTRIBUTING.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 28b454aa..0cba2540 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -233,8 +233,6 @@ fn unix_path_logic() { /* uses hardcoded unix paths but no unix-only types */ } fn block_size() { /* uses GetBlockSize which only exists on unix */ } ``` -For tests that need to be skipped based on the runtime environment (e.g., running as root), use `TEST_SKIP='test_name'` instead of custom `cfg` flags. This avoids forcing a full recompilation just to skip a 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;`.