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.

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

Multi test example #137

Merged
merged 5 commits into from
Nov 16, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 2 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions contracts/cw20-escrow/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,5 @@ thiserror = { version = "1.0.20" }

[dev-dependencies]
cosmwasm-schema = { version = "0.12.0-alpha2" }
cw-multi-test = { path = "../../packages/multi-test", version = "0.3.2" }
cw20-base = { path = "../cw20-base", version = "0.3.2", features = ["library"] }
141 changes: 141 additions & 0 deletions contracts/cw20-escrow/src/integration_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
#![cfg(test)]

use cosmwasm_std::testing::{mock_env, MockApi, MockStorage};
use cosmwasm_std::{coins, to_binary, HumanAddr, Uint128};
use cw20::{Cw20CoinHuman, Cw20Contract, Cw20HandleMsg};
use cw_multi_test::{Contract, ContractWrapper, Router, SimpleBank};

use crate::msg::{CreateMsg, DetailsResponse, HandleMsg, InitMsg, QueryMsg, ReceiveMsg};

fn mock_router() -> Router {
Copy link
Contributor

Choose a reason for hiding this comment

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

From an usability point of view, the name Router is a little confusing to me. I understand this is just the (mock) blockchain. Or, a high level description of it? I'm not sure about this, but maybe MockChain, something like that?

Something that makes clear this is a framework to write better / more versatile integration tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good Point. I will rename it.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about BaseApp or Application? That is what they call the equivalent in the Cosmos SDK.

The object that holds all state and the various handlers.

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 Application or App is better than Router. In the end, it's a matter of documenting it.

let env = mock_env();
let api = Box::new(MockApi::default());
let bank = SimpleBank {};

Router::new(api, env.block, bank, || Box::new(MockStorage::new()))
}

pub fn contract_escrow() -> Box<dyn Contract> {
let contract = ContractWrapper::new(
crate::contract::handle,
crate::contract::init,
crate::contract::query,
);
Box::new(contract)
}
maurolacy marked this conversation as resolved.
Show resolved Hide resolved

pub fn contract_cw20() -> Box<dyn Contract> {
let contract = ContractWrapper::new(
cw20_base::contract::handle,
cw20_base::contract::init,
cw20_base::contract::query,
);
Box::new(contract)
}

