From 68d10835f75fce600734e19848424b75f31a2f60 Mon Sep 17 00:00:00 2001 From: Mihai Alexandrescu Date: Tue, 28 Feb 2023 16:26:53 +0200 Subject: [PATCH 1/2] temporary update to pkg/webhooks/errors_test.go; cleanup needed --- pkg/webhooks/errors_test.go | 101 +++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 37 deletions(-) diff --git a/pkg/webhooks/errors_test.go b/pkg/webhooks/errors_test.go index cdf327a7f..afcfab7fa 100644 --- a/pkg/webhooks/errors_test.go +++ b/pkg/webhooks/errors_test.go @@ -30,21 +30,51 @@ import ( ) func TestIsAdmissionConnectionError(t *testing.T) { - err := apierrors.NewInternalError(errors.Wrap(errors.New("..."), cantConnectErrorMsg)) - - if !IsAdmissionCantConnect(err) { - t.Error("Expected is connection error to be true, got false") + testCases := []struct { + testName string + err error + want bool + }{ + { + testName: "correct error type and message", + err: apierrors.NewInternalError(errors.Wrap(errors.New("..."), cantConnectErrorMsg)), + want: true, + }, + { + testName: "incorrect both error type and message", + err: apierrors.NewServiceUnavailable("some other reason"), + want: false, + }, + { + testName: "incorrect error type with correct message", + err: apierrors.NewBadRequest(cantConnectErrorMsg), + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + got := IsAdmissionCantConnect(tc.err) + require.Equal(t, tc.want, got) + }) } + err := apierrors.NewInternalError(errors.Wrap(errors.New("..."), cantConnectErrorMsg)) + require.True(t, IsAdmissionCantConnect(err)) + //if !IsAdmissionCantConnect(err) { + // t.Error("Expected is connection error to be true, got false") + //} + err = apierrors.NewServiceUnavailable("some other reason") - if IsAdmissionCantConnect(err) { - t.Error("Expected is connection error to be false, got true") - } + require.False(t, IsAdmissionCantConnect(err)) + //if IsAdmissionCantConnect(err) { + // t.Error("Expected is connection error to be false, got true") + //} err = apierrors.NewBadRequest(cantConnectErrorMsg) - if IsAdmissionCantConnect(err) { - t.Error("Expected is connection error to be false, got true") - } + require.False(t, IsAdmissionCantConnect(err)) + //if IsAdmissionCantConnect(err) { + // t.Error("Expected is connection error to be false, got true") + //} } func TestIsInvalidReplicationFactor(t *testing.T) { @@ -55,20 +85,22 @@ func TestIsInvalidReplicationFactor(t *testing.T) { err := apierrors.NewInvalid( kafkaTopic.GetObjectKind().GroupVersionKind().GroupKind(), kafkaTopic.Name, fieldErrs) - - if !IsAdmissionInvalidReplicationFactor(err) { - t.Error("Expected is invalid replication error to be true, got false") - } + require.True(t, IsAdmissionInvalidReplicationFactor(err)) + //if !IsAdmissionInvalidReplicationFactor(err) { + // t.Error("Expected is invalid replication error to be true, got false") + //} err = apierrors.NewServiceUnavailable("some other reason") - if IsAdmissionInvalidReplicationFactor(err) { - t.Error("Expected is invalid replication error to be false, got true") - } + require.False(t, IsAdmissionInvalidReplicationFactor(err)) + //if IsAdmissionInvalidReplicationFactor(err) { + // t.Error("Expected is invalid replication error to be false, got true") + //} err = apierrors.NewServiceUnavailable(invalidReplicationFactorErrMsg) - if IsAdmissionInvalidReplicationFactor(err) { - t.Error("Expected is invalid replication error to be false, got true") - } + require.False(t, IsAdmissionInvalidReplicationFactor(err)) + //if IsAdmissionInvalidReplicationFactor(err) { + // t.Error("Expected is invalid replication error to be false, got true") + //} } func TestIsAdmissionCantConnectAPIServer(t *testing.T) { @@ -91,9 +123,8 @@ func TestIsAdmissionCantConnectAPIServer(t *testing.T) { for _, tc := range testCases { t.Run(tc.testName, func(t *testing.T) { - if got := IsAdmissionCantConnectAPIServer(tc.err); got != tc.want { - t.Errorf("Check connection to API Server error message. Expected: %t ; Got: %t", tc.want, got) - } + got := IsAdmissionCantConnectAPIServer(tc.err) + require.Equal(t, tc.want, got) }) } } @@ -101,27 +132,25 @@ func TestIsAdmissionCantConnectAPIServer(t *testing.T) { func TestIsAdmissionOutOfRangeReplicationFactor(t *testing.T) { kafkaTopic := banzaicloudv1alpha1.KafkaTopic{ObjectMeta: metav1.ObjectMeta{Name: "test-KafkaTopic"}} var fieldErrs field.ErrorList - fieldErrs = append(fieldErrs, field.Invalid(field.NewPath("spec").Child("replicationFactor"), "-2", outOfRangeReplicationFactorErrMsg)) + fieldErrs = append(fieldErrs, field.Invalid(field.NewPath("spec").Child("replicationFactor"), int32(-2), outOfRangeReplicationFactorErrMsg)) err := apierrors.NewInvalid( kafkaTopic.GetObjectKind().GroupVersionKind().GroupKind(), kafkaTopic.Name, fieldErrs) - if ok := IsAdmissionOutOfRangeReplicationFactor(err); !ok { - t.Errorf("Check Out of Range ReplicationFactor error message. Expected: %t ; Got: %t", true, ok) - } + got := IsAdmissionOutOfRangeReplicationFactor(err) + require.True(t, got) } func TestIsAdmissionOutOfRangePartitions(t *testing.T) { kafkaTopic := banzaicloudv1alpha1.KafkaTopic{ObjectMeta: metav1.ObjectMeta{Name: "test-KafkaTopic"}} var fieldErrs field.ErrorList - fieldErrs = append(fieldErrs, field.Invalid(field.NewPath("spec").Child("partitions"), "-2", outOfRangePartitionsErrMsg)) + fieldErrs = append(fieldErrs, field.Invalid(field.NewPath("spec").Child("partitions"), int32(-2), outOfRangePartitionsErrMsg)) err := apierrors.NewInvalid( kafkaTopic.GetObjectKind().GroupVersionKind().GroupKind(), kafkaTopic.Name, fieldErrs) - if ok := IsAdmissionOutOfRangePartitions(err); !ok { - t.Errorf("Check Out of Range Partitions error message. Expected: %t ; Got: %t", true, ok) - } + got := IsAdmissionOutOfRangePartitions(err) + require.True(t, got) } func TestIsAdmissionInvalidRemovingStorage(t *testing.T) { @@ -160,9 +189,8 @@ func TestIsAdmissionInvalidRemovingStorage(t *testing.T) { kafkaCluster.GetObjectKind().GroupVersionKind().GroupKind(), kafkaCluster.Name, tc.fieldErrs) - if got := IsAdmissionInvalidRemovingStorage(err); got != tc.want { - t.Errorf("Check Storage Removal Error message. Expected: %t ; Got: %t", tc.want, got) - } + got := IsAdmissionInvalidRemovingStorage(err) + require.Equal(t, tc.want, got) }) } } @@ -221,9 +249,8 @@ func TestIsAdmissionErrorDuringValidation(t *testing.T) { for _, tc := range testCases { t.Run(tc.testName, func(t *testing.T) { - if got := IsAdmissionErrorDuringValidation(tc.err); got != tc.want { - t.Errorf("Check overall Error During Validation error message. Expected: %t ; Got: %t", tc.want, got) - } + got := IsAdmissionErrorDuringValidation(tc.err) + require.Equal(t, tc.want, got) }) } } From e54e90cee3d36b4ca49d5b84200ea0f93ef6d18f Mon Sep 17 00:00:00 2001 From: Mihai Alexandrescu Date: Mon, 6 Mar 2023 14:31:25 +0200 Subject: [PATCH 2/2] clean up older commented code and fix description --- pkg/webhooks/errors_test.go | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/pkg/webhooks/errors_test.go b/pkg/webhooks/errors_test.go index afcfab7fa..8004144d6 100644 --- a/pkg/webhooks/errors_test.go +++ b/pkg/webhooks/errors_test.go @@ -46,7 +46,7 @@ func TestIsAdmissionConnectionError(t *testing.T) { want: false, }, { - testName: "incorrect error type with correct message", + testName: "incorrect error type with correct message component", err: apierrors.NewBadRequest(cantConnectErrorMsg), want: false, }, @@ -57,24 +57,6 @@ func TestIsAdmissionConnectionError(t *testing.T) { require.Equal(t, tc.want, got) }) } - - err := apierrors.NewInternalError(errors.Wrap(errors.New("..."), cantConnectErrorMsg)) - require.True(t, IsAdmissionCantConnect(err)) - //if !IsAdmissionCantConnect(err) { - // t.Error("Expected is connection error to be true, got false") - //} - - err = apierrors.NewServiceUnavailable("some other reason") - require.False(t, IsAdmissionCantConnect(err)) - //if IsAdmissionCantConnect(err) { - // t.Error("Expected is connection error to be false, got true") - //} - - err = apierrors.NewBadRequest(cantConnectErrorMsg) - require.False(t, IsAdmissionCantConnect(err)) - //if IsAdmissionCantConnect(err) { - // t.Error("Expected is connection error to be false, got true") - //} } func TestIsInvalidReplicationFactor(t *testing.T) { @@ -86,21 +68,12 @@ func TestIsInvalidReplicationFactor(t *testing.T) { kafkaTopic.GetObjectKind().GroupVersionKind().GroupKind(), kafkaTopic.Name, fieldErrs) require.True(t, IsAdmissionInvalidReplicationFactor(err)) - //if !IsAdmissionInvalidReplicationFactor(err) { - // t.Error("Expected is invalid replication error to be true, got false") - //} err = apierrors.NewServiceUnavailable("some other reason") require.False(t, IsAdmissionInvalidReplicationFactor(err)) - //if IsAdmissionInvalidReplicationFactor(err) { - // t.Error("Expected is invalid replication error to be false, got true") - //} err = apierrors.NewServiceUnavailable(invalidReplicationFactorErrMsg) require.False(t, IsAdmissionInvalidReplicationFactor(err)) - //if IsAdmissionInvalidReplicationFactor(err) { - // t.Error("Expected is invalid replication error to be false, got true") - //} } func TestIsAdmissionCantConnectAPIServer(t *testing.T) {