fix(cucumber): fix claim burn ownership proof validation in cucumber tests#1981
fix(cucumber): fix claim burn ownership proof validation in cucumber tests#1981Stackwyre wants to merge 1 commit intotari-project:developmentfrom
Conversation
|
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the wallet daemon integration tests, replacing legacy step definitions with updated implementations that utilize the JSON-RPC client and improved error handling. Feedback focuses on replacing several instances of .unwrap() with proper error propagation using the ? operator to prevent potential panics during test execution, specifically when accessing validator nodes or performing type conversions for amounts.
| .validator_nodes | ||
| .values() | ||
| .next() | ||
| .unwrap() |
There was a problem hiding this comment.
Using .unwrap() here can cause a panic if validator_nodes is empty. Since this function returns a Result, it's better to propagate the error using ? to provide a more informative failure message.
| .unwrap() | |
| .ok_or_else(|| anyhow!("No validator nodes found in the world state"))? |
| from_account: from_account.account.address, | ||
| to_account: to_account.account.address, | ||
| max_fee: None, | ||
| amount: Amount::new(amount.try_into().unwrap()), |
There was a problem hiding this comment.
The use of .unwrap() here can cause the test to panic if the u64 amount cannot be converted to an i64. It's safer to propagate the error using the ? operator, as this function already returns a Result.
| amount: Amount::new(amount.try_into().unwrap()), | |
| amount: Amount::new(amount.try_into()?), |
| _ => panic!("Expected gt, gte, lt, lte or eq, got {}", operator), | ||
| let req = AccountsCreateFreeTestCoinsRequest { | ||
| account: None, | ||
| amount: Amount::new(amount.try_into().unwrap()), |
There was a problem hiding this comment.
The use of .unwrap() here can cause the test to panic if the u64 amount cannot be converted to an i64. It's safer to propagate the error using the ? operator, as this function already returns a Result.
| amount: Amount::new(amount.try_into().unwrap()), | |
| amount: Amount::new(amount.try_into()?), |
| .cloned() | ||
| .unwrap_or_default(); | ||
|
|
||
| if balance.balance < Amount::new(amount.try_into().unwrap()) { |
There was a problem hiding this comment.
The use of .unwrap() here can cause the test to panic if the u64 amount cannot be converted to an i64. It's safer to propagate the error using the ? operator, as this function already returns a Result.
| if balance.balance < Amount::new(amount.try_into().unwrap()) { | |
| if balance.balance < Amount::new(amount.try_into()?) { |
| .cloned() | ||
| .unwrap_or_default(); | ||
|
|
||
| if balance.balance != Amount::new(amount.try_into().unwrap()) { |
There was a problem hiding this comment.
The use of .unwrap() here can cause the test to panic if the u64 amount cannot be converted to an i64. It's safer to propagate the error using the ? operator, as this function already returns a Result.
| if balance.balance != Amount::new(amount.try_into().unwrap()) { | |
| if balance.balance != Amount::new(amount.try_into()?) { |
sdbondi
changed the title
2 participants
Resolves #1974
Changes
integration_tests/tests/steps/wallet_daemon.rsFixes #1974
Generated by JARVIS AI — reviewed and tested