serviceability-cli: prune User test literals to only asserted fields - #4235
Open
ayushsingh82 wants to merge 1 commit into
Open
serviceability-cli: prune User test literals to only asserted fields#4235ayushsingh82 wants to merge 1 commit into
ayushsingh82 wants to merge 1 commit into
Conversation
test_cli_user_{delete,request_ban,update} construct a User only to
hand it back through the mocked get_user; execute() never reads any
of its fields, so the 22-field literal was pure noise. Replaced with
User::default(), letting the manual Default impl (malbeclabs#4080) do what it
was added for.
First slice of malbeclabs#4084 (cli/src/user/*); the sdk/rs and
doublezero-daemon-cli literals it also names are left for follow-up
PRs, per the issue's own file-by-file guidance.
Contributor
Author
Contributor
Author
|
The only red check here is |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Progress on #4084
Summary of Changes
test_cli_user_delete,test_cli_user_request_ban, andtest_cli_user_updateeach build a full 22-fieldUserliteral only to hand it back through the mockedget_user. None of the three tests assert on any field of that value —execute()only cares thatget_userreturns a user so the command can proceed todelete_user/update_user/request_ban_user.User::default()(the manualimpl Default for Useradded in serviceability: change user feed_pk to vec of feed_pks #4080), with a comment noting why no field is load-bearing.This is a first, narrow slice of #4084 — scoped to
smartcontract/cli/src/user/*. The issue also namessmartcontract/sdk/rsandcrates/doublezero-daemon-cli, which have their ownUserliterals (some with fields that likely are asserted on and need per-test judgment, per the issue's caveat aboutUser::default()'s non-neutral values). Left those for follow-up PRs rather than one large mechanical sweep, per the issue's own guidance.Diff Breakdown
Pure test cleanup — no production code touched.
Testing Verification
cargo test --lib user::insmartcontract/cli: all 29 user-module tests pass, including the 3 changed.cargo clippy --lib -- -Dclippy::all -D warnings: clean.uservalue it constructs — only its presence (viaget_userreturningOk) drives the command under test.