Merge pull request #1850 from cosmos/bez/1675-non-deterministic-cover

R4R: Fix Non-Deterministic Test Coverage

PENDING.md

@@ -61,3 +61,4 @@ BUG FIXES
 *  \#1799 Fix `gaiad export`
 *  \#1828 Force user to specify amount on create-validator command by removing default
 *  \#1839 Fixed bug where intra-tx counter wasn't set correctly for genesis validators
+* [tests] \#1675 Fix non-deterministic `test_cover` 

client/lcd/lcd_test.go

@@ -35,7 +35,7 @@ func init() {
 
 func TestKeys(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -65,7 +65,7 @@ func TestKeys(t *testing.T) {
 	require.NoError(t, err, "Failed to return a correct bech32 address")
 
 	// test if created account is the correct account
-	expectedInfo, _ := GetKB(t).CreateKey(newName, seed, newPassword)
+	expectedInfo, _ := GetKeyBase(t).CreateKey(newName, seed, newPassword)
 	expectedAccount := sdk.AccAddress(expectedInfo.GetPubKey().Address().Bytes())
 	assert.Equal(t, expectedAccount.String(), addr2Bech32)
 
@@ -223,7 +223,7 @@ func TestValidators(t *testing.T) {
 
 func TestCoinSend(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -265,7 +265,7 @@ func TestCoinSend(t *testing.T) {
 
 func TestIBCTransfer(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -294,7 +294,7 @@ func TestIBCTransfer(t *testing.T) {
 
 func TestTxs(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -376,7 +376,7 @@ func TestValidatorsQuery(t *testing.T) {
 
 func TestBonding(t *testing.T) {
 	name, password, denom := "test", "1234567890", "steak"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, pks, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -426,7 +426,7 @@ func TestBonding(t *testing.T) {
 
 func TestSubmitProposal(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -448,7 +448,7 @@ func TestSubmitProposal(t *testing.T) {
 
 func TestDeposit(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -482,7 +482,7 @@ func TestDeposit(t *testing.T) {
 
 func TestVote(t *testing.T) {
 	name, password := "test", "1234567890"
-	addr, seed := CreateAddr(t, "test", password, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -520,7 +520,7 @@ func TestVote(t *testing.T) {
 
 func TestUnrevoke(t *testing.T) {
 	_, password := "test", "1234567890"
-	addr, _ := CreateAddr(t, "test", password, GetKB(t))
+	addr, _ := CreateAddr(t, "test", password, GetKeyBase(t))
 	cleanup, pks, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr})
 	defer cleanup()
 
@@ -537,8 +537,8 @@ func TestUnrevoke(t *testing.T) {
 func TestProposalsQuery(t *testing.T) {
 	name, password1 := "test", "1234567890"
 	name2, password2 := "test2", "1234567890"
-	addr, seed := CreateAddr(t, "test", password1, GetKB(t))
+	addr, seed := CreateAddr(t, "test", password1, GetKeyBase(t))
-	addr2, seed2 := CreateAddr(t, "test2", password2, GetKB(t))
+	addr2, seed2 := CreateAddr(t, "test2", password2, GetKeyBase(t))
 	cleanup, _, port := InitializeTestLCD(t, 1, []sdk.AccAddress{addr, addr2})
 	defer cleanup()
 

client/lcd/test_helpers.go

@@ -12,10 +12,19 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/cosmos/cosmos-sdk/client"
+	keys "github.com/cosmos/cosmos-sdk/client/keys"
+	gapp "github.com/cosmos/cosmos-sdk/cmd/gaia/app"
+	crkeys "github.com/cosmos/cosmos-sdk/crypto/keys"
+	"github.com/cosmos/cosmos-sdk/server"
+	"github.com/cosmos/cosmos-sdk/tests"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/wire"
+	"github.com/cosmos/cosmos-sdk/x/auth"
+
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/require"
 
-	crkeys "github.com/cosmos/cosmos-sdk/crypto/keys"
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmcfg "github.com/tendermint/tendermint/config"
 	"github.com/tendermint/tendermint/crypto"
@@ -28,29 +37,21 @@ import (
 	"github.com/tendermint/tendermint/proxy"
 	tmrpc "github.com/tendermint/tendermint/rpc/lib/server"
 	tmtypes "github.com/tendermint/tendermint/types"
-
-	"github.com/cosmos/cosmos-sdk/client"
-	keys "github.com/cosmos/cosmos-sdk/client/keys"
-	gapp "github.com/cosmos/cosmos-sdk/cmd/gaia/app"
-	"github.com/cosmos/cosmos-sdk/server"
-	"github.com/cosmos/cosmos-sdk/tests"
-	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/wire"
-	"github.com/cosmos/cosmos-sdk/x/auth"
 )
 
-// f**ing long, but unique for each test
+// makePathname creates a unique pathname for each test. It will panic if it
+// cannot get the current working directory.
 func makePathname() string {
-	// get path
 	p, err := os.Getwd()
 	if err != nil {
 		panic(err)
 	}
+
 	sep := string(filepath.Separator)
 	return strings.Replace(p, sep, "_", -1)
 }
 
-// GetConfig returns a config for the test cases as a singleton
+// GetConfig returns a Tendermint config for the test cases.
 func GetConfig() *tmcfg.Config {
 	pathname := makePathname()
 	config := tmcfg.ResetTestRoot(pathname)
@@ -59,42 +60,61 @@ func GetConfig() *tmcfg.Config {
 	if err != nil {
 		panic(err)
 	}
+
 	rcpAddr, _, err := server.FreeTCPAddr()
 	if err != nil {
 		panic(err)
 	}
 
+	grpcAddr, _, err := server.FreeTCPAddr()
+	if err != nil {
+		panic(err)
+	}
+
 	config.P2P.ListenAddress = tmAddr
 	config.RPC.ListenAddress = rcpAddr
+	config.RPC.GRPCListenAddress = grpcAddr
+
 	return config
 }
 
-// get the lcd test keybase
+// GetKeyBase returns the LCD test keybase. It also requires that a directory
-// note can't use a memdb because the request is expecting to interact with the default location
+// could be made and a keybase could be fetched.
-func GetKB(t *testing.T) crkeys.Keybase {
+//
+// NOTE: memDB cannot be used because the request is expecting to interact with
+// the default location.
+func GetKeyBase(t *testing.T) crkeys.Keybase {
 	dir, err := ioutil.TempDir("", "lcd_test")
 	require.NoError(t, err)
+
 	viper.Set(cli.HomeFlag, dir)
-	keybase, err := keys.GetKeyBase() // dbm.NewMemDB()) // :(
+
+	keybase, err := keys.GetKeyBase()
 	require.NoError(t, err)
+
 	return keybase
 }
 
-// add an address to the store return name and password
+// CreateAddr adds an address to the key store and returns an address and seed.
-func CreateAddr(t *testing.T, name, password string, kb crkeys.Keybase) (addr sdk.AccAddress, seed string) {
+// It also requires that the key could be created.
-	var info crkeys.Info
+func CreateAddr(t *testing.T, name, password string, kb crkeys.Keybase) (sdk.AccAddress, string) {
-	var err error
+	var (
+		err  error
+		info crkeys.Info
+		seed string
+	)
+
 	info, seed, err = kb.CreateMnemonic(name, crkeys.English, password, crkeys.Secp256k1)
 	require.NoError(t, err)
-	addr = sdk.AccAddress(info.GetPubKey().Address())
+
-	return
+	return sdk.AccAddress(info.GetPubKey().Address()), seed
 }
 
-// strt TM and the LCD in process, listening on their respective sockets
+// InitializeTestLCD starts Tendermint and the LCD in process, listening on
-//   nValidators = number of validators
+// their respective sockets where nValidators is the total number of validators
-//   initAddrs = accounts to initialize with some steaks
+// and initAddrs are the accounts to initialize with some steak tokens. It
-func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress) (cleanup func(), validatorsPKs []crypto.PubKey, port string) {
+// returns a cleanup function, a set of validator public keys, and a port.
-
+func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress) (func(), []crypto.PubKey, string) {
 	config := GetConfig()
 	config.Consensus.TimeoutCommit = 100
 	config.Consensus.SkipTimeoutCommit = false
@@ -102,9 +122,11 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress
 
 	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
 	logger = log.NewFilter(logger, log.AllowDebug())
+
 	privValidatorFile := config.PrivValidatorFile()
 	privVal := pvm.LoadOrGenFilePV(privValidatorFile)
 	privVal.Reset()
+
 	db := dbm.NewMemDB()
 	app := gapp.NewGaiaApp(logger, db, nil)
 	cdc = gapp.MakeCodec()
@@ -113,10 +135,10 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress
 	genDoc, err := tmtypes.GenesisDocFromFile(genesisFile)
 	require.NoError(t, err)
 
-	// add more validators
 	if nValidators < 1 {
 		panic("InitializeTestLCD must use at least one validator")
 	}
+
 	for i := 1; i < nValidators; i++ {
 		genDoc.Validators = append(genDoc.Validators,
 			tmtypes.GenesisValidator{
@@ -127,14 +149,18 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress
 		)
 	}
 
-	// NOTE it's bad practice to reuse pk address for the owner address but doing in the
+	var validatorsPKs []crypto.PubKey
-	// test for simplicity
+
+	// NOTE: It's bad practice to reuse public key address for the owner
+	// address but doing in the test for simplicity.
 	var appGenTxs []json.RawMessage
 	for _, gdValidator := range genDoc.Validators {
 		pk := gdValidator.PubKey
-		validatorsPKs = append(validatorsPKs, pk) // append keys for output
+		validatorsPKs = append(validatorsPKs, pk)
+
 		appGenTx, _, _, err := gapp.GaiaAppGenTxNF(cdc, pk, sdk.AccAddress(pk.Address()), "test_val1")
 		require.NoError(t, err)
+
 		appGenTxs = append(appGenTxs, appGenTx)
 	}
 
@@ -154,79 +180,87 @@ func InitializeTestLCD(t *testing.T, nValidators int, initAddrs []sdk.AccAddress
 	require.NoError(t, err)
 	genDoc.AppState = appState
 
-	// LCD listen address
+	listenAddr, port, err := server.FreeTCPAddr()
-	var listenAddr string
-	listenAddr, port, err = server.FreeTCPAddr()
 	require.NoError(t, err)
 
-	// XXX: need to set this so LCD knows the tendermint node address!
+	// XXX: Need to set this so LCD knows the tendermint node address!
 	viper.Set(client.FlagNode, config.RPC.ListenAddress)
 	viper.Set(client.FlagChainID, genDoc.ChainID)
 
 	node, err := startTM(config, logger, genDoc, privVal, app)
 	require.NoError(t, err)
+
 	lcd, err := startLCD(logger, listenAddr, cdc)
 	require.NoError(t, err)
 
-	//time.Sleep(time.Second)
-	//tests.WaitForHeight(2, port)
 	tests.WaitForLCDStart(port)
 	tests.WaitForHeight(1, port)
 
-	// for use in defer
+	cleanup := func() {
-	cleanup = func() {
+		logger.Debug("cleaning up LCD initialization")
 		node.Stop()
 		node.Wait()
 		lcd.Close()
 	}
 
-	return
+	return cleanup, validatorsPKs, port
 }
 
-// Create & start in-process tendermint node with memdb
+// startTM creates and starts an in-process Tendermint node with memDB and
-// and in-process abci application.
+// in-process ABCI application. It returns the new node or any error that
-// TODO: need to clean up the WAL dir or enable it to be not persistent
+// occurred.
-func startTM(tmcfg *tmcfg.Config, logger log.Logger, genDoc *tmtypes.GenesisDoc, privVal tmtypes.PrivValidator, app abci.Application) (*nm.Node, error) {
+//
+// TODO: Clean up the WAL dir or enable it to be not persistent!
+func startTM(
+	tmcfg *tmcfg.Config, logger log.Logger, genDoc *tmtypes.GenesisDoc,
+	privVal tmtypes.PrivValidator, app abci.Application,
+) (*nm.Node, error) {
 	genDocProvider := func() (*tmtypes.GenesisDoc, error) { return genDoc, nil }
 	dbProvider := func(*nm.DBContext) (dbm.DB, error) { return dbm.NewMemDB(), nil }
-	n, err := nm.NewNode(tmcfg,
+	node, err := nm.NewNode(
+		tmcfg,
 		privVal,
 		proxy.NewLocalClientCreator(app),
 		genDocProvider,
 		dbProvider,
 		nm.DefaultMetricsProvider,
-		logger.With("module", "node"))
+		logger.With("module", "node"),
+	)
 	if err != nil {
 		return nil, err
 	}
 
-	err = n.Start()
+	err = node.Start()
 	if err != nil {
 		return nil, err
 	}
 
-	// wait for rpc
 	tests.WaitForRPC(tmcfg.RPC.ListenAddress)
-
 	logger.Info("Tendermint running!")
-	return n, err
+
+	return node, err
 }
 
-// start the LCD. note this blocks!
+// startLCD starts the LCD.
+//
+// NOTE: This causes the thread to block.
 func startLCD(logger log.Logger, listenAddr string, cdc *wire.Codec) (net.Listener, error) {
-	handler := createHandler(cdc)
+	return tmrpc.StartHTTPServer(listenAddr, createHandler(cdc), logger, tmrpc.Config{})
-	return tmrpc.StartHTTPServer(listenAddr, handler, logger, tmrpc.Config{})
 }
 
-// make a test lcd test request
+// Request makes a test LCD test request. It returns a response object and a
+// stringified response body.
 func Request(t *testing.T, port, method, path string, payload []byte) (*http.Response, string) {
-	var res *http.Response
+	var (
-	var err error
+		err error
+		res *http.Response
+	)
 	url := fmt.Sprintf("http://localhost:%v%v", port, path)
+
 	req, err := http.NewRequest(method, url, bytes.NewBuffer(payload))
 	require.Nil(t, err)
+
 	res, err = http.DefaultClient.Do(req)
-	//	res, err = http.Post(url, "application/json", bytes.NewBuffer(payload))
 	require.Nil(t, err)
 
 	output, err := ioutil.ReadAll(res.Body)