-
Notifications
You must be signed in to change notification settings - Fork 2
chore: Make ibc integration test more resilent #171
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
Conversation
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
WalkthroughThe integration test workflow for IBC transfers was updated to replace a fixed sleep and single balance check with a retry loop. This loop polls the Rollkit account balance multiple times, checking for IBC tokens and improving the reliability of detecting transfer completion. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant Rollkit
loop Up to 30 attempts
Workflow->>Rollkit: Query account balance
alt IBC denom found
Workflow-->>Workflow: Exit loop (success)
else
Workflow-->>Workflow: Sleep 3 seconds
end
end
alt No IBC denom after 30 attempts
Workflow-->>Workflow: Log error and exit with failure
end
Possibly related PRs
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (2)
.github/workflows/integration_test.yml (2)
370-385
: Addset -euo pipefail
to fail fast & surface unexpected errors
Inside therun:
block the script keeps executing even if any of the intermediary commands (jq
,gmd
, etc.) error out. A single silent failure would keep the loop spinning until the max-attempts timeout, hiding the real problem.celestia-appd tx ibc-transfer transfer transfer channel-0 $ROLLKIT_ADDR 100utia --from validator --node http://localhost:26654 --fees 400utia --keyring-backend test -y + # fail fast on any unexpected error + set -euo pipefail + # Wait for IBC transfer to complete with retry logic
363-386
: Trim trailing whitespace & ensure final newline to satisfy YAML lint
The workflow currently triggers YAML-lint warnings (lines 363, 369, 373, 385 and missing newline at EOF). Cleaning these up prevents noisy CI failures.- + @@ - +(Apply the same deletion of stray spaces on the other flagged lines and add a terminating newline.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml
(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/integration_test.yml
[error] 363-363: trailing spaces
(trailing-spaces)
[error] 369-369: trailing spaces
(trailing-spaces)
[error] 373-373: trailing spaces
(trailing-spaces)
[error] 385-385: trailing spaces
(trailing-spaces)
[error] 416-416: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test / Run Unit Tests
- GitHub Check: Test with Rollkit Chain
- GitHub Check: Test Migration from Cosmos SDK to Rollkit
🔇 Additional comments (1)
.github/workflows/integration_test.yml (1)
364-370
: Good resiliency improvement
Replacing the fixedsleep
with a bounded retry loop makes the test significantly less flaky while still keeping the overall wall-clock wait time reasonable.
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! do you think the other way around works too now btw? if so we should uncomment it and add this logic too.
It works on my box :-) |
Waiting for rollkit main logger changes |
Query balance in a loop.
(AI generated)
Summary by CodeRabbit