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

Implementing all messages handling in mutlitest App #398

Merged
merged 5 commits into from
Sep 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
128 changes: 0 additions & 128 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion packages/multi-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,4 @@ serde = { version = "1.0.103", default-features = false, features = ["derive"] }
prost = "0.8.0"
anyhow = "1"
thiserror = "1"
mockall = "0.10"
derivative = "2"
31 changes: 13 additions & 18 deletions packages/multi-test/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::custom_handler::CustomHandler;
use crate::executor::{AppResponse, Executor};
use crate::transactions::transactional;
use crate::wasm::{ContractData, Wasm, WasmKeeper};
use crate::{BankKeeper, MockSimpleCustomHandler};
use crate::{BankKeeper, PanickingCustomHandler};

use anyhow::Result as AnyResult;
use derivative::Derivative;
Expand Down Expand Up @@ -85,7 +85,7 @@ pub struct AppBuilder<ExecC, QueryC> {
impl<ExecC, QueryC> AppBuilder<ExecC, QueryC>
where
ExecC: Debug + PartialEq + Clone + JsonSchema + 'static,
QueryC: CustomQuery + DeserializeOwned + 'static,
QueryC: Debug + CustomQuery + DeserializeOwned + 'static,
{
/// Overwrites default wasm executor. Panic if already set.
#[track_caller]
Expand Down Expand Up @@ -149,7 +149,7 @@ where
let block = self.block.unwrap_or_else(|| mock_env().block);
let custom = self
.custom
.unwrap_or_else(|| Box::new(MockSimpleCustomHandler::new()));
.unwrap_or_else(|| Box::new(PanickingCustomHandler));

let router = Router { wasm, bank, custom };

Expand Down Expand Up @@ -1549,28 +1549,16 @@ mod test {

mod custom_messages {
use super::*;
use mockall as ma;
use crate::custom_handler::CachingCustomHandler;

#[test]
fn triggering_custom_msg() {
let api = MockApi::default();
let sender = api.addr_validate("sender").unwrap();
let owner = api.addr_validate("owner").unwrap();

let mut custom_handler = MockSimpleCustomHandler::<CustomMsg, Empty>::new();
custom_handler
.expect_execute()
.with(
ma::predicate::always(),
ma::predicate::always(),
ma::predicate::eq(CustomMsg::SetAge { age: 20 }),
)
.returning(|_, _, _| {
Ok(AppResponse {
data: None,
events: vec![],
})
});
let custom_handler = CachingCustomHandler::<CustomMsg, Empty>::new();
let custom_handler_state = custom_handler.state();

let mut app = AppBuilder::new()
.with_api(api)
Expand All @@ -1594,6 +1582,13 @@ mod test {
&[],
)
.unwrap();

assert_eq!(
Copy link
Member

Choose a reason for hiding this comment

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

This looks nice.

custom_handler_state.execs().to_owned(),
vec![CustomMsg::SetAge { age: 20 }]
);

assert!(custom_handler_state.queries().is_empty());
}
}
}
81 changes: 75 additions & 6 deletions packages/multi-test/src/custom_handler.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use cosmwasm_std::{Addr, Api, Binary, BlockInfo, Empty, Storage};
use mockall::automock;

use anyhow::Result as AnyResult;
use derivative::Derivative;
use std::cell::{Ref, RefCell};
use std::ops::Deref;
use std::rc::Rc;

use crate::AppResponse;

/// Custom message handler trait. Implementator of this trait is mocking environment behaviour on
/// Custom message handler trait. Implementor of this trait is mocking environment behavior on
/// given custom message.
pub trait CustomHandler<ExecC = Empty, QueryC = Empty> {
fn execute(
Expand All @@ -27,11 +30,8 @@ pub trait CustomHandler<ExecC = Empty, QueryC = Empty> {
}

/// Simplified version of `CustomHandler` having only arguments which are not app internals - they
/// are just discarded. Usefull for simpler mocking.
#[automock(type QueryResult = Binary;)]
/// are just discarded. Useful for simpler mocking.
pub trait SimpleCustomHandler<ExecC = Empty, QueryC = Empty> {
type QueryResult;

fn execute(&self, block: &BlockInfo, sender: Addr, msg: ExecC) -> AnyResult<AppResponse>;
fn query(&self, block: &BlockInfo, msg: QueryC) -> AnyResult<Binary>;
}
Expand All @@ -58,3 +58,72 @@ impl<ExecC, QueryC, T: SimpleCustomHandler<ExecC, QueryC>> CustomHandler<ExecC,
self.query(block, msg)
}
}

/// Custom handler implementation panicking on each call. Assuming, that unless specific behavior
/// is implemented, custom messages should not be send.
pub struct PanickingCustomHandler;

impl<ExecC, QueryC> SimpleCustomHandler<ExecC, QueryC> for PanickingCustomHandler
Copy link
Member

Choose a reason for hiding this comment

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

Why even have the SimpleCustomHandler trait? Why not just implement CustomHandler directly?

Also, I think CustomHandler is the only type in this file that should be pub (rather than pub(crate)). These other types are only meaningful for us to test multi-test itself (not for blockchains that actually use custom types)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, probably in contract we should not use in contracts tests. SimpleCustomHandler is here to make implementations kind of simpler as I assume that app is somehow accessible on the side, but it may be an overkill.

where
ExecC: std::fmt::Debug,
QueryC: std::fmt::Debug,
{
fn execute(&self, _block: &BlockInfo, sender: Addr, msg: ExecC) -> AnyResult<AppResponse> {
panic!("Unexpected custom exec msg {:?} from {:?}", msg, sender)
}

fn query(&self, _block: &BlockInfo, msg: QueryC) -> AnyResult<Binary> {
panic!("Unexpected custom query {:?}", msg)
}
}

/// Internal state of `CachingCustomHandler` wrapping internal mutability so it is not exposed to
/// user. Those have to be shared internal state, as after mock is passed to app it is not
/// possible to access mock internals which are not exposed by API.
#[derive(Derivative)]
#[derivative(Default(bound = "", new = "true"), Clone(bound = ""))]
pub struct CachingCustomHandlerState<ExecC, QueryC> {
execs: Rc<RefCell<Vec<ExecC>>>,
queries: Rc<RefCell<Vec<QueryC>>>,
}

impl<ExecC, QueryC> CachingCustomHandlerState<ExecC, QueryC> {
pub fn execs(&self) -> impl Deref<Target = [ExecC]> + '_ {
Ref::map(self.execs.borrow(), Vec::as_slice)
}

pub fn queries(&self) -> impl Deref<Target = [QueryC]> + '_ {
Ref::map(self.queries.borrow(), Vec::as_slice)
}

pub fn reset(&self) {
self.execs.borrow_mut().clear();
self.queries.borrow_mut().clear();
}
}

