-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add Local-Ic Relayer Edge Case Tests for Remote Chain Splitter #319
Merged
dowlandaiello
merged 2 commits into
main
from
dowlandaiello/testing-remotechainsplitter-merged
Aug 13, 2024
Merged
Add Local-Ic Relayer Edge Case Tests for Remote Chain Splitter #319
dowlandaiello
merged 2 commits into
main
from
dowlandaiello/testing-remotechainsplitter-merged
Aug 13, 2024
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
keyleu
requested changes
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
keyleu
reviewed
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
keyleu
reviewed
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
keyleu
reviewed
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
keyleu
reviewed
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
keyleu
reviewed
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Show resolved
Hide resolved
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Show resolved
Hide resolved
keyleu
reviewed
Aug 12, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Outdated
Show resolved
Hide resolved
bekauz
reviewed
Aug 13, 2024
local-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
Show resolved
Hide resolved
keyleu
approved these changes
Aug 13, 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
dowlandaiello
deleted the
dowlandaiello/testing-remotechainsplitter-merged
branch
August 13, 2024 18:26
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.
Closes #235 . This PR implements relayer edge case tests in local-interchain for the remote chain splitter. These tests include:
Failing Tests
Some local-ic tests are not currently working (namely the two party osmo pol test). However, all of the remote chain splitter tests are working. Fixing the two party osmo pol test is not within the scope of this PR, so I did not fix them.
Overview on Changes / Additions
local-interchaintest/Cargo.toml
- Added the remote chain splitter as a dependency to local-ic so that it can be testedCargo.lock
- See above aboutCargo.toml
local-interchaintest/src/main.rs
- Added the remote chain splitter tests as tests run by local-ictest-e2elocal-interchaintest/src/tests/mod.rs
,local-interchaintest/src/tests/remote_chain_splitter/mod.rs
- Self explanatorylocal-interchaintest/src/tests/remote_chain_splitter/remote_chain_splitter.rs
- See above on the 3 tests I added. Of note: two functions in this file,start_relayer
andstop_relayer
may be useful in other local-ic relayer tests, so moving them to a utils file could be prudent to prevent test dependencies across tests.Macro Usage
There is a very short macro in
remote_chain_splitter.rs
. I can remove it if necessary. The macro just consolidates some redundant timeout checking logic in a way that allows breaking from the timeout check on successful ICA creation, which would be slightly more convoluted with a function. IMO, it's short and relatively useful, but not necessary, so it can be removed if desired.