From 2f65e8bfe584c30ba2f551084f7bb1ededf50521 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 19 Oct 2023 17:37:16 +0530 Subject: [PATCH 1/5] add fix and tests --- api/v1/validatorstate.go | 9 +++++++-- api/v1/validatorstate_test.go | 20 +++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/api/v1/validatorstate.go b/api/v1/validatorstate.go index 404b4a9d..f9db17f9 100644 --- a/api/v1/validatorstate.go +++ b/api/v1/validatorstate.go @@ -15,6 +15,7 @@ package v1 import ( "fmt" + "github.com/pkg/errors" "strings" "github.com/attestantio/go-eth2-client/spec/phase0" @@ -94,8 +95,12 @@ func (v *ValidatorState) UnmarshalJSON(input []byte) error { return err } -func (v ValidatorState) String() string { - return validatorStateStrings[v] +func (v ValidatorState) String() (string, error) { + if int(v) < 0 || int(v) > len(validatorStateStrings) { + return "", errors.New("invalid validator state") + } + + return validatorStateStrings[v], nil } // IsPending returns true if the validator is pending. diff --git a/api/v1/validatorstate_test.go b/api/v1/validatorstate_test.go index 00e71efb..7bc6b385 100644 --- a/api/v1/validatorstate_test.go +++ b/api/v1/validatorstate_test.go @@ -131,7 +131,10 @@ func TestValidatorStateJSON(t *testing.T) { assert.Equal(t, test.isExited, res.IsExited()) assert.Equal(t, test.hasExited, res.HasExited()) assert.Equal(t, test.hasBalance, res.HasBalance()) - assert.Equal(t, strings.Trim(string(rt), `"`), res.String()) + + valState, err := res.String() + require.NoError(t, err) + assert.Equal(t, strings.Trim(string(rt), `"`), valState) } }) } @@ -261,3 +264,18 @@ func TestValidatorToState(t *testing.T) { }) } } + +func TestString(t *testing.T) { + t.Run("valid state", func(t *testing.T) { + state := api.ValidatorStateActiveOngoing + resp, err := state.String() + require.NoError(t, err) + require.Equal(t, resp, "active_ongoing") + }) + + t.Run("invalid state", func(t *testing.T) { + state := api.ValidatorState(25) + _, err := state.String() + require.Error(t, err) + }) +} From 02caeb3b0b698db4d8b44aa0f46b726a84178247 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 20 Oct 2023 07:45:08 +0530 Subject: [PATCH 2/5] fix lint --- api/v1/validatorstate.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/v1/validatorstate.go b/api/v1/validatorstate.go index f9db17f9..3d53cffa 100644 --- a/api/v1/validatorstate.go +++ b/api/v1/validatorstate.go @@ -15,7 +15,6 @@ package v1 import ( "fmt" - "github.com/pkg/errors" "strings" "github.com/attestantio/go-eth2-client/spec/phase0" @@ -97,7 +96,7 @@ func (v *ValidatorState) UnmarshalJSON(input []byte) error { func (v ValidatorState) String() (string, error) { if int(v) < 0 || int(v) > len(validatorStateStrings) { - return "", errors.New("invalid validator state") + return "", fmt.Errorf("unrecognised validator state %d", v) } return validatorStateStrings[v], nil From 9a0cfd097994e6d511b232dc47c5bd036ddf9730 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 20 Oct 2023 07:47:08 +0530 Subject: [PATCH 3/5] add check for equality --- api/v1/validatorstate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/validatorstate.go b/api/v1/validatorstate.go index 3d53cffa..ca4b4734 100644 --- a/api/v1/validatorstate.go +++ b/api/v1/validatorstate.go @@ -95,7 +95,7 @@ func (v *ValidatorState) UnmarshalJSON(input []byte) error { } func (v ValidatorState) String() (string, error) { - if int(v) < 0 || int(v) > len(validatorStateStrings) { + if int(v) < 0 || int(v) >= len(validatorStateStrings) { return "", fmt.Errorf("unrecognised validator state %d", v) } From e3e36c07912675ccc235614b2c4bf95059ebec1c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 20 Oct 2023 08:03:10 +0530 Subject: [PATCH 4/5] convert to table testst --- api/v1/validatorstate_test.go | 50 +++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/api/v1/validatorstate_test.go b/api/v1/validatorstate_test.go index 7bc6b385..f8c22f49 100644 --- a/api/v1/validatorstate_test.go +++ b/api/v1/validatorstate_test.go @@ -266,16 +266,44 @@ func TestValidatorToState(t *testing.T) { } func TestString(t *testing.T) { - t.Run("valid state", func(t *testing.T) { - state := api.ValidatorStateActiveOngoing - resp, err := state.String() - require.NoError(t, err) - require.Equal(t, resp, "active_ongoing") - }) + tests := []struct { + name string + state api.ValidatorState + expected string + valid bool + }{ + { + name: "valid state", + state: api.ValidatorStateActiveOngoing, + expected: "active_ongoing", + valid: true, + }, + { + name: "negative index", + state: -1, + valid: false, + }, + { + name: "edge bound index", + state: 10, + valid: false, + }, + { + name: "high out of bound index", + state: 250, + valid: false, + }, + } - t.Run("invalid state", func(t *testing.T) { - state := api.ValidatorState(25) - _, err := state.String() - require.Error(t, err) - }) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resp, err := test.state.String() + if test.valid { + require.NoError(t, err) + } else { + require.Error(t, err) + } + require.Equal(t, test.expected, resp) + }) + } } From 681ff2b81b5be7aa3b718d252515e49eb1c3180a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Sat, 21 Oct 2023 16:12:46 +0530 Subject: [PATCH 5/5] fix as per review comments --- api/v1/validatorstate.go | 8 ++++---- api/v1/validatorstate_test.go | 32 +++++++++++--------------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/api/v1/validatorstate.go b/api/v1/validatorstate.go index ca4b4734..9cd92f29 100644 --- a/api/v1/validatorstate.go +++ b/api/v1/validatorstate.go @@ -94,12 +94,12 @@ func (v *ValidatorState) UnmarshalJSON(input []byte) error { return err } -func (v ValidatorState) String() (string, error) { - if int(v) < 0 || int(v) >= len(validatorStateStrings) { - return "", fmt.Errorf("unrecognised validator state %d", v) +func (v ValidatorState) String() string { + if v < 0 || int(v) >= len(validatorStateStrings) { + return validatorStateStrings[0] // unknown } - return validatorStateStrings[v], nil + return validatorStateStrings[v] } // IsPending returns true if the validator is pending. diff --git a/api/v1/validatorstate_test.go b/api/v1/validatorstate_test.go index f8c22f49..e4775506 100644 --- a/api/v1/validatorstate_test.go +++ b/api/v1/validatorstate_test.go @@ -131,10 +131,7 @@ func TestValidatorStateJSON(t *testing.T) { assert.Equal(t, test.isExited, res.IsExited()) assert.Equal(t, test.hasExited, res.HasExited()) assert.Equal(t, test.hasBalance, res.HasBalance()) - - valState, err := res.String() - require.NoError(t, err) - assert.Equal(t, strings.Trim(string(rt), `"`), valState) + assert.Equal(t, strings.Trim(string(rt), `"`), res.String()) } }) } @@ -270,39 +267,32 @@ func TestString(t *testing.T) { name string state api.ValidatorState expected string - valid bool }{ { name: "valid state", state: api.ValidatorStateActiveOngoing, expected: "active_ongoing", - valid: true, }, { - name: "negative index", - state: -1, - valid: false, + name: "negative index", + state: -1, + expected: "unknown", }, { - name: "edge bound index", - state: 10, - valid: false, + name: "edge bound index", + state: 10, + expected: "unknown", }, { - name: "high out of bound index", - state: 250, - valid: false, + name: "high out of bound index", + state: 250, + expected: "unknown", }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - resp, err := test.state.String() - if test.valid { - require.NoError(t, err) - } else { - require.Error(t, err) - } + resp := test.state.String() require.Equal(t, test.expected, resp) }) }