From e711a217f70f4e1f41f16927940a588dd2d8e571 Mon Sep 17 00:00:00 2001
From: Silas Davis <silas@monax.io>
Date: Tue, 24 Jul 2018 09:57:04 +0100
Subject: [PATCH] Fix various test flakes

Signed-off-by: Silas Davis <silas@monax.io>
---
 acm/balance/balance.go                    | 11 ++++-
 acm/balance/balance_test.go               |  2 +-
 integration/governance/governance_test.go | 53 ++++++++++++++++++-----
 integration/tm/tm_server_test.go          | 18 +++++---
 4 files changed, 67 insertions(+), 17 deletions(-)

diff --git a/acm/balance/balance.go b/acm/balance/balance.go
index 20b5cc85..e8a927c7 100644
--- a/acm/balance/balance.go
+++ b/acm/balance/balance.go
@@ -1,6 +1,9 @@
 package balance
 
-import "fmt"
+import (
+	"fmt"
+	"sort"
+)
 
 type Balances []Balance
 
@@ -12,6 +15,11 @@ func New() Balances {
 	return []Balance{}
 }
 
+func (bs Balances) Sort() Balances {
+	sort.Stable(bs)
+	return bs
+}
+
 func (bs Balances) Len() int {
 	return len(bs)
 }
@@ -57,6 +65,7 @@ func Sum(bss ...Balances) Balances {
 	for k, v := range sumMap {
 		sum = sum.Add(k, v)
 	}
+	sort.Stable(sum)
 	return sum
 }
 
diff --git a/acm/balance/balance_test.go b/acm/balance/balance_test.go
index d6710e09..d69157d4 100644
--- a/acm/balance/balance_test.go
+++ b/acm/balance/balance_test.go
@@ -11,7 +11,7 @@ func TestSum(t *testing.T) {
 	one := New().Power(23223).Native(34).Native(1111)
 	two := New().Power(3).Native(22)
 	sum := one.Sum(two)
-	assert.Equal(t, New().Power(23226).Native(1167), sum)
+	assert.Equal(t, New().Power(23226).Native(1167).Sort(), sum)
 }
 
 func TestSort(t *testing.T) {
diff --git a/integration/governance/governance_test.go b/integration/governance/governance_test.go
index b188331c..af9d045b 100644
--- a/integration/governance/governance_test.go
+++ b/integration/governance/governance_test.go
@@ -41,7 +41,7 @@ func TestAlterValidators(t *testing.T) {
 	alterPower(vs, 8, 9931)
 
 	vs.Iterate(func(id crypto.Addressable, power *big.Int) (stop bool) {
-		_, err := govSync(t, tcli, governance.AlterPowerTx(inputAddress, id, power.Uint64()))
+		_, err := govSync(tcli, governance.AlterPowerTx(inputAddress, id, power.Uint64()))
 		require.NoError(t, err)
 		return
 	})
@@ -52,16 +52,17 @@ func TestAlterValidators(t *testing.T) {
 	assertValidatorsEqual(t, vs, vsOut)
 
 	// Remove validator from chain
-	txe, err := govSync(t, tcli, governance.AlterPowerTx(inputAddress, account(3), 0))
+	txe, err := govSync(tcli, governance.AlterPowerTx(inputAddress, account(3), 0))
 	// Mirror in our check set
 	alterPower(vs, 3, 0)
 	fmt.Println(txe.Events)
 	vsOut = getValidatorHistory(t, qcli)
 	assertValidatorsEqual(t, vs, vsOut)
 
+	// Now check Tendermint
 	waitNBlocks(t, ecli, 3)
-	height := int64(kernels[4].Blockchain.LastBlockHeight())
-	kernels[4].Node.ConfigureRPC()
+	height := int64(kernels[0].Blockchain.LastBlockHeight())
+	kernels[0].Node.ConfigureRPC()
 	tmVals, err := core.Validators(&height)
 	require.NoError(t, err)
 	vsOut = validator.NewTrimSet()
@@ -74,14 +75,36 @@ func TestAlterValidators(t *testing.T) {
 	assertValidatorsEqual(t, vs, vsOut)
 }
 
+func TestAlterValidatorsTooQuickly(t *testing.T) {
+	grpcAddress := testConfigs[0].RPC.GRPC.ListenAddress
+	inputAddress := privateAccounts[0].Address()
+	tcli := rpctest.NewTransactClient(t, grpcAddress)
+	qcli := rpctest.NewQueryClient(t, grpcAddress)
+
+	maxFlow := getMaxFlow(t, qcli)
+	acc1 := acm.GeneratePrivateAccountFromSecret("Foo1")
+	t.Logf("Changing power of new account %v to MaxFlow = %d that should succeed", acc1.Address(), maxFlow)
+
+	_, err := govSync(tcli, governance.AlterPowerTx(inputAddress, acc1, maxFlow))
+	require.NoError(t, err)
+
+	maxFlow = getMaxFlow(t, qcli)
+	power := maxFlow + 1
+	acc2 := acm.GeneratePrivateAccountFromSecret("Foo2")
+	t.Logf("Changing power of new account %v to MaxFlow + 1 = %d that should fail", acc2.Address(), power)
+
+	_, err = govSync(tcli, governance.AlterPowerTx(inputAddress, acc2, power))
+	require.Error(t, err)
+}
+
 func TestNoRootPermission(t *testing.T) {
 	grpcAddress := testConfigs[0].RPC.GRPC.ListenAddress
 	tcli := rpctest.NewTransactClient(t, grpcAddress)
 	// Account does not have Root permission
 	inputAddress := privateAccounts[4].Address()
-	_, err := govSync(t, tcli, governance.AlterPowerTx(inputAddress, account(5), 3433))
+	_, err := govSync(tcli, governance.AlterPowerTx(inputAddress, account(5), 3433))
 	require.Error(t, err)
-	assert.Contains(t, err.Error(), errors.ErrorCodePermissionDenied.Error())
+	assert.Contains(t, err.Error(), errors.PermissionDenied{Address: inputAddress, Perm: permission.Root}.Error())
 }
 
 func TestAlterAmount(t *testing.T) {
@@ -91,7 +114,7 @@ func TestAlterAmount(t *testing.T) {
 	qcli := rpctest.NewQueryClient(t, grpcAddress)
 	var amount uint64 = 18889
 	acc := account(5)
-	_, err := govSync(t, tcli, governance.AlterBalanceTx(inputAddress, acc, balance.New().Native(amount)))
+	_, err := govSync(tcli, governance.AlterBalanceTx(inputAddress, acc, balance.New().Native(amount)))
 	require.NoError(t, err)
 	ca, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: acc.Address()})
 	require.NoError(t, err)
@@ -104,7 +127,7 @@ func TestAlterPermissions(t *testing.T) {
 	tcli := rpctest.NewTransactClient(t, grpcAddress)
 	qcli := rpctest.NewQueryClient(t, grpcAddress)
 	acc := account(5)
-	_, err := govSync(t, tcli, governance.AlterPermissionsTx(inputAddress, acc, permission.Send))
+	_, err := govSync(tcli, governance.AlterPermissionsTx(inputAddress, acc, permission.Send))
 	require.NoError(t, err)
 	ca, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: acc.Address()})
 	require.NoError(t, err)
@@ -123,13 +146,22 @@ func TestCreateAccount(t *testing.T) {
 	qcli := rpctest.NewQueryClient(t, grpcAddress)
 	var amount uint64 = 18889
 	acc := acm.GeneratePrivateAccountFromSecret("we almost certainly don't exist")
-	_, err := govSync(t, tcli, governance.AlterBalanceTx(inputAddress, acc, balance.New().Native(amount)))
+	_, err := govSync(tcli, governance.AlterBalanceTx(inputAddress, acc, balance.New().Native(amount)))
 	require.NoError(t, err)
 	ca, err := qcli.GetAccount(context.Background(), &rpcquery.GetAccountParam{Address: acc.Address()})
 	require.NoError(t, err)
 	assert.Equal(t, amount, ca.Balance)
 }
 
+func getMaxFlow(t testing.TB, qcli rpcquery.QueryClient) uint64 {
+	vs, err := qcli.GetValidatorSet(context.Background(), &rpcquery.GetValidatorSetParam{})
+	require.NoError(t, err)
+	set := validator.UnpersistSet(vs.Set)
+	totalPower := set.TotalPower()
+	maxFlow := new(big.Int)
+	return maxFlow.Sub(maxFlow.Div(totalPower, big.NewInt(3)), big.NewInt(1)).Uint64()
+}
+
 func getValidatorHistory(t testing.TB, qcli rpcquery.QueryClient) *validator.Set {
 	history, err := qcli.GetValidatorSet(context.Background(), &rpcquery.GetValidatorSetParam{
 		IncludeHistory: true,
@@ -144,7 +176,7 @@ func account(i int) *acm.PrivateAccount {
 	return rpctest.PrivateAccounts[i]
 }
 
-func govSync(t testing.TB, cli rpctransact.TransactClient, tx *payload.GovTx) (*exec.TxExecution, error) {
+func govSync(cli rpctransact.TransactClient, tx *payload.GovTx) (*exec.TxExecution, error) {
 	return cli.BroadcastTxSync(context.Background(), &rpctransact.TxEnvelopeParam{
 		Payload: tx.Any(),
 	})
@@ -166,6 +198,7 @@ func waitNBlocks(t testing.TB, ecli rpcevents.ExecutionEventsClient, n int) {
 	})
 	defer stream.CloseSend()
 	for i := 0; i < n; i++ {
+		fmt.Println("block")
 		require.NoError(t, err)
 		_, err = stream.Recv()
 	}
diff --git a/integration/tm/tm_server_test.go b/integration/tm/tm_server_test.go
index 09ce3b58..c095365e 100644
--- a/integration/tm/tm_server_test.go
+++ b/integration/tm/tm_server_test.go
@@ -19,7 +19,7 @@ package tm
 
 import (
 	"context"
-	"fmt"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -194,13 +194,21 @@ func TestListValidators(t *testing.T) {
 }
 
 func TestDumpConsensusState(t *testing.T) {
-	startTime := time.Now()
 	testWithAllClients(t, func(t *testing.T, clientName string, client tmclient.RPCClient) {
 		resp, err := tmclient.DumpConsensusState(client)
 		assert.NoError(t, err)
-		assert.NotZero(t, startTime)
-		assert.Equal(t, fmt.Sprintf("/0/%d", types.RoundStepNewHeight),
-			strings.TrimLeft(resp.RoundState.HeightRoundStep, "0123456789"))
+		hrs := strings.Split(resp.RoundState.HeightRoundStep, "/")
+		require.Len(t, hrs, 3)
+		// height
+		_, err = strconv.ParseUint(hrs[0], 10, 64)
+		require.NoError(t, err)
+		// round
+		_, err = strconv.ParseUint(hrs[1], 10, 64)
+		require.NoError(t, err)
+		step, err := strconv.ParseUint(hrs[2], 10, 8)
+		require.NoError(t, err)
+
+		assert.True(t, step <= uint64(types.RoundStepCommit), "round step should be of proper enum type")
 	})
 }
 
-- 
GitLab