test: fix flaky tests + new method RetryForBlocks (#14609)

Co-authored-by: Marko <[email protected]>

CHANGELOG.md

@@ -70,6 +70,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* [#14609](https://github.com/cosmos/cosmos-sdk/pull/14609) Add RetryForBlocks method to use in tests that require waiting for a transaction to be included in a block.
 * [#14017](https://github.com/cosmos/cosmos-sdk/pull/14017) Simplify ADR-028 and `address.Module`.
     * This updates the [ADR-028](https://docs.cosmos.network/main/architecture/adr-028-public-key-addresses) and enhance the `address.Module` API to support module addresses and sub-module addresses in a backward compatible way.
 * [#14529](https://github.com/cosmos/cosmos-sdk/pull/14529) Add new property `BondDenom` to `SimulationState` struct.

client/rpc/rpc_test.go

@@ -3,6 +3,7 @@ package rpc_test
 import (
 	"context"
 	"fmt"
+	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
@@ -61,7 +62,9 @@ func (s *IntegrationTestSuite) TestCLIQueryConn() {
 	s.NoError(err)
 
 	blockHeight := header.Get(grpctypes.GRPCBlockHeightHeader)
-	s.Require().Equal([]string{"1"}, blockHeight)
+	height, err := strconv.Atoi(blockHeight[0])
+	s.Require().NoError(err)
+	s.Require().GreaterOrEqual(height, 1) // at least the 1st block
 
 	s.Equal("hello", res.Message)
 }

tests/e2e/auth/suite.go

@@ -310,7 +310,6 @@ func (s *E2ETestSuite) TestCLISignBatch() {
 	)
 	s.Require().NoError(err)
 	s.Require().NoError(s.network.WaitForNextBlock())
-	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// fetch the sequence after a tx, should be incremented.
 	_, seq1, err := val.ClientCtx.AccountRetriever.GetAccountNumberSequence(val.ClientCtx, val.Address)
@@ -530,9 +529,15 @@ func (s *E2ETestSuite) TestCLIQueryTxCmdByHash() {
 		s.Run(tc.name, func() {
 			cmd := authcli.QueryTxCmd()
 			clientCtx := val.ClientCtx
+			var (
+				out testutil.BufferWriter
+				err error
+			)
 
-			out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
-
+			err = s.network.RetryForBlocks(func() error {
+				out, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
+				return err
+			}, 2)
 			if tc.expectErr {
 				s.Require().Error(err)
 				s.Require().NotEqual("internal", err.Error())
@@ -568,7 +573,10 @@ func (s *E2ETestSuite) TestCLIQueryTxCmdByEvents() {
 	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// Query the tx by hash to get the inner tx.
-	out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, authcli.QueryTxCmd(), []string{txRes.TxHash, fmt.Sprintf("--%s=json", flags.FlagOutput)})
+	err = s.network.RetryForBlocks(func() error {
+		out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, authcli.QueryTxCmd(), []string{txRes.TxHash, fmt.Sprintf("--%s=json", flags.FlagOutput)})
+		return err
+	}, 3)
 	s.Require().NoError(err)
 	s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes))
 	protoTx := txRes.GetTx().(*tx.Tx)
@@ -685,7 +693,10 @@ func (s *E2ETestSuite) TestCLIQueryTxsCmdByEvents() {
 	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// Query the tx by hash to get the inner tx.
-	out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, authcli.QueryTxCmd(), []string{txRes.TxHash, fmt.Sprintf("--%s=json", flags.FlagOutput)})
+	err = s.network.RetryForBlocks(func() error {
+		out, err = clitestutil.ExecTestCLICmd(val.ClientCtx, authcli.QueryTxCmd(), []string{txRes.TxHash, fmt.Sprintf("--%s=json", flags.FlagOutput)})
+		return err
+	}, 3)
 	s.Require().NoError(err)
 	s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes))
 
@@ -865,7 +876,10 @@ func (s *E2ETestSuite) TestCLISendGenerateSignAndBroadcast() {
 	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// Ensure destiny account state
-	resp, err = clitestutil.QueryBalancesExec(val1.ClientCtx, addr)
+	err = s.network.RetryForBlocks(func() error {
+		resp, err = clitestutil.QueryBalancesExec(val1.ClientCtx, addr)
+		return err
+	}, 3)
 	s.Require().NoError(err)
 
 	err = val1.ClientCtx.Codec.UnmarshalJSON(resp.Bytes(), &balRes)
@@ -1154,7 +1168,9 @@ func (s *E2ETestSuite) TestCLIMultisign() {
 	s.Require().NoError(err)
 
 	var balRes banktypes.QueryAllBalancesResponse
-	err = val1.ClientCtx.Codec.UnmarshalJSON(resp.Bytes(), &balRes)
+	err = s.network.RetryForBlocks(func() error {
+		return val1.ClientCtx.Codec.UnmarshalJSON(resp.Bytes(), &balRes)
+	}, 3)
 	s.Require().NoError(err)
 	s.Require().True(sendTokens.Amount.Equal(balRes.Balances.AmountOf(s.cfg.BondDenom)))
 
@@ -1683,7 +1699,6 @@ func (s *E2ETestSuite) TestSignWithMultiSignersAminoJSON() {
 	)
 	require.NoError(err)
 	require.NoError(s.network.WaitForNextBlock())
-	require.NoError(s.network.WaitForNextBlock())
 
 	var txRes sdk.TxResponse
 	require.NoError(val0.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes))

tests/e2e/distribution/withdraw_all_suite.go

@@ -10,6 +10,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/crypto/hd"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+	"github.com/cosmos/cosmos-sdk/testutil"
 	clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -94,21 +95,31 @@ func (s *WithdrawAllTestSuite) TestNewWithdrawAllRewardsGenerateOnly() {
 	}
 	_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 	require.NoError(err)
-	require.NoError(s.network.WaitForNextBlock())
 
-	args = []string{
-		fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr.String()),
-		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
-		fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
-		fmt.Sprintf("--%s=1", cli.FlagMaxMessagesPerTx),
-		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
-		fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
-	}
-	cmd = cli.NewWithdrawAllRewardsCmd()
-	out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+	var out testutil.BufferWriter
+	err = s.network.RetryForBlocks(func() error {
+		args = []string{
+			fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr.String()),
+			fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+			fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
+			fmt.Sprintf("--%s=1", cli.FlagMaxMessagesPerTx),
+			fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
+			fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+		}
+		cmd = cli.NewWithdrawAllRewardsCmd()
+		out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+		if err != nil {
+			return err
+		}
+
+		// expect 2 transactions in the generated file when --max-msgs in a tx set 1.
+		txLen := len(strings.Split(strings.Trim(out.String(), "\n"), "\n"))
+		if txLen != 2 {
+			return fmt.Errorf("expected 2 transactions in the generated file, got %d", txLen)
+		}
+		return nil
+	}, 3)
 	require.NoError(err)
-	// expect 2 transactions in the generated file when --max-msgs in a tx set 1.
-	s.Require().Equal(2, len(strings.Split(strings.Trim(out.String(), "\n"), "\n")))
 
 	args = []string{
 		fmt.Sprintf("--%s=%s", flags.FlagFrom, newAddr.String()),

tests/e2e/feegrant/suite.go

@@ -102,7 +102,6 @@ func (s *E2ETestSuite) createGrant(granter, grantee sdk.Address) {
 	_, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
 	s.Require().NoError(err)
 	s.Require().NoError(s.network.WaitForNextBlock())
-	s.Require().NoError(s.network.WaitForNextBlock())
 }
 
 func (s *E2ETestSuite) TearDownSuite() {

tests/e2e/gov/deposits.go

@@ -2,6 +2,7 @@ package gov
 
 import (
 	"fmt"
+	"strconv"
 	"time"
 
 	"github.com/stretchr/testify/suite"
@@ -19,10 +20,8 @@ import (
 type DepositTestSuite struct {
 	suite.Suite
 
-	cfg         network.Config
-	network     *network.Network
-	deposits    sdk.Coins
-	proposalIDs []string
+	cfg     network.Config
+	network *network.Network
 }
 
 func NewDepositTestSuite(cfg network.Config) *DepositTestSuite {
@@ -35,38 +34,9 @@ func (s *DepositTestSuite) SetupSuite() {
 	var err error
 	s.network, err = network.New(s.T(), s.T().TempDir(), s.cfg)
 	s.Require().NoError(err)
-
-	s.Require().NoError(s.network.WaitForNextBlock())
-
-	val := s.network.Validators[0]
-
-	deposits := sdk.Coins{
-		sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0)),
-		sdk.NewCoin(s.cfg.BondDenom, v1.DefaultMinDepositTokens),
-		sdk.NewCoin(s.cfg.BondDenom, v1.DefaultMinDepositTokens.Sub(sdk.NewInt(50))),
-	}
-	s.deposits = deposits
-
-	// create 2 proposals for testing
-	for i := 0; i < len(deposits); i++ {
-		id := i + 1
-		deposit := deposits[i]
-
-		s.submitProposal(val, deposit, id)
-		s.proposalIDs = append(s.proposalIDs, fmt.Sprintf("%d", id))
-	}
 }
 
-func (s *DepositTestSuite) SetupNewSuite() {
-	s.T().Log("setting up new test suite")
-
-	var err error
-	s.network, err = network.New(s.T(), s.T().TempDir(), s.cfg)
-	s.Require().NoError(err)
-	s.Require().NoError(s.network.WaitForNextBlock())
-}
-
-func (s *DepositTestSuite) submitProposal(val *network.Validator, initialDeposit sdk.Coin, id int) {
+func (s *DepositTestSuite) submitProposal(val *network.Validator, initialDeposit sdk.Coin, name string) uint64 {
 	var exactArgs []string
 
 	if !initialDeposit.IsZero() {
@@ -76,13 +46,25 @@ func (s *DepositTestSuite) submitProposal(val *network.Validator, initialDeposit
 	_, err := govclitestutil.MsgSubmitLegacyProposal(
 		val.ClientCtx,
 		val.Address.String(),
-		fmt.Sprintf("Text Proposal %d", id),
+		fmt.Sprintf("Text Proposal %s", name),
 		"Where is the title!?",
 		v1beta1.ProposalTypeText,
 		exactArgs...,
 	)
 	s.Require().NoError(err)
 	s.Require().NoError(s.network.WaitForNextBlock())
+
+	// query proposals, return the last's id
+	cmd := cli.GetCmdQueryProposals()
+	args := []string{fmt.Sprintf("--%s=json", flags.FlagOutput)}
+	res, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
+	s.Require().NoError(err)
+
+	var proposals v1.QueryProposalsResponse
+	err = s.cfg.Codec.UnmarshalJSON(res.Bytes(), &proposals)
+	s.Require().NoError(err)
+
+	return proposals.Proposals[len(proposals.Proposals)-1].Id
 }
 
 func (s *DepositTestSuite) TearDownSuite() {
@@ -93,7 +75,10 @@ func (s *DepositTestSuite) TearDownSuite() {
 func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() {
 	val := s.network.Validators[0]
 	clientCtx := val.ClientCtx
-	proposalID := s.proposalIDs[0]
+
+	// submit proposal without initial deposit
+	id := s.submitProposal(val, sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0)), "TestQueryDepositsWithoutInitialDeposit")
+	proposalID := strconv.FormatUint(id, 10)
 
 	// deposit amount
 	depositAmount := sdk.NewCoin(s.cfg.BondDenom, v1.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String()
@@ -105,7 +90,6 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() {
 	deposit := s.queryDeposit(val, proposalID, false, "")
 	s.Require().NotNil(deposit)
 	s.Require().Equal(sdk.Coins(deposit.Amount).String(), depositAmount)
-	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// query deposits
 	deposits := s.queryDeposits(val, proposalID, false, "")
@@ -117,38 +101,50 @@ func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() {
 
 func (s *DepositTestSuite) TestQueryDepositsWithInitialDeposit() {
 	val := s.network.Validators[0]
-	proposalID := s.proposalIDs[1]
+	depositAmount := sdk.NewCoin(s.cfg.BondDenom, v1.DefaultMinDepositTokens)
+
+	// submit proposal with an initial deposit
+	id := s.submitProposal(val, depositAmount, "TestQueryDepositsWithInitialDeposit")
+	proposalID := strconv.FormatUint(id, 10)
 
 	// query deposit
 	deposit := s.queryDeposit(val, proposalID, false, "")
 	s.Require().NotNil(deposit)
-	s.Require().Equal(sdk.Coins(deposit.Amount).String(), s.deposits[1].String())
+	s.Require().Equal(sdk.Coins(deposit.Amount).String(), depositAmount.String())
 	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// query deposits
 	deposits := s.queryDeposits(val, proposalID, false, "")
 	s.Require().NotNil(deposits)
 	s.Require().Len(deposits.Deposits, 1)
 	// verify initial deposit
-	s.Require().Equal(sdk.Coins(deposits.Deposits[0].Amount).String(), s.deposits[1].String())
+	s.Require().Equal(sdk.Coins(deposits.Deposits[0].Amount).String(), depositAmount.String())
 }
 
 func (s *DepositTestSuite) TestQueryProposalAfterVotingPeriod() {
 	val := s.network.Validators[0]
-	clientCtx := val.ClientCtx
-	proposalID := s.proposalIDs[2]
+	depositAmount := sdk.NewCoin(s.cfg.BondDenom, v1.DefaultMinDepositTokens.Sub(sdk.NewInt(50)))
+
+	// submit proposal with an initial deposit
+	id := s.submitProposal(val, depositAmount, "TestQueryProposalAfterVotingPeriod")
+	proposalID := strconv.FormatUint(id, 10)
+
+	args := []string{fmt.Sprintf("--%s=json", flags.FlagOutput)}
+	cmd := cli.GetCmdQueryProposals()
+	_, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
+	s.Require().NoError(err)
 
 	// query proposal
-	args := []string{proposalID, fmt.Sprintf("--%s=json", flags.FlagOutput)}
-	cmd := cli.GetCmdQueryProposal()
-	_, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+	args = []string{proposalID, fmt.Sprintf("--%s=json", flags.FlagOutput)}
+	cmd = cli.GetCmdQueryProposal()
+	_, err = clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
 	s.Require().NoError(err)
 
 	// waiting for deposit and voting period to end
-	time.Sleep(20 * time.Second)
+	time.Sleep(25 * time.Second)
 
 	// query proposal
-	_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
+	_, err = clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
 	s.Require().Error(err)
 	s.Require().Contains(err.Error(), fmt.Sprintf("proposal %s doesn't exist", proposalID))