fix: poll token index after createToken; add integration test retries#1070
fix: poll token index after createToken; add integration test retries#1070
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughExtended integration test setup and Changes
Sequence Diagram(s)(omitted — changes are small and do not introduce a multi-component new control flow requiring visualization) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1070 +/- ##
===========================================
+ Coverage 66.33% 87.99% +21.65%
===========================================
Files 114 114
Lines 8918 8918
Branches 2022 2022
===========================================
+ Hits 5916 7847 +1931
+ Misses 2975 1044 -1931
Partials 27 27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The wallet-service has separate indexes for tx visibility and token/balance data. After createToken, pollForTx only confirms the tx is visible, but getBalance may still return empty. Adding the existing pollForTokenDetails call ensures the token index has caught up before returning. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tuliomir
force-pushed
the
fix/service-adapter-poll-token-after-create
branch
from
d155328 to
7687556
Compare
tuliomir
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
setupTests-integration.js (1)
26-27: Reconsider unconditional global retries for stateful integration tests.Global
jest.retryTimes(2)will retry every failed test up to three total attempts, which can mask genuinely flaky operations (wallet starts, token creation, transactions) rather than forcing fixes at their source. The adapter already includes deterministic polling logic for token details (pollForTokenDetailswith 20 retries, 2s delays), making test-level retries redundant and potentially hiding intermittent issues in wallet-service indexing or synchronization.Consider making retries opt-in via environment variable or limiting them to specific flaky test suites, so the polling-based fixes remain the primary signal for addressing race conditions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setupTests-integration.js` around lines 26 - 27, The global jest.retryTimes(2) unconditionally retries all tests and can mask real integration race conditions; change it so retries are opt-in or scoped: replace the unconditional call to jest.retryTimes in setupTests-integration.js with a conditional that reads an environment variable (e.g., process.env.INTEGRATION_TEST_RETRIES) or remove it here and apply jest.retryTimes only in specific flaky suites; keep existing deterministic logic like pollForTokenDetails intact and ensure wallet/transaction integration tests rely on that polling rather than global retries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@setupTests-integration.js`:
- Around line 26-27: The global jest.retryTimes(2) unconditionally retries all
tests and can mask real integration race conditions; change it so retries are
opt-in or scoped: replace the unconditional call to jest.retryTimes in
setupTests-integration.js with a conditional that reads an environment variable
(e.g., process.env.INTEGRATION_TEST_RETRIES) or remove it here and apply
jest.retryTimes only in specific flaky suites; keep existing deterministic logic
like pollForTokenDetails intact and ensure wallet/transaction integration tests
rely on that polling rather than global retries.
jest.retryTimes(2) retries each failed test case up to 2 extra times before marking it as failed, with error logging on each retry attempt. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tuliomir
force-pushed
the
fix/service-adapter-poll-token-after-create
branch
from
94345a5 to
3fffceb
Compare
raul-oliveira
left a comment
raul-oliveira
left a comment
There was a problem hiding this comment.
Please create an issue for us to revert this retry param when your pullrequests get merged.
github-project-automation
Bot
moved this from In Progress (Done)
to Waiting to be deployed
in Hathor Network
3 participants
Summary
createTokenin the service adapter, ensuring the new token is indexed before returning.jest.retryTimes(2)to integration test setup, retrying flaky test cases up to 2 extra times before failing.Why the retry on all tests, and not on specific ones?
The flakiness is not on a single test, but on the test infrastructure itself. PRs like #1033 , #1056 , #1058 , #1063 , #1069 and HathorNetwork/rfcs#103 are attempting to fix the underlying issues, but this PR would remove the immediate pain with the constant blocking we're having with the Wallet Lib CI.
Will we lose the failure data?
No, we're not. The failures are still catalogued in the CI output, which was greatly improved by #1049 . We can ask AI agents to look into the CI logs regularly to find more flaky tests, but without the great cost this is imposing on the speed of development on the Wallet Lib.
Acceptance Criteria
createTokenvia service adapter correctly polls the token index before resolving.Summary by CodeRabbit