Skip to content

fix: resolve clippy warnings, broken doctest, and add unit tests#180

Open
Nicknamess96 wants to merge 1 commit intoopentensor:stagingfrom
Nicknamess96:fix/clippy-doctest-tests
Open

fix: resolve clippy warnings, broken doctest, and add unit tests#180
Nicknamess96 wants to merge 1 commit intoopentensor:stagingfrom
Nicknamess96:fix/clippy-doctest-tests

Conversation

@Nicknamess96
Copy link

Summary

Fix compiler warnings, a broken doctest, and add comprehensive unit tests to the bittensor-wallet Rust library.

Changes

  • Fix 5 mismatched_lifetime_syntaxes warnings in PyO3 binding functions by adding explicit '_ lifetime annotations to Cow<[u8]> return types
  • Fix broken doctest in Wallet::create that caused cargo test --doc to fail by converting Python-style docstring to idiomatic Rust /// # Arguments format
  • Apply cargo fmt to resolve pre-existing formatting inconsistencies in python_bindings.rs
  • Add 66 unit tests across 5 modules (config, keypair, keyfile, wallet, utils), increasing test coverage from 1 to 67 total tests

Testing

  • cargo test --lib passes (67/67 tests)
  • cargo test --doc passes (0 failures, previously 1 failure)
  • cargo fmt --check clean
  • cargo build produces 0 lifetime warnings (previously 5)

@basfroman basfroman changed the base branch from main to staging February 25, 2026 19:04
Copy link
Collaborator

@basfroman basfroman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls resolve the conflicts after rebasing to staging

- Fix 5 mismatched_lifetime_syntaxes warnings in PyO3 bindings
- Fix broken doctest in Wallet::create by converting Python-style
  docstring to idiomatic Rust doc comment format
- Apply cargo fmt to resolve pre-existing formatting inconsistencies
- Add 66 unit tests across config, keypair, keyfile, wallet, and utils
  modules (from 1 to 67 total tests)
- All tests pass: cargo test --lib (67/67), cargo test --doc (0 failures)
@Nicknamess96 Nicknamess96 force-pushed the fix/clippy-doctest-tests branch from cbebf94 to 6b1ea3d Compare February 25, 2026 20:19
@Nicknamess96
Copy link
Author

Rebased onto staging and resolved conflicts — ready for re-review.

@basfroman basfroman added run-bittensor-sdk-tests Runs Bittensor SDK tests. run-bittensor-cli-tests Runs BTCLI tests. labels Feb 25, 2026
@Nicknamess96
Copy link
Author

Hey @basfroman — just a heads-up on the cargo test failure: the error is a pyo3 linker issue (undefined symbol: Py_InitializeEx, _Py_NoneStruct, etc.), not related to the changes in this PR.

The check-rust.yml workflow runs cargo test --workspace --all-features, which enables the pyo3 feature and requires Python development headers to link. But the CI job doesn't install python3-dev, so the linker can't resolve the Python symbols.

This appears to have been introduced when the workflow was simplified in #177 (the old cargo-check-no-python job was removed).

The newer CI runs after my rebase are also pending your approval (action_required for first-time contributors). Could you approve those when you get a chance? Thanks!

@Nicknamess96
Copy link
Author

Just to confirm — I checked the latest Check Rust CI run on the staging branch itself (run #22327290507), and it also fails cargo test with the same pyo3 linker errors. In fact, staging has more failures than this PR (clippy, fmt, and zepter also fail there).

This PR actually fixes the clippy and doctest issues — the only remaining failure (cargo test) is inherited from staging's broken CI config.

Happy to help with a fix for the CI workflow too if that would be useful!

@Nicknamess96
Copy link
Author

Hey @basfroman — rebased onto staging and all conflicts are resolved. The cargo test failure is a pre-existing pyo3 linker issue that also fails on staging itself. Ready for re-review whenever you get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-bittensor-cli-tests Runs BTCLI tests. run-bittensor-sdk-tests Runs Bittensor SDK tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants