Skip to content

test(cli): cover rm purge option defaults#104

Merged
overtrue merged 1 commit intomainfrom
codex/rm-purge-option-default-gap
Apr 19, 2026
Merged

test(cli): cover rm purge option defaults#104
overtrue merged 1 commit intomainfrom
codex/rm-purge-option-default-gap

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

@overtrue overtrue commented Apr 9, 2026

Summary

This adds focused unit coverage for the request-option mapping behind the recent rm --purge work.

The merged change in #82 introduced delete_request_options so the CLI can opt into RustFS force-delete semantics, but the unit tests only exercised the positive purge = true branch. That left the default branch and the easy-to-confuse force = true, purge = false branch unverified.

Root cause

Recent follow-up PRs already covered --purge help and parser wiring, plus the S3 client header behavior. The small helper in crates/cli/src/commands/rm.rs still had a gap around the non-purge paths that feed those lower-level calls.

Fix

The test module now adds two narrow assertions:

  • default remove options keep force_delete disabled;
  • --force alone does not enable force_delete unless --purge is also set.

This keeps the scope limited to the recent rm change surface and avoids production code changes.

Validation

I ran these successfully:

  • cargo test -p rustfs-cli delete_request_options --lib
  • cargo fmt --all --check
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace

I also ran make pre-commit per the automation instructions, but this checkout does not define that target, so make exits with No rule to make target 'pre-commit'.

@overtrue overtrue marked this pull request as ready for review April 19, 2026 14:57
@overtrue overtrue merged commit f0c5b77 into main Apr 19, 2026
15 checks passed
@overtrue overtrue deleted the codex/rm-purge-option-default-gap branch April 19, 2026 14:57
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