From 9a97577c0036f6a1bceb529d085dc273f66d80f0 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Sat, 17 Jul 2021 11:49:06 +0200 Subject: [PATCH] lntest: refactor assertNumConnections The assertNumConnection function currently takes in an 'expected' number of connections argument and asserts that both alice and bob only each only have that number of connections. So this fails to be useful if say alice is also connected to charlie cause then if we call assertNumConnections between alice and bob it will fail saying there are 2 connections between them since all it does is count alice's total number of connections. This commit replaces this function with 2 new functions: assertConnected which asserts that at least one connection exists between two peers and assertNotConnected which asserts that no connections exists between the two peers. --- lntest/itest/assertions.go | 99 ++++++++++++++++++++++++++++------- lntest/itest/lnd_misc_test.go | 12 ++--- 2 files changed, 85 insertions(+), 26 deletions(-) diff --git a/lntest/itest/assertions.go b/lntest/itest/assertions.go index 0702ba34d..18c385164 100644 --- a/lntest/itest/assertions.go +++ b/lntest/itest/assertions.go @@ -471,43 +471,102 @@ func assertNumOpenChannelsPending(t *harnessTest, require.NoError(t.t, err) } -// assertNumConnections asserts number current connections between two peers. -func assertNumConnections(t *harnessTest, alice, bob *lntest.HarnessNode, - expected int) { +// checkPeerInPeersList returns true if Bob appears in Alice's peer list. +func checkPeerInPeersList(ctx context.Context, alice, + bob *lntest.HarnessNode) (bool, error) { + + peers, err := alice.ListPeers(ctx, &lnrpc.ListPeersRequest{}) + if err != nil { + return false, fmt.Errorf( + "error listing %s's node (%v) peers: %v", + alice.Name(), alice.NodeID, err, + ) + } + + for _, peer := range peers.Peers { + if peer.PubKey == bob.PubKeyStr { + return true, nil + } + } + + return false, nil +} + +// assertConnected asserts that two peers are connected. +func assertConnected(t *harnessTest, alice, bob *lntest.HarnessNode) { ctxb := context.Background() - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout) + defer cancel() err := wait.NoError(func() error { - aNumPeers, err := alice.ListPeers( - ctxt, &lnrpc.ListPeersRequest{}, - ) + bobIsAlicePeer, err := checkPeerInPeersList(ctxt, alice, bob) if err != nil { + return err + } + + if !bobIsAlicePeer { return fmt.Errorf( - "unable to fetch %s's node (%v) list peers %v", - alice.Name(), alice.NodeID, err, + "expected %s and %s to be connected "+ + "but %s is not in %s's peer list", + alice.Name(), bob.Name(), + bob.Name(), alice.Name(), ) } - bNumPeers, err := bob.ListPeers(ctxt, &lnrpc.ListPeersRequest{}) + aliceIsBobPeer, err := checkPeerInPeersList(ctxt, bob, alice) if err != nil { + return err + } + + if !aliceIsBobPeer { return fmt.Errorf( - "unable to fetch %s's node (%v) list peers %v", - bob.Name(), bob.NodeID, err, + "expected %s and %s to be connected "+ + "but %s is not in %s's peer list", + alice.Name(), bob.Name(), + alice.Name(), bob.Name(), ) } - if len(aNumPeers.Peers) != expected { + return nil + + }, defaultTimeout) + require.NoError(t.t, err) +} + +// assertNotConnected asserts that two peers are not connected. +func assertNotConnected(t *harnessTest, alice, bob *lntest.HarnessNode) { + ctxb := context.Background() + ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + + err := wait.NoError(func() error { + bobIsAlicePeer, err := checkPeerInPeersList(ctxt, alice, bob) + if err != nil { + return err + } + + if bobIsAlicePeer { return fmt.Errorf( - "number of peers connected to %s is "+ - "incorrect: expected %v, got %v", - alice.Name(), expected, len(aNumPeers.Peers), + "expected %s and %s not to be "+ + "connected but %s is in %s's "+ + "peer list", + alice.Name(), bob.Name(), + bob.Name(), alice.Name(), ) } - if len(bNumPeers.Peers) != expected { + + aliceIsBobPeer, err := checkPeerInPeersList(ctxt, bob, alice) + if err != nil { + return err + } + + if aliceIsBobPeer { return fmt.Errorf( - "number of peers connected to %s is "+ - "incorrect: expected %v, got %v", - bob.Name(), expected, len(bNumPeers.Peers), + "expected %s and %s not to be "+ + "connected but %s is in %s's "+ + "peer list", + alice.Name(), bob.Name(), + alice.Name(), bob.Name(), ) } diff --git a/lntest/itest/lnd_misc_test.go b/lntest/itest/lnd_misc_test.go index 7de314e85..4a08de9c7 100644 --- a/lntest/itest/lnd_misc_test.go +++ b/lntest/itest/lnd_misc_test.go @@ -49,7 +49,7 @@ func testDisconnectingTargetPeer(net *lntest.NetworkHarness, t *harnessTest) { net.ConnectNodes(t.t, alice, bob) // Check existing connection. - assertNumConnections(t, alice, bob, 1) + assertConnected(t, alice, bob) // Give Alice some coins so she can fund a channel. net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, alice) @@ -82,7 +82,7 @@ func testDisconnectingTargetPeer(net *lntest.NetworkHarness, t *harnessTest) { time.Sleep(time.Millisecond * 300) // Assert that the connection was torn down. - assertNumConnections(t, alice, bob, 0) + assertNotConnected(t, alice, bob) fundingTxID, err := chainhash.NewHash(pendingUpdate.Txid) if err != nil { @@ -128,7 +128,7 @@ func testDisconnectingTargetPeer(net *lntest.NetworkHarness, t *harnessTest) { } // Check existing connection. - assertNumConnections(t, alice, bob, 0) + assertNotConnected(t, alice, bob) // Reconnect both nodes before force closing the channel. net.ConnectNodes(t.t, alice, bob) @@ -152,14 +152,14 @@ func testDisconnectingTargetPeer(net *lntest.NetworkHarness, t *harnessTest) { err) } - // Check zero peer connections. - assertNumConnections(t, alice, bob, 0) + // Check that the nodes not connected. + assertNotConnected(t, alice, bob) // Finally, re-connect both nodes. net.ConnectNodes(t.t, alice, bob) // Check existing connection. - assertNumConnections(t, alice, net.Bob, 1) + assertConnected(t, alice, bob) // Cleanup by mining the force close and sweep transaction. cleanupForceClose(t, net, alice, chanPoint)