Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switching from String to anyhow::Error for error type in multi-test #389

Merged
merged 2 commits into from
Aug 19, 2021
+357 −234
Merged

Switching from String to anyhow::Error for error type in multi-test #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
163b345
Switching from String to anyhow::Error for type erased error type in …
hashedone Aug 17, 2021
267ad38
Aliasing anyhow::Result to AnyResult in multitest
hashedone Aug 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 92 additions & 14 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 23 additions & 17 deletions contracts/cw3-flex-multisig/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ mod tests {
None,
)
.unwrap_err();
assert_eq!(err, ContractError::ZeroThreshold {}.to_string());
assert_eq!(ContractError::ZeroThreshold {}, err.downcast().unwrap());
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼 I think this is more robust. Probably more efficient too.


// Total weight less than required weight not allowed
let instantiate_msg = InstantiateMsg {
Expand All @@ -635,7 +635,10 @@ mod tests {
None,
)
.unwrap_err();
assert_eq!(err, ContractError::UnreachableThreshold {}.to_string());
assert_eq!(
ContractError::UnreachableThreshold {},
err.downcast().unwrap()
);

// All valid
let instantiate_msg = InstantiateMsg {
Expand Down Expand Up @@ -703,7 +706,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(SOMEBODY), flex_addr.clone(), &proposal, &[])
.unwrap_err();
assert_eq!(err, ContractError::Unauthorized {}.to_string());
assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());

// Wrong expiration option fails
let msgs = match proposal.clone() {
Expand All @@ -724,7 +727,7 @@ mod tests {
&[],
)
.unwrap_err();
assert_eq!(err, ContractError::WrongExpiration {}.to_string());
assert_eq!(ContractError::WrongExpiration {}, err.downcast().unwrap());

// Proposal from voter works
let res = app
Expand Down Expand Up @@ -920,13 +923,13 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(OWNER), flex_addr.clone(), &yes_vote, &[])
.unwrap_err();
assert_eq!(ContractError::AlreadyVoted {}.to_string(), err);
assert_eq!(ContractError::AlreadyVoted {}, err.downcast().unwrap());

// Only voters can vote
let err = app
.execute_contract(Addr::unchecked(SOMEBODY), flex_addr.clone(), &yes_vote, &[])
.unwrap_err();
assert_eq!(ContractError::Unauthorized {}.to_string(), err);
assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());

// But voter1 can
let res = app
Expand Down Expand Up @@ -971,14 +974,14 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(VOTER3), flex_addr.clone(), &yes_vote, &[])
.unwrap_err();
assert_eq!(ContractError::AlreadyVoted {}.to_string(), err);
assert_eq!(ContractError::AlreadyVoted {}, err.downcast().unwrap());

// Expired proposals cannot be voted
app.update_block(expire(voting_period));
let err = app
.execute_contract(Addr::unchecked(VOTER4), flex_addr.clone(), &yes_vote, &[])
.unwrap_err();
assert_eq!(ContractError::Expired {}.to_string(), err);
assert_eq!(ContractError::Expired {}, err.downcast().unwrap());
app.update_block(unexpire(voting_period));

// Powerful voter supports it, so it passes
Expand All @@ -999,7 +1002,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(VOTER5), flex_addr.clone(), &yes_vote, &[])
.unwrap_err();
assert_eq!(ContractError::NotOpen {}.to_string(), err);
assert_eq!(ContractError::NotOpen {}, err.downcast().unwrap());

// query individual votes
// initial (with 0 weight)
Expand Down Expand Up @@ -1073,7 +1076,10 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(OWNER), flex_addr.clone(), &execution, &[])
.unwrap_err();
assert_eq!(ContractError::WrongExecuteStatus {}.to_string(), err);
assert_eq!(
ContractError::WrongExecuteStatus {},
err.downcast().unwrap()
);

// Vote it, so it passes
let vote = ExecuteMsg::Vote {
Expand All @@ -1098,7 +1104,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(OWNER), flex_addr.clone(), &closing, &[])
.unwrap_err();
assert_eq!(ContractError::WrongCloseStatus {}.to_string(), err);
assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap());

