-
Notifications
You must be signed in to change notification settings - Fork 208
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
integration test initial network setup #1256
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
assafmo
approved these changes
Aug 7, 2024
assafmo
approved these changes
Aug 7, 2024
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.
LGTM!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is still very much a work in progress, but figured it'd be good to checkpoint and get something merged in before continuing!
Note to review: I'd recommend reviewing commit by commit!
Context
The main motivation for the new integration test framework is to have the ability to easily spin up multiple networks in parallel to accommodate different testing setups.
The setup for each suite of tests are often mutually exclusive (e.g. some tests need long epochs, some tests need short epochs). And even for those that are not mutually exclusive, there's a compounding effect that comes with trying to use a single network across different tests.
For instance, the dydx trade route has an involved startup cost (requires stride, dydx, osmo, noble, and relayers across each pair). There's no reason the airdrop tests should have to wait for this setup!
Our current integration test setup (dockernet), while great for iterative development, fell short of these requirements, and also made it difficult to write tests (since it was in bash). The new network framework introduced in this PR, coupled with the new testing client interface in #1244 (by @assafmo), will make it much easier to write tests and integrate them in our CI.
Brief Changelog
add-comsumer-section
(testing cli command for ICS vals) to make it more flexible. It was previously hard coded to work only with dockernetstride1
,stride2
, etc.init-chain.sh
), start each validator (init-node.sh
) and create validators/governors (create-validators.sh
)init-chain.sh
andinit-node.sh
run in the pod's init container.create-validators.sh
runs in a post start hook (since the network needs to be up first)README
for explanation of design decision here)Next steps