From 23a04c280274df7e723a3c2306b68e02586708e6 Mon Sep 17 00:00:00 2001
From: Pavel Zbitskiy <65323360+algorandskiy@users.noreply.github.com>
Date: Thu, 8 Aug 2024 16:49:42 -0400
Subject: [PATCH] tests: flaky tests fixes (#6098)

---
 ledger/ledger_test.go                         | 46 ++++++++++---------
 network/p2p/p2p_test.go                       |  8 +++-
 .../scripts/e2e_subs/goal-partkey-commands.sh | 15 ++++--
 3 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/ledger/ledger_test.go b/ledger/ledger_test.go
index c4bb74fcb5..e73d648b4d 100644
--- a/ledger/ledger_test.go
+++ b/ledger/ledger_test.go
@@ -2915,6 +2915,12 @@ func testVotersReloadFromDiskAfterOneStateProofCommitted(t *testing.T, cfg confi
 	require.NoError(t, err)
 	defer l.Close()
 
+	// quit the commitSyncer goroutine: this test flushes manually with triggerTrackerFlush
+	l.trackers.ctxCancel()
+	l.trackers.ctxCancel = nil
+	<-l.trackers.commitSyncerClosed
+	l.trackers.commitSyncerClosed = nil
+
 	blk := genesisInitState.Block
 
 	sp := bookkeeping.StateProofTrackingData{
@@ -2929,6 +2935,9 @@ func testVotersReloadFromDiskAfterOneStateProofCommitted(t *testing.T, cfg confi
 		blk.BlockHeader.Round++
 		err = l.AddBlock(blk, agreement.Certificate{})
 		require.NoError(t, err)
+		if i > 0 && i%100 == 0 {
+			triggerTrackerFlush(t, l)
+		}
 	}
 
 	// we simulate that the stateproof for round 512 is confirmed on chain, and we can move to the next one.
@@ -2941,31 +2950,12 @@ func testVotersReloadFromDiskAfterOneStateProofCommitted(t *testing.T, cfg confi
 		blk.BlockHeader.Round++
 		err = l.AddBlock(blk, agreement.Certificate{})
 		require.NoError(t, err)
-	}
-
-	// wait all pending commits to finish
-	l.trackers.accountsWriting.Wait()
-
-	// quit the commitSyncer goroutine: this test flushes manually with triggerTrackerFlush
-	l.trackers.ctxCancel()
-	l.trackers.ctxCancel = nil
-	<-l.trackers.commitSyncerClosed
-	l.trackers.commitSyncerClosed = nil
-
-	// it is possible a commmit was scheduled while commitSyncer was closing so that there is one pending task
-	// that required to be done before before the ledger can be closed, so drain the queue
-outer:
-	for {
-		select {
-		case <-l.trackers.deferredCommits:
-			log.Info("drained deferred commit")
-			l.trackers.accountsWriting.Done()
-		default:
-			break outer
+		if i%100 == 0 {
+			triggerTrackerFlush(t, l)
 		}
 	}
 
-	// flush one final time
+	// flush remaining blocks
 	triggerTrackerFlush(t, l)
 
 	var vtSnapshot map[basics.Round]*ledgercore.VotersForRound
@@ -2982,6 +2972,18 @@ outer:
 		require.NotContains(t, vtSnapshot, basics.Round(240))
 	}()
 
+	t.Log("reloading ledger")
+	// drain any deferred commits since AddBlock above triggered scheduleCommit
+outer:
+	for {
+		select {
+		case <-l.trackers.deferredCommits:
+			l.trackers.accountsWriting.Done()
+		default:
+			break outer
+		}
+	}
+
 	err = l.reloadLedger()
 	require.NoError(t, err)
 
diff --git a/network/p2p/p2p_test.go b/network/p2p/p2p_test.go
index 2da5782afc..021a5fbd8b 100644
--- a/network/p2p/p2p_test.go
+++ b/network/p2p/p2p_test.go
@@ -290,7 +290,13 @@ func TestP2PMakeHostAddressFilter(t *testing.T) {
 		mala, err := multiaddr.NewMultiaddr(la)
 		require.NoError(t, err)
 		host.Network().Listen(mala)
-		require.Empty(t, host.Addrs())
+		addrs := host.Addrs()
+		if len(addrs) > 0 {
+			// CI servers might have a single public IP interface, validate if this is a case
+			for _, a := range addrs {
+				require.True(t, manet.IsPublicAddr(a))
+			}
+		}
 		host.Close()
 	}
 
diff --git a/test/scripts/e2e_subs/goal-partkey-commands.sh b/test/scripts/e2e_subs/goal-partkey-commands.sh
index dd60d44016..b333a0e8aa 100755
--- a/test/scripts/e2e_subs/goal-partkey-commands.sh
+++ b/test/scripts/e2e_subs/goal-partkey-commands.sh
@@ -63,7 +63,8 @@ fail_test () {
 
 create_and_fund_account () {
   set +x  # disable command echoing to hide the account funding output
-  local TEMP_ACCT=$(${gcmd} account new|awk '{ print $6 }')
+  local TEMP_ACCT
+  TEMP_ACCT=$(${gcmd} account new|awk '{ print $6 }')
   SEND_OUTOUT=$(${gcmd} clerk send -f "$INITIAL_ACCOUNT" -t "$TEMP_ACCT" -a 1000000 2>&1)
   if [[ $SEND_OUTOUT == *"Couldn't broadcast tx"* ]]; then
     fail_test "Failed to fund account: $SEND_OUTOUT"
@@ -77,17 +78,21 @@ create_and_fund_account () {
 # $2 - a participation id
 # $3 - error message
 verify_registered_state () {
+  SEARCH_STATE=$(echo "$1" | xargs)
+  SEARCH_KEY=$(echo "$2" | xargs)
+  SEARCH_INVOKE_CONTEXT=$(echo "$3" | xargs)
+
   # look for participation ID anywhere in the partkeyinfo output
   PARTKEY_OUTPUT=$(${gcmd} account partkeyinfo)
-  if ! echo "$PARTKEY_OUTPUT" | grep -q "$2"; then
-    fail_test "Key $2 was not installed properly for cmd '$3':\n$PARTKEY_OUTPUT"
+  if ! echo "$PARTKEY_OUTPUT" | grep -q -F "$SEARCH_KEY"; then
+    fail_test "Key $SEARCH_KEY was not installed properly for cmd '$SEARCH_INVOKE_CONTEXT':\n$PARTKEY_OUTPUT"
   fi
 
   # looking for yes/no, and the 8 character head of participation id in this line:
   # yes         LFMT...RHJQ  4UPT6AQC...               4            0     3000
   LISTKEY_OUTPUT=$(${gcmd} account listpartkeys)
-  if ! echo "$LISTKEY_OUTPUT" | grep -q "$1.*$(echo "$2" | cut -c1-8)"; then
-    fail_test "Unexpected key $2 state ($1) for cmd '$3':\n$LISTKEY_OUTPUT"
+  if ! echo "$LISTKEY_OUTPUT" | grep -q "$SEARCH_STATE.*$(echo "$SEARCH_KEY" | cut -c1-8)"; then
+    fail_test "Unexpected key $SEARCH_KEY state (looked for $SEARCH_STATE ) for cmd '$SEARCH_INVOKE_CONTEXT':\n$LISTKEY_OUTPUT"
   fi
 }