Fix what bolt2_open_channel_sending_node_checks_part1 tests
#1317
Conversation
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
Codecov Report
@@ Coverage Diff @@
## main #1317 +/- ##
==========================================
+ Coverage 90.60% 90.68% +0.07%
==========================================
Files 72 72
Lines 40075 41412 +1337
==========================================
+ Hits 36310 37553 +1243
- Misses 3765 3859 +94
Continue to review full report at Codecov.
|
valentinewallace
previously approved these changes
Feb 22, 2022
|
Took a commit to address #1311 (comment) |
There are currently two issues with `bolt2_open_channel_sending_node_checks_part1` which counteract each other and hide that the test isn't testing what it should be. First of all, the final `create_channel` call actually fails because we try to open a channel with ourselves, instead of panicing as the test is supposed to check for. However, when we fix the create_channel call to panic, when we drop `nodes[1]` after `create_channel` panics, we fail the no-pending-messages test as it as an expeted `accept_channel` in its outbound buffer. This causes a double-panic. Previously, these two offset each other - instead of panicing in `create_channel` we'd panic in the Node drop checks. This fixes both by fetching the `accept_channel` before we go into the panic'ing `create_channel` call (who's arguments were corrected).
Its very confusing to have multiple fields that do the same thing, one of which isn't even used for its stated purpose anymore after the previous few commits.
TheBlueMatt
force-pushed
the
2022-02-fix-bunk-test
branch
from
9c6a377 to
6e776d9
Compare
|
Rebased to update upstream changes which were broken by this change. |
jkczyz
approved these changes
Mar 4, 2022
valentinewallace
approved these changes
Mar 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is split out from #1311.
There are currently two issues with
bolt2_open_channel_sending_node_checks_part1which counteracteach other and hide that the test isn't testing what it should be.
First of all, the final
create_channelcall actually failsbecause we try to open a channel with ourselves, instead of
panicing as the test is supposed to check for.
However, when we fix the create_channel call to panic, when we
drop
nodes[1]aftercreate_channelpanics, we fail theno-pending-messages test as it as an expeted
accept_channelinits outbound buffer. This causes a double-panic.
Previously, these two offset each other - instead of panicing in
create_channelwe'd panic in the Node drop checks.This fixes both by fetching the
accept_channelbefore we go intothe panic'ing
create_channelcall (who's arguments werecorrected).