test(ls): cover alias recursive listing mode#75
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds focused unit coverage for the rc ls <alias>/ --recursive mode-selection behavior by extracting the alias-only branching logic into a dedicated helper, ensuring the recent user-visible ls behavior change is guarded by tests.
Changes:
- Refactors alias-only listing selection into a small
alias_listing_modehelper. - Adds unit tests covering alias-only recursive vs non-recursive behavior and ensuring bucket-qualified paths bypass the alias-only branch.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn alias_listing_mode(bucket: Option<&String>, recursive: bool) -> Option<AliasListingMode> { | ||
| if bucket.is_some() { | ||
| return None; | ||
| } | ||
|
|
||
| if recursive { | ||
| Some(AliasListingMode::AllObjects) |
There was a problem hiding this comment.
alias_listing_mode takes Option<&String>, which unnecessarily couples the helper to String and forces callers to pass bucket.as_ref(). Consider taking Option<&str> instead (and call with bucket.as_deref()), since the function only needs to know whether a bucket is present and this matches other helpers in the CLI that accept &str/Option<&str>.
Summary
A recent
lsbehavior change maderc ls <alias>/ --recursivetraverse every bucket and list objects server-wide, but that alias-only recursive branch had no direct unit coverage. That left the command selection logic unguarded even though it changed user-visible behavior.User Impact
If that branch regresses,
ls alias/ -rcan silently fall back to bucket listing instead of object listing. Users would still get successful output, but it would be the wrong mode and the regression would be easy to miss.Root Cause
The branch that distinguishes alias-only bucket listing from alias-only recursive object listing lived inline inside
execute, so the changed path was only indirectly exercised. Existing tests covered path parsing, but not the mode decision introduced by the recent-rbehavior change.Fix
This PR extracts the alias-only listing decision into a small
alias_listing_modehelper and adds focused unit tests for the three relevant cases:--recursiveselects all-object listing--recursiveselects bucket listingThe implementation behavior is unchanged; the refactor is only there to make the recent branch testable and keep the scope tight to the changed area.
Validation
I ran the required checks locally:
cargo fmt --all --checkcargo clippy --workspace -- -D warningscargo test --workspace