Merge PR #3517: Increasing test coverage in keys/client package

.circleci/config.yml

@@ -204,7 +204,7 @@ jobs:
             export VERSION="$(git describe --tags --long | sed 's/v\(.*\)/\1/')"
             for pkg in $(go list ./... | grep -v github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test | grep -v '/simulation' | circleci tests split --split-by=timings); do
               id=$(echo "$pkg" | sed 's|[/.]|_|g')
-              GOCACHE=off go test -timeout 8m -race -coverprofile=/tmp/workspace/profiles/$id.out -covermode=atomic "$pkg" | tee "/tmp/logs/$id-$RANDOM.log"
+              GOCACHE=off go test -timeout 8m -race -coverprofile=/tmp/workspace/profiles/$id.out -covermode=atomic -tags='ledger test_ledger_mock' "$pkg" | tee "/tmp/logs/$id-$RANDOM.log"
             done
       - persist_to_workspace:
           root: /tmp/workspace

Makefile

@@ -145,10 +145,13 @@ test_cli:
 	@go test -p 4 `go list github.com/cosmos/cosmos-sdk/cmd/gaia/cli_test` -tags=cli_test
 
 test_ledger:
-	@go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger test_real_ledger'
+    # First test with mock
+	@go test `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger test_ledger_mock'
+    # Now test with a real device
+	@go test -v `go list github.com/cosmos/cosmos-sdk/crypto` -tags='cgo ledger'
 
 test_unit:
-	@VERSION=$(VERSION) go test $(PACKAGES_NOSIMULATION)
+	@VERSION=$(VERSION) go test $(PACKAGES_NOSIMULATION) -tags='test_ledger_mock'
 
 test_race:
 	@VERSION=$(VERSION) go test -race $(PACKAGES_NOSIMULATION)

PENDING.md

@@ -77,6 +77,7 @@ IMPROVEMENTS
   * [\#3497](https://github.com/cosmos/cosmos-sdk/issues/3497) `gaiad gentx` supports `--ip` and `--node-id` flags to override defaults.
   * [\#3518](https://github.com/cosmos/cosmos-sdk/issues/3518) Fix flow in
   `keys add` to show the mnemonic by default.
+  * [\#3517](https://github.com/cosmos/cosmos-sdk/pull/3517) Increased test coverage
 
 * Gaia
   * [\#3418](https://github.com/cosmos/cosmos-sdk/issues/3418) Add vesting account

baseapp/baseapp.go

@@ -770,7 +770,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 			)
 
 			if ctx.BlockGasMeter().GasConsumed() < startingGas {
-				panic(sdk.ErrorGasOverflow{"tx gas summation"})
+				panic(sdk.ErrorGasOverflow{Descriptor: "tx gas summation"})
 			}
 		}
 	}()

client/input.go

@@ -15,10 +15,26 @@ import (
 // MinPassLength is the minimum acceptable password length
 const MinPassLength = 8
 
+var currentStdin *bufio.Reader
+
+func init() {
+	currentStdin = bufio.NewReader(os.Stdin)
+}
+
 // BufferStdin is used to allow reading prompts for stdin
 // multiple times, when we read from non-tty
 func BufferStdin() *bufio.Reader {
-	return bufio.NewReader(os.Stdin)
+	return currentStdin
+}
+
+// OverrideStdin allows to temporarily override stdin
+func OverrideStdin(newStdin *bufio.Reader) (cleanUp func()) {
+	prevStdin := currentStdin
+	currentStdin = newStdin
+	cleanUp = func() {
+		currentStdin = prevStdin
+	}
+	return cleanUp
 }
 
 // GetPassword will prompt for a password one-time (to sign a tx)

client/keys/add.go

@@ -90,7 +90,7 @@ input
 output
 	- armor encrypted private key (saved to file)
 */
-func runAddCmd(cmd *cobra.Command, args []string) error {
+func runAddCmd(_ *cobra.Command, args []string) error {
 	var kb keys.Keybase
 	var err error
 	var encryptPassword string
@@ -265,7 +265,7 @@ func printCreate(info keys.Info, showMnemonic bool, mnemonic string) error {
 	output := viper.Get(cli.OutputFlag)
 
 	switch output {
-	case "text":
+	case OutputFormatText:
 		fmt.Fprintln(os.Stderr)
 		printKeyInfo(info, Bech32KeyOutput)
 
@@ -276,7 +276,7 @@ func printCreate(info keys.Info, showMnemonic bool, mnemonic string) error {
 			fmt.Fprintln(os.Stderr, "")
 			fmt.Fprintln(os.Stderr, mnemonic)
 		}
-	case "json":
+	case OutputFormatJSON:
 		out, err := Bech32KeyOutput(info)
 		if err != nil {
 			return err

client/keys/add_test.go

@@ -0,0 +1,104 @@
+package keys
+
+import (
+	"bufio"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+
+	"github.com/spf13/viper"
+	"github.com/tendermint/tendermint/libs/cli"
+
+	"github.com/cosmos/cosmos-sdk/tests"
+
+	"github.com/cosmos/cosmos-sdk/client"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_runAddCmdBasic(t *testing.T) {
+	cmd := addKeyCommand()
+	assert.NotNil(t, cmd)
+
+	// Missing input (enter password)
+	err := runAddCmd(cmd, []string{"keyname"})
+	assert.EqualError(t, err, "EOF")
+
+	// Prepare a keybase
+	kbHome, kbCleanUp, err := tests.GetTempDir("Test_runDeleteCmd")
+	assert.NoError(t, err)
+	assert.NotNil(t, kbHome)
+	defer kbCleanUp()
+	viper.Set(cli.HomeFlag, kbHome)
+
+	/// Test Text
+	viper.Set(cli.OutputFlag, OutputFormatText)
+	// Now enter password
+	cleanUp1 := client.OverrideStdin(bufio.NewReader(strings.NewReader("test1234\ntest1234\n")))
+	defer cleanUp1()
+	err = runAddCmd(cmd, []string{"keyname1"})
+	assert.NoError(t, err)
+
+	/// Test Text - Replace? >> FAIL
+	viper.Set(cli.OutputFlag, OutputFormatText)
+	// Now enter password
+	cleanUp2 := client.OverrideStdin(bufio.NewReader(strings.NewReader("test1234\ntest1234\n")))
+	defer cleanUp2()
+	err = runAddCmd(cmd, []string{"keyname1"})
+	assert.Error(t, err)
+
+	/// Test Text - Replace? Answer >> PASS
+	viper.Set(cli.OutputFlag, OutputFormatText)
+	// Now enter password
+	cleanUp3 := client.OverrideStdin(bufio.NewReader(strings.NewReader("y\ntest1234\ntest1234\n")))
+	defer cleanUp3()
+	err = runAddCmd(cmd, []string{"keyname1"})
+	assert.NoError(t, err)
+
+	// Check JSON
+	viper.Set(cli.OutputFlag, OutputFormatJSON)
+	// Now enter password
+	cleanUp4 := client.OverrideStdin(bufio.NewReader(strings.NewReader("test1234\ntest1234\n")))
+	defer cleanUp4()
+	err = runAddCmd(cmd, []string{"keyname2"})
+	assert.NoError(t, err)
+}
+
+type MockResponseWriter struct {
+	dataHeaderStatus int
+	dataBody         []byte
+}
+
+func (MockResponseWriter) Header() http.Header {
+	panic("Unexpected call!")
+}
+
+func (w *MockResponseWriter) Write(data []byte) (int, error) {
+	w.dataBody = append(w.dataBody, data...)
+	return len(data), nil
+}
+
+func (w *MockResponseWriter) WriteHeader(statusCode int) {
+	w.dataHeaderStatus = statusCode
+}
+
+func TestCheckAndWriteErrorResponse(t *testing.T) {
+	mockRW := MockResponseWriter{}
+
+	mockRW.WriteHeader(100)
+	assert.Equal(t, 100, mockRW.dataHeaderStatus)
+
+	detected := CheckAndWriteErrorResponse(&mockRW, http.StatusBadRequest, errors.New("some ERROR"))
+	require.True(t, detected)
+	require.Equal(t, http.StatusBadRequest, mockRW.dataHeaderStatus)
+	require.Equal(t, "some ERROR", string(mockRW.dataBody[:]))
+
+	mockRW = MockResponseWriter{}
+	detected = CheckAndWriteErrorResponse(&mockRW, http.StatusBadRequest, nil)
+	require.False(t, detected)
+	require.Equal(t, 0, mockRW.dataHeaderStatus)
+	require.Equal(t, "", string(mockRW.dataBody[:]))
+}

client/keys/codec_test.go

@@ -0,0 +1,100 @@
+package keys
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+type testCases struct {
+	Keys    []KeyOutput
+	Answers []KeyOutput
+	JSON    [][]byte
+}
+
+func getTestCases() testCases {
+	return testCases{
+		[]KeyOutput{
+			{"A", "B", "C", "D", "E"},
+			{"A", "B", "C", "D", ""},
+			{"", "B", "C", "D", ""},
+			{"", "", "", "", ""},
+		},
+		make([]KeyOutput, 4),
+		[][]byte{
+			[]byte(`{"name":"A","type":"B","address":"C","pub_key":"D","mnemonic":"E"}`),
+			[]byte(`{"name":"A","type":"B","address":"C","pub_key":"D"}`),
+			[]byte(`{"name":"","type":"B","address":"C","pub_key":"D"}`),
+			[]byte(`{"name":"","type":"","address":"","pub_key":""}`),
+		},
+	}
+}
+
+func TestMarshalJSON(t *testing.T) {
+	type args struct {
+		o KeyOutput
+	}
+
+	data := getTestCases()
+
+	tests := []struct {
+		name    string
+		args    args
+		want    []byte
+		wantErr bool
+	}{
+		{"basic", args{data.Keys[0]}, []byte(data.JSON[0]), false},
+		{"mnemonic is optional", args{data.Keys[1]}, []byte(data.JSON[1]), false},
+
+		// REVIEW: Are the next results expected??
+		{"empty name", args{data.Keys[2]}, []byte(data.JSON[2]), false},
+		{"empty object", args{data.Keys[3]}, []byte(data.JSON[3]), false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := MarshalJSON(tt.args.o)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("MarshalJSON() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			fmt.Printf("%s\n", got)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("MarshalJSON() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestUnmarshalJSON(t *testing.T) {
+	type args struct {
+		bz  []byte
+		ptr interface{}
+	}
+
+	data := getTestCases()
+
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{"basic", args{data.JSON[0], &data.Answers[0]}, false},
+		{"mnemonic is optional", args{data.JSON[1], &data.Answers[1]}, false},
+
+		// REVIEW: Are the next results expected??
+		{"empty name", args{data.JSON[2], &data.Answers[2]}, false},
+		{"empty object", args{data.JSON[3], &data.Answers[3]}, false},
+	}
+	for idx, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := UnmarshalJSON(tt.args.bz, tt.args.ptr); (err != nil) != tt.wantErr {
+				t.Errorf("UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			// Confirm deserialized objects are the same
+			require.Equal(t, data.Keys[idx], data.Answers[idx])
+		})
+	}
+}

client/keys/delete.go

@@ -91,6 +91,17 @@ func runDeleteCmd(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+func confirmDeletion(buf *bufio.Reader) error {
+	answer, err := client.GetConfirmation("Key reference will be deleted. Continue?", buf)
+	if err != nil {
+		return err
+	}
+	if !answer {
+		return errors.New("aborted")
+	}
+	return nil
+}
+
 ////////////////////////
 // REST
 
@@ -110,42 +121,31 @@ func DeleteKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
 	err := decoder.Decode(&m)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
-		w.Write([]byte(err.Error()))
+		_, _ = w.Write([]byte(err.Error()))
 		return
 	}
 
 	kb, err = NewKeyBaseFromHomeFlag()
 	if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte(err.Error()))
+		_, _ = w.Write([]byte(err.Error()))
 		return
 	}
 
 	err = kb.Delete(name, m.Password, false)
 	if keyerror.IsErrKeyNotFound(err) {
 		w.WriteHeader(http.StatusNotFound)
-		w.Write([]byte(err.Error()))
+		_, _ = w.Write([]byte(err.Error()))
 		return
 	} else if keyerror.IsErrWrongPassword(err) {
 		w.WriteHeader(http.StatusUnauthorized)
-		w.Write([]byte(err.Error()))
+		_, _ = w.Write([]byte(err.Error()))
 		return
 	} else if err != nil {
 		w.WriteHeader(http.StatusInternalServerError)
-		w.Write([]byte(err.Error()))
+		_, _ = w.Write([]byte(err.Error()))
 		return
 	}
 
 	w.WriteHeader(http.StatusOK)
 }
-
-func confirmDeletion(buf *bufio.Reader) error {
-	answer, err := client.GetConfirmation("Key reference will be deleted. Continue?", buf)
-	if err != nil {
-		return err
-	}
-	if !answer {
-		return errors.New("aborted")
-	}
-	return nil
-}