Problem: reopen ica account is not tested

[mmsqe]

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

PR Checklist:

• Have you read the CONTRIBUTING.md?
• Does your PR follow the C4 patch requirements?
• Have you rebased your work on top of the latest master?
• Have you checked your code compiles? (make)
• Have you included tests for any non-trivial functionality?
• Have you checked your code passes the unit tests? (make test)
• Have you checked your code formatting is correct? (go fmt)
• Have you checked your basic code style is fine? (golangci-lint run)
• If you added any dependencies, have you checked they do not contain any known vulnerabilities? (go list -json -m all | nancy sleuth)
• If your changes affect the client infrastructure, have you run the integration test?
• If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
• If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
• If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

Summary by CodeRabbit

• New Features
  • Introduced an optional parameter to specify the expected state for a channel to be considered ready.
• Tests
  • Added a new function for waiting for new blocks and modified existing functions to improve test logic and functionality.
  • Introduced a retry mechanism in the packet log function.
  • Added a new test case with flaky test marking.
• Refactor
  • Refactored several functions for improved waiting logic and test case execution.
• Chores
  • Updated the command used to run pytest for specific test cases.

[coderabbitai]

Walkthrough

The changes primarily revolve around enhancing the functionality and reliability of the integration tests in the test_ica_precompile.py and ibc_utils.py files. The modifications include the introduction of new functions, parameters, and logic to improve the waiting mechanisms and assertions in the tests. Additionally, the command to run specific tests in the run-integration-tests script has been updated.

Changes

File	Change Summary
integration_tests/ibc_utils.py	Introduced a new optional parameter target to the wait_for_check_channel_ready function. Added import of the os module and launched a subprocess for rly in the prepare_network function.
integration_tests/test_ica_precompile.py	Refactored waiting logic and assertions, added a new function wait_for_new_blocks, and introduced a new test case test_sc_call with flaky test marking.
scripts/run-integration-tests	Updated the command to run pytest, specifically targeting the test_ica_precompile.py file and the test_sc_call test.

Poem

🐇 Hopping through the code, making changes bright,
In the season of fall, under the moonlight. 🌙
With each test we refine, with each bug we fight,
Our code becomes a beacon, shining ever so bright. 💡
On this day in history, a rabbit took flight, 🚀
Celebrating our progress, oh what a sight! 🎉
So here's to the coders, working day and night,
Making the world better, byte by byte. 💻

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 376da21 and 241f493.

Files selected for processing (2)

• integration_tests/ibc_utils.py (2 hunks)
• integration_tests/test_ica_precompile.py (7 hunks)

Additional comments: 8

integration_tests/ibc_utils.py (1)

• 589-595: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [578-595]

The function wait_for_check_channel_ready has been updated to accept a new parameter target which is used to check the state of the channel. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

integration_tests/test_ica_precompile.py (7)

• 23-27: The import of the new function wait_for_new_blocks is correctly placed in alphabetical order.

• 180-191: The function wait_for_packet_log has been renamed and updated to include a new parameter status. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

• 204-216: The get_next_channel function is now called with cli_controller and connid as arguments. This change seems to be in line with the new functionality.

• 260-266: The wait_for_packet_log function is called with the new status argument. This change seems to be in line with the new functionality.

• 303-314: The submit_msgs function has been updated to include a new timeout value. This change seems to be in line with the new functionality.

• 324-338: The get_next_channel function is now called with cli_controller and connid as arguments. This change seems to be in line with the new functionality.

• 341-360: The new function wait_for_new_blocks is used correctly. This change seems to be in line with the new functionality.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 241f493 and 03c7138.

Files ignored due to filter (1)

• nix/sources.json

Files selected for processing (1)

• integration_tests/test_ica_precompile.py (7 hunks)

Additional comments: 6

integration_tests/test_ica_precompile.py (6)

• 115-121: The wait_for_check_tx function is now being used to wait for a transaction to be processed. This is a good change as it makes the code more robust by ensuring that the transaction has been processed before proceeding. However, it's important to ensure that the wait_for_check_tx function handles all possible transaction statuses correctly.

• 179-190: The wait_for_packet_log function has been introduced to wait for a specific log event. This is a good change as it makes the code more robust by ensuring that the expected log event has occurred before proceeding. However, it's important to ensure that the wait_for_packet_log function handles all possible log events correctly.

• 193-194: The test_sc_call function has been marked as flaky. This is a good practice for tests that intermittently fail. However, it's important to investigate the root cause of the flakiness and fix it to improve the reliability of the tests.

• 204-216: The get_next_channel function is now being called with cli_controller and connid as arguments. This is a good change as it makes the code more flexible by allowing the next channel to be determined dynamically. However, it's important to ensure that the get_next_channel function handles all possible scenarios correctly.

• 324-329: The wait_for_packet_log function is being used to wait for a specific log event after a status change. This is a good change as it makes the code more robust by ensuring that the expected log event has occurred before proceeding. However, it's important to ensure that the wait_for_packet_log function handles all possible log events correctly.

