Uh oh!
There was an error while loading. Please reload this page.
Test coverage for transaction_unconfirmed - #1481
Conversation
1ba1c6d to
640af71CompareCodecov Report
@@ Coverage Diff @@## main #1481 +/- ##
==========================================
+ Coverage 90.89% 91.51% +0.62%
==========================================
Files 76 77 +1 Lines 42054 46787 +4733 Branches 42054 46787 +4733 ==========================================
+ Hits 38223 42815 +4592 - Misses 3831 3972 +141
Continue to review full report at Codecov.
|
TheBlueMatt
commented
May 17, 2022
Tagging 108 because #1486 depends on this. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| // Check we only broadcast 1 timeout tx | ||
| let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); | ||
| assert_eq!(claim_txn.len(), 8); |
There was a problem hiding this comment.
Is this still checking that we only broadcast 1 timeout tx per the comment?
There was a problem hiding this comment.
Kinda? I mean we do broadcast one timeout tx per commitment, but, yea, I'm just gonna drop the comment its confusing.
| let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); | ||
| assert_eq!(node_txn.len(), 9); | ||
| assert!(node_txn.len() == 9 || node_txn.len() == 10); | ||
| // ChannelMonitor: justice tx revoked offered htlc, justice tx revoked received htlc, justice tx revoked to_local (3) | ||
| // ChannelManager: local commmitment + local HTLC-timeout (2) | ||
| // ChannelMonitor: bumped justice tx, after one increase, bumps on HTLC aren't generated not being substantial anymore, bump on revoked to_local isn't generated due to more room for expiration (2) |
There was a problem hiding this comment.
Are these comments enumerating the txs in node_txn? Do they need updating?
There was a problem hiding this comment.
I just dropped them. We aren't actually checking all those things so the comment is just going to get stale and confusing over time anyway.
Uh oh!
There was an error while loading. Please reload this page.
valentinewallace
commented
May 17, 2022
I think a warning may have been introduced in |
3936511 to
51a58a9ComparePreviously `transaction_unconfirmed` was never called in tests!
In the next commit we'll randomize the `ConnectStyle` used in each test. However, some tests are slightly too prescriptive, which we address here in a few places.
We have a bunch of fancy infrastructure to ensure we can connect blocks using all our different connection interfaces, but we only bother to use it in a few select tests. This expands our use of `ConnectStyle` to most of our tests by simply randomizing the style in each test. This makes our tests non-deterministic, but we print the connection style at start so that it's easy to reproduce a failure deterministically.
51a58a9 to
b083870CompareTheBlueMatt
commented
May 17, 2022
Squashed without changes: |
I noticed we have ~zero test coverage of the
transaction_unconfirmedAPI, which is somewhat frightening. This adds such coverage via a newConnectionStylein the functional tests and also randomizes theConnectionStyleused during functional tests, providing more coverage of the various connection methods generally.