From a2118afc2174491fade1e26bded7227886f4b756 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Tue, 20 Sep 2022 22:59:11 -0700 Subject: [PATCH] fix: x/gov/client/cli: update & tighten Prompt.Parse tests (#13350) 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 --- x/gov/client/cli/prompt.go | 6 ++-- x/gov/client/cli/prompt_test.go | 61 ++++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/x/gov/client/cli/prompt.go b/x/gov/client/cli/prompt.go index faf7d352e1..337808bb3e 100644 --- a/x/gov/client/cli/prompt.go +++ b/x/gov/client/cli/prompt.go @@ -4,7 +4,7 @@ import ( "encoding/json" "fmt" "os" - "reflect" + "reflect" // #nosec "sort" "strconv" "strings" @@ -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) } @@ -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) diff --git a/x/gov/client/cli/prompt_test.go b/x/gov/client/cli/prompt_test.go index 2629cc6530..e5b01d7da3 100644 --- a/x/gov/client/cli/prompt_test.go +++ b/x/gov/client/cli/prompt_test.go @@ -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", @@ -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) }) } }