lightningdevkit
Move tests in `channelmanager.rs` into more appropriate test files
The channelmanager.rs file is huuuge³ at the moment and we shouldn't be adding more tests to it. We should move them into more appropriate test files (which may involve creating new _test.rs files too.
This is unlikely to create a ton of git noise as it should just be pure moves of tests so it could probably be done in one PR.
I would like to work on this issue, is this up for grabs?
Hey, thanks! It sure is! Let us know on this issue if you have any other questions while working on this. I'm not sure everything would move into existing test files so you may need to create some new ones that are more relevant but please feel free to ask!
Hey @dunxen I have analyzed the channelmanager tests and prepared the following test groups, lemme know of your thoughts
There are total 22 tests
Group 1: Key Send and Payment Verification
- test_keysend_dup_hash_partial_mpp
- test_keysend_dup_payment_hash
- test_keysend_hash_mismatch
- test_keysend_msg_with_secret_err
- bad_inbound_payment_hash
- reject_excessively_underpaying_htlcs
- test_final_incorrect_cltv
- test_payment_display
- test_malformed_forward_htlcs_ser
Group 2: Channel Management and Limits
- test_notify_limits
- test_drop_disconnected_peers_when_removing_channels
- test_outpoint_to_peer_coverage
- test_api_calls_with_unkown_counterparty_node
- test_api_calls_with_unavailable_channel
- test_connection_limiting
- test_outbound_chans_unlimited
- test_0conf_limiting
- test_trigger_lnd_force_close
Group 3: Anchor Channel and Configuration Tests
- test_inbound_anchors_manual_acceptance
- test_anchors_zero_fee_htlc_tx_fallback
- test_update_channel_config
Hey @srikanth-iyengar, those groupings seem to make sense to me from first glance! I think we can get some more comments on the moves int he PR to see it in context.
I started working on the issue, Currently I found that some of the fields/functions in the channel manager are private , should I make them public like pub(crate)?
Hey @dunxen I have raised a PR for this issue, Can you take a look ?