-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: implement simapp and e2e tests #2
Conversation
Important Review SkippedReview was skipped due to path filters Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughThe recent updates encompass a comprehensive approach to testing a blockchain application with GitHub Actions, Docker enhancements, and Makefile refinements. These changes elevate the testing framework, validate interchain interactions, parameter updates, and upgrade scheduling, while enhancing the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (7)
e2e/go.mod
is excluded by!**/*.mod
e2e/go.sum
is excluded by!**/*.sum
go.mod
is excluded by!**/*.mod
go.work
is excluded by!**/*.work
go.work.sum
is excluded by!**/*.sum
simapp/go.mod
is excluded by!**/*.mod
simapp/go.sum
is excluded by!**/*.sum
Files selected for processing (21)
- .github/workflows/e2e-tests.yaml (1 hunks)
- .gitignore (1 hunks)
- Makefile (2 hunks)
- chains.yaml (1 hunks)
- e2e/clients.json (1 hunks)
- e2e/clients_test.go (1 hunks)
- e2e/ownership_test.go (1 hunks)
- e2e/params.json (1 hunks)
- e2e/params_test.go (1 hunks)
- e2e/upgrade.json (1 hunks)
- e2e/upgrade_test.go (1 hunks)
- e2e/utils.go (1 hunks)
- local.sh (1 hunks)
- simapp/Makefile (1 hunks)
- simapp/app.go (1 hunks)
- simapp/app.yaml (1 hunks)
- simapp/export.go (1 hunks)
- simapp/ibc.go (1 hunks)
- simapp/simd/cmd/commands.go (1 hunks)
- simapp/simd/cmd/root.go (1 hunks)
- simapp/simd/main.go (1 hunks)
Files skipped from review due to trivial changes (5)
- .gitignore
- chains.yaml
- e2e/ownership_test.go
- e2e/params.json
- e2e/upgrade.json
Additional Context Used
ShellCheck (3)
local.sh (3)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
[warning] 20-20: PENDING_OWNER appears unused. Verify use (or export if used externally).
[info] 24-24: Double quote to prevent globbing and word splitting.
Additional comments not posted (17)
e2e/clients.json (1)
1-26
: The JSON structure for IBC client recovery messages is well-formed and correctly defines necessary fields.e2e/params_test.go (1)
14-32
: The test case for parameter updates is implemented correctly, with proper setup and validation steps.e2e/upgrade_test.go (1)
14-32
: The test case for scheduling software upgrades is implemented correctly, ensuring proper functionality.simapp/Makefile (1)
1-27
: The Makefile is well-structured with dynamic version determination and appropriate build flags for thesimd
binary.simapp/simd/main.go (1)
1-40
: The main function for thesimd
application is correctly set up with appropriate Bech32 prefix configurations and command initialization.local.sh (1)
20-20
: Verify the usage ofPENDING_OWNER
to ensure it's utilized appropriately in the script or elsewhere.simapp/export.go (2)
13-47
: The functionExportAppStateAndValidators
is implemented correctly with comprehensive error handling and state export logic.
51-53
: TheprepForZeroHeightGenesis
function is marked as unimplemented. Confirm if this is intentional or if it should be implemented.simapp/app.yaml (1)
1-56
: The YAML configurations forSimApp
modules are correctly defined, ensuring proper application behavior and module interaction.e2e/clients_test.go (1)
19-73
: The test functionTestClientSubstitution
is well-structured and covers the necessary steps to validate IBC client substitution effectively..github/workflows/e2e-tests.yaml (1)
1-90
: The GitHub Actions workflow is correctly set up for end-to-end testing, with well-defined jobs and steps that follow best practices for CI/CD pipelines.Makefile (1)
Line range hint
1-78
: The Makefile is well-structured with clear targets for building, testing, and managing Docker images. It follows best practices for Makefile configuration.simapp/ibc.go (1)
22-81
: TheRegisterIBCModules
function is correctly implemented for setting up IBC modules and keepers. It follows best practices for error handling and modular code.simapp/simd/cmd/commands.go (1)
30-100
: The functions incommands.go
are well-implemented for managing CLI commands of thesimd
application. They use appropriate Cobra configurations and handle errors effectively.simapp/simd/cmd/root.go (1)
37-130
: TheNewRootCmd
function is correctly implemented for setting up the root command of thesimd
application. It handles dependency injection and error scenarios effectively.simapp/app.go (1)
58-162
: TheSimApp
struct and its initialization function are well-implemented for configuring the application. They use dependency injection effectively and follow best practices for modular configuration.e2e/utils.go (1)
31-207
: The utility functions ine2e/utils.go
are well-implemented for supporting end-to-end tests. They use appropriate testing and blockchain APIs and handle errors effectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
e2e/go.mod
is excluded by!**/*.mod
e2e/go.sum
is excluded by!**/*.sum
go.mod
is excluded by!**/*.mod
Files selected for processing (1)
- .github/workflows/e2e-tests.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e-tests.yaml
3291971
to
63133e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Clean and easy to read.
Only thought is if you think it is necessary to test non happy paths such as submitting update owner, params and client transactions from non owner account.
Thanks for the review @boojamya!
We should talk more about this on our internal sync later today, but it comes down to our internal reasoning around Unit vs E2E tests 🤔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/e2e-tests.yaml (1 hunks)
- .github/workflows/unit-tests.yaml (2 hunks)
- e2e/utils.go (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/unit-tests.yaml
Files skipped from review as they are similar to previous changes (2)
- .github/workflows/e2e-tests.yaml
- e2e/utils.go
Closes #1
Summary by CodeRabbit
New Features
SimApp
with new configurations and functionalities for various modules, including IBC modules.Bug Fixes
Chores
.gitignore
to exclude.authority
andbuild
directories.Makefile
with new targets for building, local image creation, and end-to-end testing.Tests