-
Notifications
You must be signed in to change notification settings - Fork 386
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
Genericize Test Functions #2177
Genericize Test Functions #2177
Conversation
In the coming commits we'll add some additional complexity to the event handling flows, so best to DRY them up before we get there.
Assigning val and wilmer cause y'all are already on 2111. |
&'a test_utils::TestFeeEstimator, &'a test_utils::TestRouter<'a>, | ||
&'a test_utils::TestLogger>; | ||
|
||
struct ANodeHolder<'a, P: Persist<InMemorySigner>> { |
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.
Can we migrate the BackgroundProcessor
utils as well so they can use functional_test_utils
?
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.
I think so, but its a bit big, would have to add a bunch of new accessors and maybe do the chain tracking via the trait too.
1be3caf
to
db9cd01
Compare
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #2177 +/- ##
==========================================
- Coverage 91.36% 91.34% -0.02%
==========================================
Files 102 102
Lines 50369 50401 +32
Branches 50369 50401 +32
==========================================
+ Hits 46018 46040 +22
- Misses 4351 4361 +10
... and 4 files with indirect coverage changes 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 in Codecov by Sentry. |
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 after @wpaulino takes a look
In our test utilities, we generally refer to a `Node` struct which holds a `ChannelManager` and a number of other structs. However, we use the same utilities in benchmarking, where we have a different `Node`-like struct. This made moving from macros to functions entirely impossible, as we end up needing multiple types in a given context. Thus, here, we take the pain and introduce some wrapper traits which encapsulte what we need from `Node`, swapping some of our macros to functions.
This marginally reduces the quantity of code compiled in tests further.
db9cd01
to
cfb9eb9
Compare
Squashed with typo fixes:
|
This is the prefactors from #2111 which just set us up to have more functions in testing, avoiding more macros in tests.