// Execute works. Anybody can execute Passed proposals
let res = app
Expand Down Expand Up @@ -1128,7 +1134,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(OWNER), flex_addr, &closing, &[])
.unwrap_err();
assert_eq!(ContractError::WrongCloseStatus {}.to_string(), err);
assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap());
}

#[test]
Expand Down Expand Up @@ -1159,7 +1165,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(SOMEBODY), flex_addr.clone(), &closing, &[])
.unwrap_err();
assert_eq!(ContractError::NotExpired {}.to_string(), err);
assert_eq!(ContractError::NotExpired {}, err.downcast().unwrap());

// Expired proposals can be closed
app.update_block(expire(voting_period));
Expand All @@ -1180,7 +1186,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(SOMEBODY), flex_addr, &closing, &[])
.unwrap_err();
assert_eq!(ContractError::WrongCloseStatus {}.to_string(), err);
assert_eq!(ContractError::WrongCloseStatus {}, err.downcast().unwrap());
}

// uses the power from the beginning of the voting period
Expand Down Expand Up @@ -1289,7 +1295,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(newbie), flex_addr.clone(), &yes_vote, &[])
.unwrap_err();
assert_eq!(ContractError::Unauthorized {}.to_string(), err);
assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());

// previously removed VOTER3 can still vote, passing the proposal
app.execute_contract(Addr::unchecked(VOTER3), flex_addr.clone(), &yes_vote, &[])
Expand Down Expand Up @@ -1420,7 +1426,7 @@ mod tests {
&[],
)
.unwrap_err();
assert_eq!(ContractError::Unauthorized {}.to_string(), err);
assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());
Copy link
Member

Choose a reason for hiding this comment

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

I need to look at docs for downcast(), but it reads nicely.

How does it know the type? Does it work with matches!? Like matches!(err.downcast().unwrap(), ContractError::InvalidAddr{..})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Downcast is basically generic with signature fn downcast<T: Any>(self) -> T;, and you can call it just like err.downcast::<Error>(). Here it is just type elision - assert_eq internally uses PartiallEq, so type elision checks all PartiallEq implementations of ContractError. It turns out that there is only one such implementation, so right side have to be of proper type (here ContractError).

Note - because of how it worsk, the downcasted error have to be on the righ side of == - because left-side PartialEq implementation is used. If error is on left side, type elision need to solve query "Find any type which have implementation of PartialEq<ContractError>" which is semantically impossible, as anyone may add such implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see it does runtime checks...

I tried:

        let unwrapped = err.downcast::<&str>().unwrap();
        assert_eq!("unauthorized", unwrapped);

and got

thread 'contract::tests::execute_group_changes_from_proposal' panicked at 'called `Result::unwrap()` on an `Err` value: Unauthorized', contracts/cw3-flex-multisig/src/contract.rs:1429:48

Anyway, makes better sense to me how this works in practice. Code is fine, just my learning the new lib.


// extra: ensure no one else can call the hook
let hook_hack = ExecuteMsg::MemberChangedHook(MemberChangedHookMsg {
Expand All @@ -1429,7 +1435,7 @@ mod tests {
let err = app
.execute_contract(Addr::unchecked(VOTER2), flex_addr.clone(), &hook_hack, &[])
.unwrap_err();
assert_eq!(ContractError::Unauthorized {}.to_string(), err);
assert_eq!(ContractError::Unauthorized {}, err.downcast().unwrap());
}

// uses the power from the beginning of the voting period
Expand Down
3 changes: 3 additions & 0 deletions packages/multi-test/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ documentation = "https://docs.cosmwasm.com"
default = ["iterator"]
iterator = ["cosmwasm-std/iterator"]
stargate = ["cosmwasm-std/stargate"]
backtrace = ["anyhow/backtrace"]

[dependencies]
cw0 = { path = "../../packages/cw0", version = "0.8.0" }
Expand All @@ -24,3 +25,5 @@ itertools = "0.10.1"
schemars = "0.8.1"
serde = { version = "1.0.103", default-features = false, features = ["derive"] }
prost = "0.8.0"
anyhow = "1"
thiserror = "1"
Loading