ibc-rs
ibc-rs copied to clipboard
imp(tests): refactor table driven tests
Improvement Summary
Currently, some of the rust tests contain a group of table/data-driven tests. These tests will fail if one in the group fails.
https://github.com/cosmos/ibc-rs/blob/c83f26aeb87004ca464917e250da62ba3e92238b/crates/ibc/src/applications/transfer/coin.rs#L125-L148
We want to treat each testcases individually. If one fails, the others in the group may continue.
Proposal
The current Rust release doesn't support this feature. The common opinion in the Rust community is to keep each testcases in a separate test method, potentially using a macro.
We don't want to implement/maintain our macro. So we will use rstest
as a solution. The following is a refactored code of the above example using rstest
.
https://github.com/cosmos/ibc-rs/blob/1aa853bb646d37aab3848096b4221450f3a2e760/crates/ibc/src/applications/transfer/coin.rs#L144-L159
hi, can i work on this
@kien6034 thanks for your interest! but this requires significant changes, that
- requires decisions from maintainers
- may conflict with the testkit rework branch (#1109).
Hey has this issue un-blocked after #1109 @rnbguy
hey @TropicalDog17, thanks for taking interest. This requires some thinking from our side. But you can work on the straight-forward cases.
@rnbguy can I rewrite the tests on ibc-testkit
crate?
Don't try to refactor the complicated tests with fixtures. If you come across some tests which look like the following, you may refactor.
#[test]
fn test_my_function() {
my_function("hello world").expect("no error");
my_function("hello mars").expect_err("error");
}
#[rstest]
#[case("hello world")]
#[should_panic]
#[case("hello mars)]
fn test_my_function_with_rstest(value: &str) {
my_function(value).unwrap();
}