• 309-314: The submit_msgs function is now being called with a timeout value. This is a good change as it makes the code more robust by allowing the function to timeout if it takes too long to complete. However, it's important to ensure that the submit_msgs function handles the timeout correctly.

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 03c7138 and 691342c.

Files selected for processing (3)

• integration_tests/ibc_utils.py (4 hunks)
• integration_tests/test_ica_precompile.py (7 hunks)
• scripts/run-integration-tests (1 hunks)

Files skipped from review due to trivial changes (1)

• scripts/run-integration-tests

Additional comments: 9

integration_tests/ibc_utils.py (3)

• 1-3: The os module has been imported to use its setsid function for creating a new session for the subprocess. This is a good practice as it allows the subprocess to run independently of the parent process.

• 179-198: The prepare_network function has been updated to launch a subprocess for rly if is_hermes is False. This is a good practice as it allows the function to support both hermes and rly relayers. However, ensure that the rly command and its arguments are correct and that the rly executable is available in the system's PATH.

• 601-607: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [591-607]

The wait_for_check_channel_ready function now accepts an additional target parameter, which defaults to "STATE_OPEN". This is a good practice as it allows the function to wait for the channel to reach a specific state. However, ensure that the target parameter is always one of the valid channel states.

integration_tests/test_ica_precompile.py (6)

• 179-190: The wait_for_packet_log function is introduced to wait for a specific log event. This is a good practice as it ensures that the test waits for the specific event before proceeding, reducing the chance of flaky tests.

• 203-215: The channel_id variable is introduced and used in the register_acc function. This is a good practice as it makes the code more readable and maintainable.

• 259-265: The wait_for_packet_log function is used to wait for a specific log event. This is a good practice as it ensures that the test waits for the specific event before proceeding, reducing the chance of flaky tests.

• 280-286: The wait_for_packet_log function is used again to wait for a specific log event. This is a good practice as it ensures that the test waits for the specific event before proceeding, reducing the chance of flaky tests.

• 302-313: The wait_for_packet_log function is used again to wait for a specific log event. This is a good practice as it ensures that the test waits for the specific event before proceeding, reducing the chance of flaky tests.

• 323-357: The wait_for_check_channel_ready function is used to wait for the channel to be in a specific state. This is a good practice as it ensures that the test waits for the specific state before proceeding, reducing the chance of flaky tests.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 241f493 and 38319c1.

Files ignored due to filter (3)

• integration_tests/poetry.lock
• integration_tests/pyproject.toml
• nix/sources.json

Files selected for processing (2)

• integration_tests/test_ica.py (2 hunks)
• integration_tests/test_ica_precompile.py (9 hunks)

Additional comments: 11

integration_tests/test_ica_precompile.py (6)

• 78-82: The submit_msgs function now includes a new parameter msg_num with a default value of 2. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

• 88-94: The submit_msgs function call is updated to pass the msg_num parameter. This change seems to be correct as it allows for the generation of a variable number of messages.

• 116-122: The previous comment about the potential issue when timeout is equal to no_timeout is still valid. Consider adjusting the condition to include this case.

• 180-191: The assert_packet_result function is replaced with the new wait_for_packet_log function. This change seems to be correct as it includes a retry mechanism.

• 203-227: The test_sc_call function has been updated to include changes related to gas and channel ID handling. Ensure that these changes do not affect the expected behavior of the function.

• 334-362: The test_sc_call function now includes additional assertions and function calls. Ensure that these changes do not affect the expected behavior of the function.

integration_tests/test_ica.py (5)

• 16-27: The ibc fixture has been updated to include a new parameter relayer. Ensure that all calls to this fixture throughout the codebase have been updated to match the new signature.

• 35-49: The register_acc helper function has been added. It registers an interchain account and waits for the channel to be ready. It then queries the interchain account and returns the account address and channel ID. This function seems to be well-structured and follows good practices.

• 57-70: The generated_tx_txt helper function has been added. It generates a transaction to send to the host chain and writes it to a file. The function seems to be well-structured and follows good practices.

• 74-86: The submit_msgs helper function has been added. It submits a transaction on the host chain on behalf of the interchain account and waits for the transaction to be processed. The function seems to be well-structured and follows good practices.

• 88-100: The test_ica function has been refactored to use the new helper functions. It tests the process of submitting large transactions to trigger a timeout, reopening the interchain account after the channel gets closed, and submitting normal transactions. The function seems to be well-structured and follows good practices.

[codecov]

Codecov Report

Merging #1234 (6a18c2c) into main (376da21) will decrease coverage by 20.23%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1234       +/-   ##
===========================================
- Coverage   36.89%   16.67%   -20.23%     
===========================================
  Files         115       79       -36     
  Lines       10255     5786     -4469     
===========================================
- Hits         3784      965     -2819     
+ Misses       6097     4743     -1354     
+ Partials      374       78      -296     

see 54 files with indirect coverage changes