#[test]
// receive cw20 tokens and release upon approval
fn escrow_happy_path_cw20_tokens() {
let mut router = mock_router();

// set personal balance
let owner = HumanAddr::from("owner");
let init_funds = coins(2000, "btc");
router
.set_bank_balance(owner.clone(), init_funds.clone())
maurolacy marked this conversation as resolved.
Show resolved Hide resolved
.unwrap();

// set up cw20 contract with some tokens
let cw20_id = router.store_code(contract_cw20());
let msg = cw20_base::msg::InitMsg {
name: "Cash Money".to_string(),
symbol: "CASH".to_string(),
decimals: 2,
initial_balances: vec![Cw20CoinHuman {
address: owner.clone(),
amount: Uint128(5000),
}],
mint: None,
};
let cash_addr = router
.instantiate_contract(cw20_id, &owner, &msg, &[], "CASH")
.unwrap();

// set up reflect contract
let escrow_id = router.store_code(contract_escrow());
let escrow_addr = router
.instantiate_contract(escrow_id, &owner, &InitMsg {}, &[], "Escrow")
.unwrap();

// they are different
assert_ne!(cash_addr, escrow_addr);

// set up cw20 helpers
let cash = Cw20Contract(cash_addr.clone());
Copy link
Contributor

@maurolacy maurolacy Nov 14, 2020

Choose a reason for hiding this comment

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

This was confusing to me.
Maybe Cw20Addr would be a better name? Or even, following the usual api helpers: deps.api.cw20_address().

Update: I see now this is more than an address (more like a collection of helper functions?). Maybe what was confusing was the instantiation from an HumanAddr. Will revisit later, in any case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you propose better naming or docs for this struct? We will be using this pattern a lot more and this should not be confusing.

I would be happy for a PR or an issue with a clearly proposed design. I think these helpers are only used in this new multi-test

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I'll think about this, and play with it using the multi-test code base, to see if I can come up with a better name / design.


// ensure our balances
let owner_balance = cash.balance(&router, owner.clone()).unwrap();
assert_eq!(owner_balance, Uint128(5000));
let escrow_balance = cash.balance(&router, escrow_addr.clone()).unwrap();
assert_eq!(escrow_balance, Uint128(0));

// send some tokens to create an escrow
let arb = HumanAddr::from("arbiter");
let ben = HumanAddr::from("beneficiary");
let id = "demo".to_string();
let create_msg = ReceiveMsg::Create(CreateMsg {
id: id.clone(),
arbiter: arb.clone(),
recipient: ben.clone(),
end_height: None,
end_time: None,
cw20_whitelist: None,
});
let create_bin = to_binary(&create_msg).unwrap();
let send_msg = Cw20HandleMsg::Send {
contract: escrow_addr.clone(),
amount: Uint128(1200),
msg: Some(create_bin),
};
let res = router
.execute_contract(&cash_addr, &owner, &send_msg, &[])
maurolacy marked this conversation as resolved.
Show resolved Hide resolved
.unwrap();
println!("{:?}", res.attributes);
assert_eq!(6, res.attributes.len());

// ensure balances updated
let owner_balance = cash.balance(&router, owner.clone()).unwrap();
Copy link
Contributor

@maurolacy maurolacy Nov 14, 2020

Choose a reason for hiding this comment

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

This is totally accessory, but maybe doing a query for the balance here using router would be clearer. I mean, given that this is testing the integration, it would be nice to test the queries too.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does the query via router.

I really need to rename/document the cash: CW20Contract type better. Rather than manually building QueryRequests for this contract, cash.balance will do that for you, a short-cut for 6 lines or so of boilerplate.

All the actual work happens via the router, but this provides a nice helper wrapper.

We could add one explicit query to show that. But mainly I think we need to start documenting and improving naming of these contract-specific helpers. That would make it much easier for all the tests to just call methods rather than build up the raw request and message types.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. Some documentation / usage examples will solve this. Explicitly mentioning that these are wrappers / helpers on top of Router / App is a good idea.

assert_eq!(owner_balance, Uint128(3800));
let escrow_balance = cash.balance(&router, escrow_addr.clone()).unwrap();
assert_eq!(escrow_balance, Uint128(1200));

// ensure escrow properly created
let details: DetailsResponse = router
.wrap()
Copy link
Contributor

@maurolacy maurolacy Nov 14, 2020

Choose a reason for hiding this comment

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

It would be nice if this could be avoided, i.e. handled internally or so. In any case, get_wrapper() or get_querier() would be a better name? This is exposing something, not wrapping it, AFAIK.

Copy link
Member Author

Choose a reason for hiding this comment

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

router.as_querier_wrapper() would be the most explicit. I just felt it was too verbose.

I am happy for better naming, but take a look at some of the big changes in cosmwasm-std for 0.12 when we took in dyn Querier and such instead of generics. (And I am happy for better approaches/naming).

Basically, the Querier trait only required one method (raw_query) and had a lot of default generic implementations. As I made this &dyn Querier, we could no longer include generics and defaults on the trait. You can see how QuerierWrapper assumed most of the functionality that used to be in Querier. And can easily be created from a Querier.

I am not sure what a better way to handle this is. I cannot see how to use "deref" magic here to allow router.query_wasm_smart(...) directly, which is what I wanted. Please take a look at that wrapper struct (now you understand why) and I am happy for a concrete suggestion to make this cleaner.

Copy link
Contributor

@maurolacy maurolacy Nov 17, 2020

Choose a reason for hiding this comment

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

I see, thanks for the explanation about the need for Wrapper. Naming is tricky... will play with the code base to see if I can propose something concrete.

.query_wasm_smart(&escrow_addr, &QueryMsg::Details { id: id.clone() })
maurolacy marked this conversation as resolved.
Show resolved Hide resolved
.unwrap();
assert_eq!(id, details.id);
assert_eq!(arb, details.arbiter);
assert_eq!(ben, details.recipient);
assert_eq!(
vec![Cw20CoinHuman {
address: cash_addr.clone(),
amount: Uint128(1200)
}],
details.cw20_balance
);

// release escrow
let approve_msg = HandleMsg::Approve { id: id.clone() };
let _ = router
.execute_contract(&escrow_addr, &arb, &approve_msg, &[])
.unwrap();

// ensure balances updated - release to ben
let owner_balance = cash.balance(&router, owner.clone()).unwrap();
assert_eq!(owner_balance, Uint128(3800));
let escrow_balance = cash.balance(&router, escrow_addr.clone()).unwrap();
assert_eq!(escrow_balance, Uint128(0));
let ben_balance = cash.balance(&router, ben.clone()).unwrap();
assert_eq!(ben_balance, Uint128(1200));
}
1 change: 1 addition & 0 deletions contracts/cw20-escrow/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod contract;
mod error;
mod integration_test;
pub mod msg;
pub mod state;

