Drop proptests in lightning-liquidity
#4406
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
TheBlueMatt
force-pushed
the
2026-02-fix-msrv
branch
from
b71fcd5 to
3635f32
Compare
TheBlueMatt
force-pushed
the
2026-02-fix-msrv
branch
2 times, most recently
from
db91d4e to
bcd1a32
Compare
`proptest`'s transitive dependency tree has always been somewhat large, but one of them (`rusty-fork`'s `tempfile` dependency) just went ahead with a bump of their `rand` dependency, breaking our MSRV yet again. Because we don't actually use `proptest` for anything interesting, the simplest solution is to simply drop it, which we do here. Note that we'll likely transition the LSPS5 URL type to simply use the `bitreq` URL type over the next few days anyway, so there's not much reason to care about its continued test coverage. Further, in writing this commit it was discovered that our tests in `lsps2/utils.rs` were actually broken on the vast majority of inputs, but proptest wasn't testing with any interesting test cases at all, causing it to be missed entirely!
5427b0d changed the `lightning-types` API but we forgot to bump the crate version to make semver tests pass.
TheBlueMatt
force-pushed
the
2026-02-fix-msrv
branch
from
bcd1a32 to
4deb2f7
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4406 +/- ##
==========================================
- Coverage 86.05% 86.03% -0.02%
==========================================
Files 156 156
Lines 103188 103141 -47
Branches 103188 103141 -47
==========================================
- Hits 88799 88739 -60
- Misses 11878 11890 +12
- Partials 2511 2512 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Okay, fine, I give you this one ;)
No, really, while I maintain that there are cases to choose proptests over fuzzing, this change looks reasonable to me. Going ahead landing this.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
3 participants
proptest's transitive dependency tree has always been somewhat large, but one of them (rusty-fork'stempfiledependency) just went ahead with a bump of theirranddependency, breaking our MSRV yet again.Because we don't actually use
proptestfor anything interesting, the simplest solution is to simply drop it, which we do here. Note that we'll likely transition the LSPS5 URL type to simply use thebitreqURL type over the next few days anyway, so there's not much reason to care about its continued test coverage.