Skip to content

Commit

Permalink
fix: x/gov/client/cli: update & tighten Prompt.Parse tests (#13350)
Browse files Browse the repository at this point in the history
Adds more rigorous checks to ensure that the output
is as expected with range failures. While here also
added control tests to ensure that we can parse integers
within range of int.

Updates #13346
  • Loading branch information
odeke-em committed Sep 21, 2022
1 parent 9c2aef3 commit a2118af
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 11 deletions.
6 changes: 3 additions & 3 deletions x/gov/client/cli/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"encoding/json"
"fmt"
"os"
"reflect"
"reflect" // #nosec
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -94,7 +94,7 @@ func Prompt[T any](data T, namePrefix string) (T, error) {
case reflect.String:
v.Field(i).SetString(result)
case reflect.Int:
resultInt, err := strconv.Atoi(result)
resultInt, err := strconv.ParseInt(result, 10, 0)
if err != nil {
return data, fmt.Errorf("invalid value for int: %w", err)
}
Expand All @@ -104,7 +104,7 @@ func Prompt[T any](data T, namePrefix string) (T, error) {
// [minInt64, maxInt64]
// of which on 64-bit machines, which are most common,
// int==int64
v.Field(i).SetInt(int64(resultInt))
v.Field(i).SetInt(resultInt)
default:
// skip other types
// possibly in the future we can add more types (like slices)
Expand Down
61 changes: 53 additions & 8 deletions x/gov/client/cli/prompt_test.go
Original file line number Diff line number Diff line change
@@ -1,26 +1,33 @@
//go:build !race
// +build !race

// Disabled -race because the package github.com/manifoldco/promptui@v0.9.0
// has a data race and this code exposes it, but fixing it would require
// holding up the associated change to this.

package cli_test

import (
"sync"
"fmt"
"math"
"os"
"testing"

"github.com/chzyer/readline"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
)

type st struct {
ToOverflow int
I int
}

// On the tests running in Github actions, somehow there are races.
var globalMu sync.Mutex

// Tests that we successfully report overflows in parsing ints
// See https://github.com/cosmos/cosmos-sdk/issues/13346
func TestPromptIntegerOverflow(t *testing.T) {
// Intentionally sending a value out of the range of
// Intentionally sending values out of the range of int.
intOverflowers := []string{
"-9223372036854775809",
"9223372036854775808",
Expand All @@ -33,11 +40,49 @@ func TestPromptIntegerOverflow(t *testing.T) {
for _, intOverflower := range intOverflowers {
overflowStr := intOverflower
t.Run(overflowStr, func(t *testing.T) {
readline.Stdout.Write([]byte(overflowStr + "\n"))
origStdin := readline.Stdin
defer func() {
readline.Stdin = origStdin
}()

fin, fw := readline.NewFillableStdin(os.Stdin)
readline.Stdin = fin
fw.Write([]byte(overflowStr + "\n"))

v, err := cli.Prompt(st{}, "")
assert.NotNil(t, err, "expected a report of an overflow")
assert.Equal(t, st{}, v, "expected a value of zero")
require.NotNil(t, err, "expected a report of an overflow")
require.Contains(t, err.Error(), "range")
})
}
}

func TestPromptParseInteger(t *testing.T) {
// Intentionally sending a value out of the range of
values := []struct {
in string
want int
}{
{fmt.Sprintf("%d", math.MinInt), math.MinInt},
{"19991", 19991},
{"991000000199", 991000000199},
}

for _, tc := range values {
tc := tc
t.Run(tc.in, func(t *testing.T) {
origStdin := readline.Stdin
defer func() {
readline.Stdin = origStdin
}()

fin, fw := readline.NewFillableStdin(os.Stdin)
readline.Stdin = fin
fw.Write([]byte(tc.in + "\n"))

v, err := cli.Prompt(st{}, "")
assert.Nil(t, err, "expected a nil error")
assert.Equal(t, tc.want, v.I, "expected %d = %d", tc.want, v.I)
})
}
}

0 comments on commit a2118af

Please sign in to comment.