itest: fix flake in garbage collect test, use require

This commit uses the require library for the link nodes garbage collect
test and fixes a flake that was discovered while hunting for other
flakes. Some times the channel isn't updated fast enough so we can
detech it on first try. So we give it a few more tries to stabilize the
test.

lntest/itest/lnd_misc_test.go

@@ -610,7 +610,7 @@ func testMaxPendingChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	}
 }
 
-// testGarbageCollectLinkNodes tests that we properly garbase collect link nodes
+// testGarbageCollectLinkNodes tests that we properly garbage collect link nodes
 // from the database and the set of persistent connections within the server.
 func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) {
 	ctxb := context.Background()
@@ -623,8 +623,7 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) {
 	// cooperatively closed.
 	ctxt, _ := context.WithTimeout(ctxb, channelOpenTimeout)
 	coopChanPoint := openChannelAndAssert(
-		ctxt, t, net, net.Alice, net.Bob,
+		ctxt, t, net, net.Alice, net.Bob, lntest.OpenChannelParams{
-		lntest.OpenChannelParams{
 			Amt: chanAmt,
 		},
 	)
@@ -639,8 +638,7 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) {
 	// closed.
 	ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout)
 	forceCloseChanPoint := openChannelAndAssert(
-		ctxt, t, net, net.Alice, carol,
+		ctxt, t, net, net.Alice, carol, lntest.OpenChannelParams{
-		lntest.OpenChannelParams{
 			Amt: chanAmt,
 		},
 	)
@@ -654,8 +652,7 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) {
 	net.ConnectNodes(ctxt, t.t, net.Alice, dave)
 	ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout)
 	persistentChanPoint := openChannelAndAssert(
-		ctxt, t, net, net.Alice, dave,
+		ctxt, t, net, net.Alice, dave, lntest.OpenChannelParams{
-		lntest.OpenChannelParams{
 			Amt: chanAmt,
 		},
 	)
@@ -1620,30 +1617,22 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) {
 		ctxt, t, net, net.Alice, net.Bob, channelParam,
 	)
 	txid, err := lnrpc.GetChanPointFundingTxid(chanPoint)
-	if err != nil {
+	require.NoError(t.t, err, "alice bob get channel funding txid")
-		t.Fatalf("unable to get txid: %v", err)
-	}
 	chanPointStr := fmt.Sprintf("%v:%v", txid, chanPoint.OutputIndex)
 
 	// Wait for channel to be confirmed open.
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	err = net.Alice.WaitForNetworkChannelOpen(ctxt, chanPoint)
-	if err != nil {
+	require.NoError(t.t, err, "alice wait for network channel open")
-		t.Fatalf("alice didn't report channel: %v", err)
-	}
 	err = net.Bob.WaitForNetworkChannelOpen(ctxt, chanPoint)
-	if err != nil {
+	require.NoError(t.t, err, "bob wait for network channel open")
-		t.Fatalf("bob didn't report channel: %v", err)
-	}
 
 	// Now that the channel is open, we'll obtain its channel ID real quick
 	// so we can use it to query the graph below.
 	listReq := &lnrpc.ListChannelsRequest{}
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	aliceChannelList, err := net.Alice.ListChannels(ctxt, listReq)
-	if err != nil {
+	require.NoError(t.t, err)
-		t.Fatalf("unable to fetch alice's channels: %v", err)
-	}
 	var chanID uint64
 	for _, channel := range aliceChannelList.Channels {
 		if channel.ChannelPoint == chanPointStr {
@@ -1651,18 +1640,13 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) {
 		}
 	}
 
-	if chanID == 0 {
+	require.NotZero(t.t, chanID, "unable to find channel")
-		t.Fatalf("unable to find channel")
-	}
 
 	// To make sure the channel is removed from the backup file as well when
 	// being abandoned, grab a backup snapshot so we can compare it with the
 	// later state.
 	bkupBefore, err := ioutil.ReadFile(net.Alice.ChanBackupPath())
-	if err != nil {
+	require.NoError(t.t, err, "channel backup before abandoning channel")
-		t.Fatalf("could not get channel backup before abandoning "+
-			"channel: %v", err)
-	}
 
 	// Send request to abandon channel.
 	abandonChannelRequest := &lnrpc.AbandonChannelRequest{
@@ -1671,44 +1655,31 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) {
 
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	_, err = net.Alice.AbandonChannel(ctxt, abandonChannelRequest)
-	if err != nil {
+	require.NoError(t.t, err, "abandon channel")
-		t.Fatalf("unable to abandon channel: %v", err)
-	}
 
 	// Assert that channel in no longer open.
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	aliceChannelList, err = net.Alice.ListChannels(ctxt, listReq)
-	if err != nil {
+	require.NoError(t.t, err, "list channels")
-		t.Fatalf("unable to list channels: %v", err)
+	require.Zero(t.t, len(aliceChannelList.Channels), "alice open channels")
-	}
-	if len(aliceChannelList.Channels) != 0 {
-		t.Fatalf("alice should only have no channels open, "+
-			"instead she has %v",
-			len(aliceChannelList.Channels))
-	}
 
 	// Assert that channel is not pending closure.
 	pendingReq := &lnrpc.PendingChannelsRequest{}
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	alicePendingList, err := net.Alice.PendingChannels(ctxt, pendingReq)
-	if err != nil {
+	require.NoError(t.t, err, "alice list pending channels")
-		t.Fatalf("unable to list pending channels: %v", err)
+	require.Zero(
-	}
+		t.t, len(alicePendingList.PendingClosingChannels), //nolint:staticcheck
-	if len(alicePendingList.PendingClosingChannels) != 0 { //nolint:staticcheck
+		"alice pending channels",
-		t.Fatalf("alice should only have no pending closing channels, "+
+	)
-			"instead she has %v",
+	require.Zero(
-			len(alicePendingList.PendingClosingChannels)) //nolint:staticcheck
+		t.t, len(alicePendingList.PendingForceClosingChannels),
-	}
+		"alice pending force close channels",
-	if len(alicePendingList.PendingForceClosingChannels) != 0 {
+	)
-		t.Fatalf("alice should only have no pending force closing "+
+	require.Zero(
-			"channels instead she has %v",
+		t.t, len(alicePendingList.WaitingCloseChannels),
-			len(alicePendingList.PendingForceClosingChannels))
+		"alice waiting close channels",
-	}
+	)
-	if len(alicePendingList.WaitingCloseChannels) != 0 {
-		t.Fatalf("alice should only have no waiting close "+
-			"channels instead she has %v",
-			len(alicePendingList.WaitingCloseChannels))
-	}
 
 	// Assert that channel is listed as abandoned.
 	closedReq := &lnrpc.ClosedChannelsRequest{
@@ -1716,45 +1687,49 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) {
 	}
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	aliceClosedList, err := net.Alice.ClosedChannels(ctxt, closedReq)
-	if err != nil {
+	require.NoError(t.t, err, "alice list closed channels")
-		t.Fatalf("unable to list closed channels: %v", err)
+	require.Len(t.t, aliceClosedList.Channels, 1, "alice closed channels")
-	}
-	if len(aliceClosedList.Channels) != 1 {
-		t.Fatalf("alice should only have a single abandoned channel, "+
-			"instead she has %v",
-			len(aliceClosedList.Channels))
-	}
 
 	// Ensure that the channel can no longer be found in the channel graph.
-	_, err = net.Alice.GetChanInfo(ctxb, &lnrpc.ChanInfoRequest{
+	err = wait.NoError(func() error {
+		_, err := net.Alice.GetChanInfo(ctxb, &lnrpc.ChanInfoRequest{
 			ChanId: chanID,
 		})
-	if !strings.Contains(err.Error(), "marked as zombie") {
+		if err == nil {
-		t.Fatalf("channel shouldn't be found in the channel " +
+			return fmt.Errorf("expected error but got nil")
-			"graph!")
 		}
 
+		if !strings.Contains(err.Error(), "marked as zombie") {
+			return fmt.Errorf("expected error to contain '%s' but "+
+				"was '%v'", "marked as zombie", err)
+		}
+
+		return nil
+	}, defaultTimeout)
+	require.NoError(t.t, err, "marked as zombie")
+
 	// Make sure the channel is no longer in the channel backup list.
-	err = wait.Predicate(func() bool {
+	err = wait.NoError(func() error {
 		bkupAfter, err := ioutil.ReadFile(net.Alice.ChanBackupPath())
 		if err != nil {
-			t.Fatalf("could not get channel backup before "+
+			return fmt.Errorf("could not get channel backup "+
-				"abandoning channel: %v", err)
+				"before abandoning channel: %v", err)
 		}
 
-		return len(bkupAfter) < len(bkupBefore)
+		if len(bkupAfter) >= len(bkupBefore) {
-	}, defaultTimeout)
+			return fmt.Errorf("expected backups after to be less "+
-	if err != nil {
+				"than %d but was %d", bkupBefore, bkupAfter)
-		t.Fatalf("channel wasn't removed from channel backup file")
 		}
 
+		return nil
+	}, defaultTimeout)
+	require.NoError(t.t, err, "channel removed from backup file")
+
 	// Calling AbandonChannel again, should result in no new errors, as the
 	// channel has already been removed.
 	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	_, err = net.Alice.AbandonChannel(ctxt, abandonChannelRequest)
-	if err != nil {
+	require.NoError(t.t, err, "abandon channel second time")
-		t.Fatalf("unable to abandon channel a second time: %v", err)
-	}
 
 	// Now that we're done with the test, the channel can be closed. This
 	// is necessary to avoid unexpected outcomes of other tests that use