Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enables the assertions_on_result_states clippy lint across the workspace to improve test code quality. The lint warns against using assert!(result.is_ok()) or assert!(result.is_err()) patterns, which don't provide useful error information when tests fail. Instead, the PR updates these assertions to use .unwrap(), .expect(), .unwrap_err(), or .expect_err() with descriptive messages, and in some cases uses assert_matches! for more complex pattern matching.
Changes:
- Added workspace-level clippy lint configuration for
assertions_on_result_statesin rootCargo.toml - Added
[lints] workspace = trueto all crate-levelCargo.tomlfiles - Replaced
assert!(result.is_ok())patterns with.expect()or.unwrap()with descriptive messages - Replaced
assert!(result.is_err())patterns with.expect_err()or.unwrap_err()with descriptive messages - Introduced
assert_matches!macro usage in contract and attestation tests for more expressive error pattern matching - Added
assert_matchesdependency to relevant crates
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Added workspace-level clippy lint configuration for assertions_on_result_states |
| Cargo.lock | Updated to reflect new assert_matches dependencies |
| crates/*/Cargo.toml | Added [lints] workspace = true to inherit workspace lint settings; added assert_matches dependency where needed |
| crates/node/src/tee/remote_attestation.rs | Replaced assertion with .expect() for better error messages |
| crates/node/src/storage.rs | Replaced .is_ok() assertions with .expect() calls |
| crates/node/src/p2p.rs | Replaced .is_err() timeout assertions with .unwrap_err() |
| crates/node/src/network.rs | Replaced result state assertions with .expect() and .expect_err() |
| crates/node/src/migration_service/web/test_utils.rs | Replaced assertion with .expect() |
| crates/node/src/migration_service/web/authentication.rs | Replaced assertions with .expect_err() and .expect() |
| crates/node/src/migration_service/web.rs | Replaced assertions with .expect_err() |
| crates/node/src/migration_service/onboarding.rs | Replaced assertions with .expect_err() |
| crates/node/src/keyshare/permanent.rs | Replaced assertions with .expect_err() |
| crates/node/src/keyshare.rs | Replaced assertions with .expect(), .expect_err(), and .unwrap_err() |
| crates/node/src/indexer/participants.rs | Replaced assertion with .expect_err() |
| crates/node/src/db.rs | Replaced assertions with .expect_err() |
| crates/node/src/config.rs | Replaced assertions with .expect() |
| crates/node/src/cli.rs | Replaced assertions with .expect() |
| crates/node/src/assets.rs | Replaced assertions with .expect_err() |
| crates/include-measurements/src/lib.rs | Replaced assertions with .expect_err() and simplified error checking |
| crates/devnet/Cargo.toml | Added workspace lints configuration |
| crates/contract/src/tee/tee_state.rs | Replaced assertion with .expect() |
| crates/contract/src/state/running.rs | Replaced assertions with .unwrap_err() |
| crates/contract/src/state/resharing.rs | Replaced assertions with .unwrap(), .unwrap_err(), and .expect() |
| crates/contract/src/state/initializing.rs | Replaced assertions with .unwrap(), .unwrap_err(), and .expect() |
| crates/contract/src/primitives/thresholds.rs | Replaced assertions with .unwrap_err(), .unwrap(), and .expect() |
| crates/contract/src/primitives/participants.rs | Replaced assertions with .unwrap() and .expect() |
| crates/contract/src/primitives/key_state.rs | Replaced assertions with .unwrap_err(), .unwrap(), and .expect() |
| crates/contract/src/primitives/domain.rs | Replaced assertions with .unwrap_err() |
| crates/contract/src/lib.rs | Replaced assertions with .expect(), .expect_err(), and introduced assert_matches! usage |
| crates/backup-cli/src/adapters/keyshare_storage.rs | Replaced assertions with .expect_err() |
| crates/attestation/tests/collateral.rs | Introduced assert_matches! for error pattern matching, replaced one assertion with .expect_err() |
| crates/attestation/src/tcb_info.rs | Replaced assertions with .expect_err() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nt-assertions_on_result_states
gilcu3
left a comment
There was a problem hiding this comment.
LGTM, left a few comments
closes #1882