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

Refactor unit tests #429

Open
Tracked by #554
plafer opened this issue Feb 14, 2023 · 3 comments
Open
Tracked by #554

Refactor unit tests #429

plafer opened this issue Feb 14, 2023 · 3 comments
Labels
O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding O: testing Objective: aims to improve testing coverage

Comments

@plafer
Copy link
Contributor

plafer commented Feb 14, 2023

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.

@plafer plafer added O: testing Objective: aims to improve testing coverage O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding labels Feb 14, 2023
@plafer
Copy link
Contributor Author

plafer commented Feb 14, 2023

After we refactor our errors with #269, each test should confirm that the right error was emitted as opposed to using Result::is_err().

@Farhad-Shabani
Copy link
Member

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

@rnbguy
Copy link
Collaborator

rnbguy commented Apr 8, 2024

I think #1074 was a duplicate of this. If that is the case, we can close this after fixing this title.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding O: testing Objective: aims to improve testing coverage
Projects
Status: 📥 To Do
Development

No branches or pull requests

3 participants