From 7cc5907be38e15aa39bb40d6e716e5e4818b03c6 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Mon, 11 Nov 2024 02:58:35 +0100 Subject: [PATCH] chore: use testify instead of testing.Fatal or testing.Error in applicationset (#20726) Signed-off-by: Matthieu MOREL --- .../applicationset_controller_test.go | 18 +++++++-------- .../services/internal/http/client_test.go | 22 +++++-------------- .../services/plugin/plugin_service_test.go | 13 ++++------- .../services/pull_request/github_test.go | 9 ++++---- .../services/pull_request/gitlab_test.go | 9 +++----- applicationset/webhook/webhook_test.go | 5 ++--- 6 files changed, 27 insertions(+), 49 deletions(-) diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go index b33465a679caa..f4e63a9e6866b 100644 --- a/applicationset/controllers/applicationset_controller_test.go +++ b/applicationset/controllers/applicationset_controller_test.go @@ -1178,18 +1178,18 @@ func TestRemoveFinalizerOnInvalidDestination_FinalizerTypes(t *testing.T) { // argoDB := db.NewDB("namespace", settingsMgr, r.KubeClientset) // clusterList, err := argoDB.ListClusters(context.Background()) clusterList, err := utils.ListClusters(context.Background(), kubeclientset, "namespace") - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": ""}) appInputParam := app.DeepCopy() err = r.removeFinalizerOnInvalidDestination(context.Background(), appSet, appInputParam, clusterList, appLog) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) retrievedApp := v1alpha1.Application{} err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) // App on the cluster should have the expected finalizers assert.ElementsMatch(t, c.expectedFinalizers, retrievedApp.Finalizers) @@ -1334,18 +1334,18 @@ func TestRemoveFinalizerOnInvalidDestination_DestinationTypes(t *testing.T) { // argoDB := db.NewDB("argocd", settingsMgr, r.KubeClientset) // clusterList, err := argoDB.ListClusters(context.Background()) clusterList, err := utils.ListClusters(context.Background(), kubeclientset, "namespace") - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": ""}) appInputParam := app.DeepCopy() err = r.removeFinalizerOnInvalidDestination(context.Background(), appSet, appInputParam, clusterList, appLog) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) retrievedApp := v1alpha1.Application{} err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) finalizerRemoved := len(retrievedApp.Finalizers) == 0 @@ -1402,7 +1402,7 @@ func TestRemoveOwnerReferencesOnDeleteAppSet(t *testing.T) { } err := controllerutil.SetControllerReference(&appSet, &app, scheme) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) initObjs := []crtclient.Object{&app, &appSet} @@ -1418,11 +1418,11 @@ func TestRemoveOwnerReferencesOnDeleteAppSet(t *testing.T) { } err = r.removeOwnerReferencesOnDeleteAppSet(context.Background(), appSet) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) retrievedApp := v1alpha1.Application{} err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp) - require.NoError(t, err, "Unexpected error") + require.NoError(t, err) ownerReferencesRemoved := len(retrievedApp.OwnerReferences) == 0 assert.True(t, ownerReferencesRemoved) diff --git a/applicationset/services/internal/http/client_test.go b/applicationset/services/internal/http/client_test.go index 83d304835c70a..29c183b5b2481 100644 --- a/applicationset/services/internal/http/client_test.go +++ b/applicationset/services/internal/http/client_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestClient(t *testing.T) { @@ -24,9 +25,7 @@ func TestClient(t *testing.T) { var clientOptionFns []ClientOptionFunc _, err := NewClient(server.URL, clientOptionFns...) - if err != nil { - t.Fatalf("Failed to create client: %v", err) - } + require.NoError(t, err, "Failed to create client") } func TestClientDo(t *testing.T) { @@ -118,14 +117,10 @@ func TestClientDo(t *testing.T) { defer cc.fakeServer.Close() client, err := NewClient(cc.fakeServer.URL, cc.clientOptionFns...) - if err != nil { - t.Fatalf("NewClient returned unexpected error: %v", err) - } + require.NoError(t, err, "NewClient returned unexpected error") req, err := client.NewRequest("POST", "", cc.params, nil) - if err != nil { - t.Fatalf("NewRequest returned unexpected error: %v", err) - } + require.NoError(t, err, "NewRequest returned unexpected error") var data []map[string]interface{} @@ -149,12 +144,5 @@ func TestCheckResponse(t *testing.T) { } err := CheckResponse(resp) - if err == nil { - t.Error("Expected an error, got nil") - } - - expected := "API error with status code 400: invalid_request" - if err.Error() != expected { - t.Errorf("Expected error '%s', got '%s'", expected, err.Error()) - } + require.EqualError(t, err, "API error with status code 400: invalid_request") } diff --git a/applicationset/services/plugin/plugin_service_test.go b/applicationset/services/plugin/plugin_service_test.go index 75e7f2c4a095f..c6e795c40371b 100644 --- a/applicationset/services/plugin/plugin_service_test.go +++ b/applicationset/services/plugin/plugin_service_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestPlugin(t *testing.T) { @@ -31,19 +32,13 @@ func TestPlugin(t *testing.T) { defer ts.Close() client, err := NewPluginService(context.Background(), "plugin-test", ts.URL, token, 0) - if err != nil { - t.Errorf("unexpected error: %v", err) - } + require.NoError(t, err) data, err := client.List(context.Background(), nil) - if err != nil { - t.Errorf("unexpected error: %v", err) - } + require.NoError(t, err) var expectedData ServiceResponse err = json.Unmarshal([]byte(expectedJSON), &expectedData) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) assert.Equal(t, &expectedData, data) } diff --git a/applicationset/services/pull_request/github_test.go b/applicationset/services/pull_request/github_test.go index 30b908f9fb1b6..d68bcd8f124de 100644 --- a/applicationset/services/pull_request/github_test.go +++ b/applicationset/services/pull_request/github_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/google/go-github/v63/github" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func toPtr(s string) *string { @@ -52,9 +52,8 @@ func TestContainLabels(t *testing.T) { for _, c := range cases { t.Run(c.Name, func(t *testing.T) { - if got := containLabels(c.Labels, c.PullLabels); got != c.Expect { - t.Errorf("expect: %v, got: %v", c.Expect, got) - } + got := containLabels(c.Labels, c.PullLabels) + require.Equal(t, got, c.Expect) }) } } @@ -83,7 +82,7 @@ func TestGetGitHubPRLabelNames(t *testing.T) { for _, test := range Tests { t.Run(test.Name, func(t *testing.T) { labels := getGithubPRLabelNames(test.PullLabels) - assert.Equal(t, test.ExpectedResult, labels) + require.Equal(t, test.ExpectedResult, labels) }) } } diff --git a/applicationset/services/pull_request/gitlab_test.go b/applicationset/services/pull_request/gitlab_test.go index 9abb8d73ea491..18467fe08815d 100644 --- a/applicationset/services/pull_request/gitlab_test.go +++ b/applicationset/services/pull_request/gitlab_test.go @@ -17,13 +17,10 @@ import ( func writeMRListResponse(t *testing.T, w io.Writer) { t.Helper() f, err := os.Open("fixtures/gitlab_mr_list_response.json") - if err != nil { - t.Fatalf("error opening fixture file: %v", err) - } + require.NoErrorf(t, err, "error opening fixture file: %v", err) - if _, err = io.Copy(w, f); err != nil { - t.Fatalf("error writing response: %v", err) - } + _, err = io.Copy(w, f) + require.NoErrorf(t, err, "error writing response: %v", err) } func TestGitLabServiceCustomBaseURL(t *testing.T) { diff --git a/applicationset/webhook/webhook_test.go b/applicationset/webhook/webhook_test.go index 046bbf35f09ab..baf3035b97b5c 100644 --- a/applicationset/webhook/webhook_test.go +++ b/applicationset/webhook/webhook_test.go @@ -242,9 +242,8 @@ func TestWebhookHandler(t *testing.T) { for i := range list.Items { gotAppSet := &list.Items[i] if _, isEffected := effectedAppSetsAsExpected[gotAppSet.Name]; isEffected { - if expected, got := test.expectedRefresh, gotAppSet.RefreshRequired(); expected != got { - t.Errorf("unexpected RefreshRequired() for appset '%s' expect: %v got: %v", gotAppSet.Name, expected, got) - } + expected, got := test.expectedRefresh, gotAppSet.RefreshRequired() + require.Equalf(t, expected, got, "unexpected RefreshRequired() for appset '%s' expect: %v got: %v", gotAppSet.Name, expected, got) effectedAppSetsAsExpected[gotAppSet.Name] = true } else { assert.False(t, gotAppSet.RefreshRequired())