-
Notifications
You must be signed in to change notification settings - Fork 70
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
fix(env): fix default background data verification stream timeout #287
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
error @permaweb/[email protected]: The engine "yarn" is incompatible with this module. Expected version "please-use-npm". Got "1.22.22" 📝 WalkthroughWalkthroughThe pull request introduces a modification to the Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Timeout does not apply when used in the describe or the it, as it is waiting on the before to finish. Adding it there should prevent endless execution in CI.
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)
test/end-to-end/indexing.test.ts (2)
875-881
: Consider adjusting the short verification interval in tests.
SettingBACKGROUND_DATA_VERIFICATION_INTERVAL_SECONDS
to'1'
enables rapid verification checks but may introduce intermittent flakiness or resource overhead in longer test runs. Evaluate whether a slightly higher interval would provide a more stable test environment.
885-893
: Good approach for queueing the transaction in a test scenario.
No error handling is present, but it’s typical for tests to rely on uncaught exceptions. If you want more granular errors, consider wrapping with try/catch to provide descriptive test failure messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/end-to-end/indexing.test.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (macos-latest)
- GitHub Check: test (ubuntu-latest)
🔇 Additional comments (4)
test/end-to-end/indexing.test.ts (4)
846-846
: New describe block introduced.
This "Background data verification" test suite is well-scoped and clearly labeled.
884-884
: No issues found with this clarifying comment.
The added comment helps guide future developers and testers.
895-908
: Consistent queue logic for bundle indexing.
Tests appear to rely on waiting for full data item indexing, which is fine. No immediate concerns.
909-911
: Explicit wait ensures stable test flow.
Waiting for indexing and verification completion is a solid approach to avoid race conditions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #287 +/- ##
========================================
Coverage 71.83% 71.83%
========================================
Files 39 39
Lines 9818 9818
Branches 564 564
========================================
Hits 7053 7053
Misses 2761 2761
Partials 4 4 ☔ View full report in Codecov by Sentry. |
No description provided.