Expand Down
34 changes: 20 additions & 14 deletions packages/multi-test/src/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,33 @@ type QueryFn<T, E> = fn(deps: Deps, env: Env, msg: T) -> Result<Binary, E>;

/// Wraps the exported functions from a contract and provides the normalized format
/// TODO: Allow to customize return values (CustomMsg beyond Empty)
/// TODO: Allow different error types?
pub struct ContractWrapper<T1, T2, T3, E>
pub struct ContractWrapper<T1, T2, T3, E1, E2, E3>
where
T1: DeserializeOwned,
T2: DeserializeOwned,
T3: DeserializeOwned,
E: std::fmt::Display,
E1: std::fmt::Display,
E2: std::fmt::Display,
E3: std::fmt::Display,
{
handle_fn: ContractFn<T1, HandleResponse, E>,
init_fn: ContractFn<T2, InitResponse, E>,
query_fn: QueryFn<T3, E>,
handle_fn: ContractFn<T1, HandleResponse, E1>,
init_fn: ContractFn<T2, InitResponse, E2>,
query_fn: QueryFn<T3, E3>,
}

impl<T1, T2, T3, E> ContractWrapper<T1, T2, T3, E>
impl<T1, T2, T3, E1, E2, E3> ContractWrapper<T1, T2, T3, E1, E2, E3>
where
T1: DeserializeOwned,
T2: DeserializeOwned,
T3: DeserializeOwned,
E: std::fmt::Display,
E1: std::fmt::Display,
E2: std::fmt::Display,
E3: std::fmt::Display,
{
pub fn new(
handle_fn: ContractFn<T1, HandleResponse, E>,
init_fn: ContractFn<T2, InitResponse, E>,
query_fn: QueryFn<T3, E>,
handle_fn: ContractFn<T1, HandleResponse, E1>,
init_fn: ContractFn<T2, InitResponse, E2>,
query_fn: QueryFn<T3, E3>,
) -> Self {
ContractWrapper {
handle_fn,
Expand All @@ -68,12 +71,14 @@ where
}
}

impl<T1, T2, T3, E> Contract for ContractWrapper<T1, T2, T3, E>
impl<T1, T2, T3, E1, E2, E3> Contract for ContractWrapper<T1, T2, T3, E1, E2, E3>
where
T1: DeserializeOwned,
T2: DeserializeOwned,
T3: DeserializeOwned,
E: std::fmt::Display,
E1: std::fmt::Display,
E2: std::fmt::Display,
E3: std::fmt::Display,
{
fn handle(
&self,
Expand Down Expand Up @@ -311,7 +316,8 @@ impl<'a> WasmCache<'a> {
// TODO: better addr generation
fn next_address(&self) -> HumanAddr {
let count = self.router.contracts.len() + self.state.contracts.len();
HumanAddr::from(count.to_string())
// we make this longer so it is not rejected by tests
HumanAddr::from("Contract #".to_string() + &count.to_string())
}

pub fn handle(
Expand Down