-
Notifications
You must be signed in to change notification settings - Fork 384
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
Move router test utils into their own module #1731
Move router test utils into their own module #1731
Conversation
Useful for testing onion message pathfinding in upcoming PRs and any future routing modules we add
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.
Generally looks good, but it could get a bit confusing that there are now two test_utils
that are even both referenced in router.rs
. Maybe we could solve this for the time being by removing the use
statements and always using the full routing::test_utils
/util::test_utils
prefix to make it explicit?
To disambiguate it from routing::test_utils
Hmm I feel like that would add a lot of verbosity since both of those test util modules are used in a ton of places. Do you think it would help if I modified the import to be: |
I'm fine with any resolution to the module resolution. |
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.
Makes sense and LGTM!
Yup, Also, feel free to squash. |
2f19b64
Sg, I went ahead with that. I kinda prefer to keep the first big commit as a "pure code move" (modulo imports/exposing stuff as |
Codecov ReportBase: 90.70% // Head: 90.82% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1731 +/- ##
==========================================
+ Coverage 90.70% 90.82% +0.12%
==========================================
Files 86 87 +1
Lines 46679 48356 +1677
Branches 46679 48356 +1677
==========================================
+ Hits 42338 43920 +1582
- Misses 4341 4436 +95
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
To disambiguate between util::test_utils and router::test_utils
2f19b64
to
95a4173
Compare
I don't think that CI failure is related to the PR 🤔 |
Yeah seems some no-std dependency broke the MSRV. (Assuming it's the same issue I have on one PR) |
Used in #1724 to test onion message pathfinding