Open
Conversation
Co-authored-by: Copilot <copilot@github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidates the standalone Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ndk/lib/test_utils/cache_manager_test_suite_cashu.dart`:
- Around line 76-79: Calls to getProofs are passing a nonexistent state argument
(e.g. CashuProofState.unspend); the method signature is getProofs({String?
mintUrl, String? keysetId}), so remove the state parameter from all
getProofs(...) invocations (for example the calls that currently look like
getProofs(mintUrl: 'https://test.mint.com', state: CashuProofState.unspend)) and
leave only the supported named args (mintUrl and/or keysetId); update every
occurrence in the test suite where getProofs is called with state (including the
other reported spots) so they match the actual getProofs signature.
In `@packages/ndk/lib/test_utils/cache_manager_test_suite.dart`:
- Around line 16-20: The example in runCacheManagerTestSuite uses the wrong
parameter name and implies wrong timing; update all examples (including the
snippets at lines mentioned) to use the correct parameter name cleanUp instead
of tearDown and state that cleanUp is invoked after each test (it is executed
via tearDown), e.g. mention cleanUp: (cacheManager) async => await
cacheManager.close(), and clarify in the doc text that cleanUp runs after every
test, not after the test group, referencing runCacheManagerTestSuite and the
cleanUp callback to locate the code to update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2acbc84a-8c80-4303-9dfe-2405e397c2ff
📒 Files selected for processing (10)
packages/drift/pubspec.yamlpackages/drift/test/drift_cache_manager_test.dartpackages/ndk/lib/test_utils/cache_manager_test_suite.dartpackages/ndk/lib/test_utils/cache_manager_test_suite_cashu.dartpackages/ndk/lib/testing.dartpackages/ndk/test/data_layer/cache_manager/mem_cache_manager_test.dartpackages/ndk/test/data_layer/cache_manager/sembast_cache_manager_test.dartpackages/objectbox/pubspec.yamlpackages/objectbox/test/data_layer/cache_manager/objectbox_cache_manager_test.dartpubspec.yaml
💤 Files with no reviewable changes (3)
- packages/drift/pubspec.yaml
- packages/objectbox/pubspec.yaml
- pubspec.yaml
Comment on lines
+76
to
+79
| final loadedProofs = await cacheManager.getProofs( | ||
| mintUrl: 'https://test.mint.com', | ||
| state: CashuProofState.unspend, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CacheManager method signatures =="
rg -n --type=dart -C2 'Future<.*>\s+getProofs\s*\(|Future<.*>\s+getMintInfos\s*\(' packages/ndk/lib/domain_layer/repositories/cache_manager.dart
echo
echo "== Named-argument usage in shared cashu suite =="
rg -n --type=dart -C2 'getProofs\s*\(|getMintInfos\s*\(' packages/ndk/lib/test_utils/cache_manager_test_suite_cashu.dartRepository: relaystr/ndk
Length of output: 2190
Remove undeclared state parameter from getProofs() calls.
The method signature is getProofs({String? mintUrl, String? keysetId}) but these calls pass a state parameter that does not exist in the interface. Update all calls to match the actual signature.
Affected lines: 76–79, 116–119, 123–126, 200–201, 204–205.
Note: getMintInfos(mintUrls: ...) calls are correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ndk/lib/test_utils/cache_manager_test_suite_cashu.dart` around lines
76 - 79, Calls to getProofs are passing a nonexistent state argument (e.g.
CashuProofState.unspend); the method signature is getProofs({String? mintUrl,
String? keysetId}), so remove the state parameter from all getProofs(...)
invocations (for example the calls that currently look like getProofs(mintUrl:
'https://test.mint.com', state: CashuProofState.unspend)) and leave only the
supported named args (mintUrl and/or keysetId); update every occurrence in the
test suite where getProofs is called with state (including the other reported
spots) so they match the actual getProofs signature.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #596 +/- ##
==========================================
+ Coverage 72.20% 74.04% +1.84%
==========================================
Files 201 203 +2
Lines 10144 10874 +730
==========================================
+ Hits 7324 8052 +728
- Misses 2820 2822 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nogringo
approved these changes
Apr 30, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Embeds the test suite into
ndkpackage.Exported via
package:ndk/testing.dartThis is necessary because of embedding of sembast into ndk (circular dependency)
ndk -> test_suite -> ndkNot super happy with this solution but i don't see other good options.
The
package:ndk/testing.dartis used so we can export other test tools in the future => so others can use exposed tests for their db implementation etc.Summary by CodeRabbit
package:ndk/testing.dartinstead of a dedicated package.