Skip to content

Lint the test targets, which clippy currently skips - #10

Open
kridaydave wants to merge 1 commit into
mainfrom
k5/pass-taproot-clippy-all-targets
Open

kridaydave wants to merge 1 commit into
mainfrom
k5/pass-taproot-clippy-all-targets

Conversation

@kridaydave

Copy link
Copy Markdown
Contributor

Problem

.github/workflows/ci.yml:29 ran cargo clippy -- -D warnings. That is the same target selection as cargo check, which covers the lib and bin targets only. It never compiled tests/cli.rs (843 lines) or any #[cfg(test)] mod tests block in src/, so all of the test code was unlinted. Cargo.toml has no [lints] table and there is no clippy.toml, so nothing else covered it. cargo fmt --check on the line above does walk test code, so formatting was enforced there and lints were not.

Change

Two flags. --all-targets covers the test targets. --locked matches the flag cargo test on line 32 already uses, so clippy lints the graph CI actually builds rather than a re-resolved one.

The single warning that --all-targets surfaced is fixed in the same change, because -D warnings would otherwise fail the build:

src/fabric.rs:234  assert_eq!(loaded.require_check_strict, false)  ->  assert!(!loaded.require_check_strict)

Verification

I ran the new command on this tree, unmodified otherwise:

$ cargo clippy --all-targets --locked -- -D warnings
    Checking taproot v0.0.1 (/root/code/_watch-worktrees/taproot)
    Finished \`dev\` profile [unoptimized + debuginfo] target(s) in 10.89s

Exit 0, zero warnings. Before the assert! fix, the same command reported exactly one warning, bool_assert_comparison at src/fabric.rs:234, confirming the coverage gap was real rather than theoretical. cargo fmt --all -- --check is clean.

Not shipped

  • src/mount.rs:437 casts a FUSE setattr size (u64) to usize and resizes a buffer with it. A truncate through the mount with a size near u64::MAX reaches a multi-exabyte allocation. I did not reproduce it, so it is a write-up rather than a fix.
  • src/registry.rs:24 bounds a log walk at MAX_LOG_HOPS, and no test reaches that branch. Worth a negative test later.

cargo clippy -- -D warnings covers the lib and bin targets only, so the
843-line tests/cli.rs and every #[cfg(test)] block in src/ were unlinted,
while cargo fmt --check at ci.yml:26 does walk them. Adding --all-targets
closes that asymmetry. --locked matches the flag the test step already
uses.

The one warning that surfaced is fixed in the same change, since it
would otherwise break the build:

src/fabric.rs:234  assert_eq!(x, false) -> assert!(!x)

Ran the new command on this tree. Clean, exit 0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant