Skip to content

Commit d4a9774

Browse files
KSXGitHubclaude
andauthored
test: replace custom cfg with TEST_SKIP environment variable (#372)
* 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 * 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 * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 51e784d commit d4a9774

File tree

9 files changed

+18
-22
lines changed

9 files changed

+18
-22
lines changed

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
1616
- Minimize `unwrap()` in non-test code — use proper error handling
1717
- 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)
1818
- Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
19-
- 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.
19+
- 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.
2020
- **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.
2121
- 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

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
1616
- Minimize `unwrap()` in non-test code — use proper error handling
1717
- 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)
1818
- Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
19-
- 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.
19+
- 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.
2020
- **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.
2121
- 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

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
1616
- Minimize `unwrap()` in non-test code — use proper error handling
1717
- 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)
1818
- Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
19-
- 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.
19+
- 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.
2020
- **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.
2121
- 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
2222
- `gh` (GitHub CLI) is not installed — do not attempt to use it

CONTRIBUTING.md

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ pub enum RuntimeError {
215215

216216
### Conditional Test Skipping: `#[cfg]` vs `#[cfg_attr(..., ignore)]`
217217

218-
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.
218+
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.
219219

220220
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.
221221

@@ -231,16 +231,6 @@ fn unix_path_logic() { /* uses hardcoded unix paths but no unix-only types */ }
231231
#[cfg(unix)]
232232
#[test]
233233
fn block_size() { /* uses GetBlockSize which only exists on unix */ }
234-
235-
// Good — test compiles with the flag, skipped at runtime
236-
#[test]
237-
#[cfg_attr(pdu_test_skip_some_test, ignore = "pdu_test_skip_some_test is set")]
238-
fn some_test() { /* ... */ }
239-
240-
// Bad — excludes the test from compilation entirely when it could still compile
241-
#[cfg(not(pdu_test_skip_some_test))]
242-
#[test]
243-
fn some_test() { /* ... */ }
244234
```
245235

246236
### Using `pipe-trait`
@@ -380,4 +370,4 @@ FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh
380370
> 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.
381371
382372
> [!NOTE]
383-
> Some tests may fail with a hint about `RUSTFLAGS` and `--cfg pdu_test_skip_*` flags. Follow the hint and rerun with the suggested flags.
373+
> Some tests may fail with a hint about `TEST_SKIP`. Follow the hint and rerun with the suggested variable.

Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,3 @@ maplit = "1.0.2"
9191
normalize-path = "0.2.1"
9292
pretty_assertions = "1.4.1"
9393
rand = "0.10.0"
94-
95-
[lints.rust]
96-
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(pdu_test_skip_fs_errors)'] }

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ Environment Variables
9393
| `TEST` | `true` or `false` | `true` | Whether to run `cargo test` |
9494
| `BUILD_FLAGS` | string | _(empty)_ | Space-separated list of flags for `cargo build` |
9595
| `TEST_FLAGS` | string | _(empty)_ | Space-separated list of flags for `cargo test` |
96+
| `TEST_SKIP` | string | _(empty)_ | Space-separated list of test names to skip |
9697

9798
</details>
9899

template/ai-instructions/shared.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c
1616
- Minimize `unwrap()` in non-test code — use proper error handling
1717
- 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)
1818
- Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy`
19-
- 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.
19+
- 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.
2020
- **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.
2121
- 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

test.sh

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,19 @@ run_if() (
5656
unit() (
5757
read -ra build_flags <<<"${BUILD_FLAGS:-}"
5858
read -ra test_flags <<<"${TEST_FLAGS:-}"
59+
read -ra test_skip <<<"${TEST_SKIP:-}"
60+
skip_args=()
61+
for name in ${test_skip[@]+"${test_skip[@]}"}; do
62+
skip_args+=(--skip "$name")
63+
done
5964
run_if "${LINT:-true}" cargo clippy "$@" -- -D warnings
6065
run_if "${DOC:-false}" cargo doc "$@"
6166
run_if "${BUILD:-true}" cargo build ${build_flags[@]+"${build_flags[@]}"} "$@"
62-
run_if "${TEST:-true}" cargo test ${test_flags[@]+"${test_flags[@]}"} "$@"
67+
if [[ ${#skip_args[@]} -gt 0 ]]; then
68+
run_if "${TEST:-true}" cargo test ${test_flags[@]+"${test_flags[@]}"} "$@" -- "${skip_args[@]}"
69+
else
70+
run_if "${TEST:-true}" cargo test ${test_flags[@]+"${test_flags[@]}"} "$@"
71+
fi
6372
)
6473

6574
run_if "${FMT:-true}" cargo fmt -- --check

tests/cli_errors.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,12 @@ fn max_depth_0() {
107107

108108
#[cfg(unix)]
109109
#[test]
110-
#[cfg_attr(pdu_test_skip_fs_errors, ignore = "pdu_test_skip_fs_errors is set")]
111110
fn fs_errors() {
112111
if unsafe { libc::geteuid() } == 0 {
113112
panic!(
114113
"{}\n{}",
115114
"error: This test must not be run as root because running with elevated privileges would affect its accuracy.",
116-
"hint: Either run this test as a non-root user or set `RUSTFLAGS='--cfg pdu_test_skip_fs_errors'` to skip this test.",
115+
"hint: Either run this test as a non-root user or rerun via `TEST_SKIP='fs_errors' ./test.sh` to skip this test.",
117116
);
118117
}
119118

0 commit comments

Comments
 (0)