From 2827fd50347ced99833ab6425f458d7c32f40898 Mon Sep 17 00:00:00 2001 From: Jiawei Zhao Date: Sat, 13 Jan 2024 22:15:00 +0800 Subject: [PATCH 1/2] test: add an unit test to cover AsResult method Signed-off-by: Jiawei Zhao --- internal/controller/sharded_test.go | 40 +++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 internal/controller/sharded_test.go diff --git a/internal/controller/sharded_test.go b/internal/controller/sharded_test.go new file mode 100644 index 0000000..15f2ce6 --- /dev/null +++ b/internal/controller/sharded_test.go @@ -0,0 +1,40 @@ +package controller + +import ( + "errors" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + update_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" +) + +func TestAsResult(t *testing.T) { + // Test case 1: No errors + errSet := StCtrlErrSet{} + result, err := errSet.AsResult() + assert.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + + // Test case 2: Update conflict error + statusErr := update_errors.StatusError{ErrStatus: metav1.Status{ + Reason: metav1.StatusReasonConflict, + Code: http.StatusConflict, + }} + errSet = StCtrlErrSet{Update: error(&statusErr)} + result, err = errSet.AsResult() + assert.NoError(t, err) + assert.Equal(t, ctrl.Result{Requeue: true}, result) + + // Test case 3: Other Errors + errSet = StCtrlErrSet{ + Rec: errors.New("reconciliation error"), + Sync: errors.New("sync error"), + Update: errors.New("sync error"), + } + result, err = errSet.AsResult() + assert.Error(t, err) + assert.Equal(t, ctrl.Result{Requeue: true}, result) +} From 74224d7f1eda57e6beb966dc7fb4c94d87decadd Mon Sep 17 00:00:00 2001 From: Jiawei Zhao Date: Sat, 13 Jan 2024 22:16:03 +0800 Subject: [PATCH 2/2] refactor: remove MergeErrorsWithTag method Signed-off-by: Jiawei Zhao --- internal/controller/sharded.go | 5 ++--- internal/util/mutierror.go | 14 -------------- internal/util/mutierror_test.go | 24 ++---------------------- 3 files changed, 4 insertions(+), 39 deletions(-) diff --git a/internal/controller/sharded.go b/internal/controller/sharded.go index 0b7bbff..756c8cc 100644 --- a/internal/controller/sharded.go +++ b/internal/controller/sharded.go @@ -48,14 +48,13 @@ func (r *StCtrlErrSet) AsResult() (ctrl.Result, error) { if r.Update != nil { errMap["update-status"] = r.Update } - mergedErr := util.MergeErrorsWithTag(errMap) - if mergedErr == nil { + if len(errMap) == 0 { if updateConflict { return ctrl.Result{Requeue: true}, nil } else { return ctrl.Result{}, nil } } else { - return ctrl.Result{Requeue: true}, mergedErr + return ctrl.Result{Requeue: true}, &util.MultiTaggedError{Errors: errMap} } } diff --git a/internal/util/mutierror.go b/internal/util/mutierror.go index 1f60f51..e337e07 100644 --- a/internal/util/mutierror.go +++ b/internal/util/mutierror.go @@ -99,17 +99,3 @@ func (e *MultiTaggedError) Error() string { } return strings.Join(errStrs, "; ") } - -// MergeErrorsWithTag merges multiple errors into one with tags. -func MergeErrorsWithTag(errors map[string]error) *MultiTaggedError { - errMap := make(map[string]error) - for tag, err := range errors { - if err != nil { - errMap[tag] = err - } - } - if len(errMap) == 0 { - return nil - } - return &MultiTaggedError{Errors: errMap} -} diff --git a/internal/util/mutierror_test.go b/internal/util/mutierror_test.go index 38c909b..c7d0483 100644 --- a/internal/util/mutierror_test.go +++ b/internal/util/mutierror_test.go @@ -20,9 +20,9 @@ package util import ( "fmt" - "github.com/stretchr/testify/assert" - "k8s.io/utils/strings/slices" "testing" + + "github.com/stretchr/testify/assert" ) func TestMergeErrors(t *testing.T) { @@ -38,23 +38,3 @@ func TestMergeErrors(t *testing.T) { err4 := MergeErrors(nil, nil) assert.Nil(t, err4) } - -func TestMergeErrorsWithTags(t *testing.T) { - err1 := MergeErrorsWithTag(map[string]error{ - "tag1": fmt.Errorf("foo"), - "tag2": fmt.Errorf("bar"), - }) - assert.True(t, slices.Contains([]string{"[tag1] foo; [tag2] bar", "[tag2] bar; [tag1] foo"}, err1.Error())) - - err2 := MergeErrorsWithTag(map[string]error{ - "tag1": fmt.Errorf("foo"), - "tag2": nil, - }) - assert.Equal(t, "[tag1] foo", err2.Error()) - - err3 := MergeErrorsWithTag(map[string]error{ - "tag1": nil, - "tag2": nil, - }) - assert.Nil(t, err3) -}