/// Custom handler storing all the messages it received, so they can be later verified. State is
/// thin shared state, so it can be hold after mock is passed to App to read state.
#[derive(Clone, Derivative)]
#[derivative(Default(bound = "", new = "true"))]
pub struct CachingCustomHandler<ExecC, QueryC> {
state: CachingCustomHandlerState<ExecC, QueryC>,
}

impl<ExecC, QueryC> CachingCustomHandler<ExecC, QueryC> {
pub fn state(&self) -> CachingCustomHandlerState<ExecC, QueryC> {
self.state.clone()
}
}

impl<ExecC, QueryC> SimpleCustomHandler<ExecC, QueryC> for CachingCustomHandler<ExecC, QueryC> {
fn execute(&self, _block: &BlockInfo, _sender: Addr, msg: ExecC) -> AnyResult<AppResponse> {
self.state.execs.borrow_mut().push(msg);
Ok(AppResponse::default())
}

fn query(&self, _block: &BlockInfo, msg: QueryC) -> AnyResult<Binary> {
self.state.queries.borrow_mut().push(msg);
Ok(Binary::default())
}
}
4 changes: 2 additions & 2 deletions packages/multi-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
mod app;
mod bank;
mod contracts;
mod custom_handler;
pub mod custom_handler;
pub mod error;
mod executor;
mod test_helpers;
Expand All @@ -19,6 +19,6 @@ mod wasm;
pub use crate::app::{next_block, App, AppBuilder, Router};
pub use crate::bank::{Bank, BankKeeper};
pub use crate::contracts::{Contract, ContractWrapper};
pub use crate::custom_handler::{CustomHandler, MockSimpleCustomHandler, SimpleCustomHandler};
pub use crate::custom_handler::{CustomHandler, PanickingCustomHandler, SimpleCustomHandler};
pub use crate::executor::{AppResponse, Executor};
pub use crate::wasm::{parse_contract_addr, Wasm, WasmKeeper};
Loading