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 ICS02 update client tests to utilize the testing approach used by ICS03 connection tests #1082

Open
seanchen1991 opened this issue Feb 9, 2024 · 3 comments
Labels
A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews O: code-hygiene Objective: aims to improve code hygiene O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding

Comments

@seanchen1991
Copy link
Contributor

Summary

The ICS03 connection tests utilize a custom testing configuration approach that defines a Ctx enum that enumerates the different possible context variants that the tests might make use of. For example, see https://github.com/cosmos/ibc-rs/blob/main/ibc-testkit/tests/core/ics03_connection/conn_open_ack.rs.

This is a much more streamlined way to configure tests with different context and message types than the approach prescribed by rstest. We should refactor the update client tests to follow a similar approach and framework.

@seanchen1991 seanchen1991 added O: code-hygiene Objective: aims to improve code hygiene O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews labels Feb 9, 2024
@gr4yha7
Copy link
Contributor

gr4yha7 commented Feb 28, 2024

@seanchen1991 can I take this?

@Farhad-Shabani
Copy link
Member

@seanchen1991 can I take this?

Hey @gr4yha7! Thank you for stepping up to help with this issue. We're currently in the process of refactoring the ibc-testkit (#677), making progress in the feat/refactor-testkit branch. Since the changes for this issue will be extensive and could potentially lead to a lot of conflicts, we'd like to wrap up that work first. Once everything is settled, we'll give you the heads-up. Does that sound good to you? Thanks again for your willingness to contribute!
CC @rnbguy

@github-project-automation github-project-automation bot moved this to 📥 To Do in ibc-rs Feb 28, 2024
@gr4yha7
Copy link
Contributor

gr4yha7 commented Feb 28, 2024

@seanchen1991 can I take this?

Hey @gr4yha7! Thank you for stepping up to help with this issue. We're currently in the process of refactoring the ibc-testkit (#677), making progress in the feat/refactor-testkit branch. Since the changes for this issue will be extensive and could potentially lead to a lot of conflicts, we'd like to wrap up that work first. Once everything is settled, we'll give you the heads-up. Does that sound good to you? Thanks again for your willingness to contribute! CC @rnbguy

Thanks @Farhad-Shabani. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews O: code-hygiene Objective: aims to improve code hygiene O: maintainability Objective: cause to ease modification, fault corrections and improve code understanding
Projects
Status: 📥 To Do
Development

No branches or pull requests

3 participants