ibc-rs
ibc-rs copied to clipboard
Refactor unit tests
Our unit tests make extensive use of default values, including in "util" functions implicitly used, which creates hard to understand relationships, and ultimately makes it difficult to understand why a test succeeds or fails. For example, some validation checks probably work because we used the default value for our ClientId
, which happens to be also used when, say, creating the "dummy" packet.
I've also seen tests such as this one which claims a reason for the test to fail (e.g. the client doesn't have a consensus state at the required height), but no client is even installed in that MockContext
.
After we refactor our errors with #269, each test should confirm that the right error was emitted as opposed to using Result::is_err()
.
One possible solution regarding defaults:
Found it effective and can result in a cleaner, easier-to-follow code, not just for default values, even for any new object construction within our tests, is to implement and use more helper methods. This can exist for each type in its own module under the mod test_util
. Then simply be called everywhere needed. Similar to what we did here for MsgConnectionOpenInit
, a new_valid_dummy
method or something like that can be defined for e.g CleintId
, Height
, etc
I think #1074 was a duplicate of this. If that is the case, we can close this after fixing this title.