Skip to content

Commit

Permalink
chore: use testify instead of testing.Fatal or testing.Error in appli…
Browse files Browse the repository at this point in the history
…cationset

Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 committed Nov 8, 2024
1 parent 5d0a3e6 commit 001c137
Show file tree
Hide file tree
Showing 16 changed files with 72 additions and 91 deletions.
58 changes: 29 additions & 29 deletions applicationset/controllers/applicationset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.NoErrorf(t, err, "Unexpected error")

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.NoErrorf(t, err, "Unexpected error")

retrievedApp := v1alpha1.Application{}
err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp)
require.NoError(t, err, "Unexpected error")
require.NoErrorf(t, err, "Unexpected error")

// App on the cluster should have the expected finalizers
assert.ElementsMatch(t, c.expectedFinalizers, retrievedApp.Finalizers)
Expand Down Expand Up @@ -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.NoErrorf(t, err, "Unexpected error")

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.NoErrorf(t, err, "Unexpected error")

retrievedApp := v1alpha1.Application{}
err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp)
require.NoError(t, err, "Unexpected error")
require.NoErrorf(t, err, "Unexpected error")

finalizerRemoved := len(retrievedApp.Finalizers) == 0

Expand Down Expand Up @@ -1402,7 +1402,7 @@ func TestRemoveOwnerReferencesOnDeleteAppSet(t *testing.T) {
}

err := controllerutil.SetControllerReference(&appSet, &app, scheme)
require.NoError(t, err, "Unexpected error")
require.NoErrorf(t, err, "Unexpected error")

initObjs := []crtclient.Object{&app, &appSet}

Expand All @@ -1418,11 +1418,11 @@ func TestRemoveOwnerReferencesOnDeleteAppSet(t *testing.T) {
}

err = r.removeOwnerReferencesOnDeleteAppSet(context.Background(), appSet)
require.NoError(t, err, "Unexpected error")
require.NoErrorf(t, err, "Unexpected error")

retrievedApp := v1alpha1.Application{}
err = client.Get(context.Background(), crtclient.ObjectKeyFromObject(&app), &retrievedApp)
require.NoError(t, err, "Unexpected error")
require.NoErrorf(t, err, "Unexpected error")

ownerReferencesRemoved := len(retrievedApp.OwnerReferences) == 0
assert.True(t, ownerReferencesRemoved)
Expand Down Expand Up @@ -2092,16 +2092,16 @@ func TestValidateGeneratedApplications(t *testing.T) {
}

if len(errorMessages) == 0 {
assert.Empty(t, cc.expectedErrors, "Expected errors but none were seen")
assert.Emptyf(t, cc.expectedErrors, "Expected errors but none were seen")
} else {
// An error was returned: it should be expected
matched := false
for _, expectedErr := range cc.expectedErrors {
foundMatch := strings.Contains(strings.Join(errorMessages, ";"), expectedErr)
assert.True(t, foundMatch, "Unble to locate expected error: %s", cc.expectedErrors)
assert.Truef(t, foundMatch, "Unble to locate expected error: %s", cc.expectedErrors)
matched = matched || foundMatch
}
assert.True(t, matched, "An unexpected error occurrred: %v", err)
assert.Truef(t, matched, "An unexpected error occurrred: %v", err)
// validation message was returned: it should be expected
matched = false
foundMatch := reflect.DeepEqual(validationErrors, cc.validationErrors)
Expand All @@ -2110,9 +2110,9 @@ func TestValidateGeneratedApplications(t *testing.T) {
message = v.Error()
break
}
assert.True(t, foundMatch, "Unble to locate validation message: %s", message)
assert.Truef(t, foundMatch, "Unble to locate validation message: %s", message)
matched = matched || foundMatch
assert.True(t, matched, "An unexpected error occurrred: %v", err)
assert.Truef(t, matched, "An unexpected error occurrred: %v", err)
}
})
}
Expand Down Expand Up @@ -2294,7 +2294,7 @@ func TestSetApplicationSetStatusCondition(t *testing.T) {
}
}

assert.False(t, isProgressingCondition, "no RolloutProgressing should be set for applicationsets that don't have rolling strategy")
assert.Falsef(t, isProgressingCondition, "no RolloutProgressing should be set for applicationsets that don't have rolling strategy")
},
},
{
Expand Down Expand Up @@ -2357,7 +2357,7 @@ func TestSetApplicationSetStatusCondition(t *testing.T) {
}
}

assert.True(t, isProgressingCondition, "RolloutProgressing should be set for rollout strategy appset")
assert.Truef(t, isProgressingCondition, "RolloutProgressing should be set for rollout strategy appset")
},
},
}
Expand Down Expand Up @@ -3727,8 +3727,8 @@ func TestBuildAppDependencyList(t *testing.T) {
}

appDependencyList, appStepMap := r.buildAppDependencyList(log.NewEntry(log.StandardLogger()), cc.appSet, cc.apps)
assert.Equal(t, cc.expectedList, appDependencyList, "expected appDependencyList did not match actual")
assert.Equal(t, cc.expectedStepMap, appStepMap, "expected appStepMap did not match actual")
assert.Equalf(t, cc.expectedList, appDependencyList, "expected appDependencyList did not match actual")
assert.Equalf(t, cc.expectedStepMap, appStepMap, "expected appStepMap did not match actual")
})
}
}
Expand Down Expand Up @@ -4394,7 +4394,7 @@ func TestBuildAppSyncMap(t *testing.T) {
}

appSyncMap := r.buildAppSyncMap(cc.appSet, cc.appDependencyList, cc.appMap)
assert.Equal(t, cc.expectedMap, appSyncMap, "expected appSyncMap did not match actual")
assert.Equalf(t, cc.expectedMap, appSyncMap, "expected appSyncMap did not match actual")
})
}
}
Expand Down Expand Up @@ -5348,8 +5348,8 @@ func TestUpdateApplicationSetApplicationStatus(t *testing.T) {
appStatuses[i].LastTransitionTime = nil
}

require.NoError(t, err, "expected no errors, but errors occurred")
assert.Equal(t, cc.expectedAppStatus, appStatuses, "expected appStatuses did not match actual")
require.NoErrorf(t, err, "expected no errors, but errors occurred")
assert.Equalf(t, cc.expectedAppStatus, appStatuses, "expected appStatuses did not match actual")
})
}
}
Expand Down Expand Up @@ -6097,8 +6097,8 @@ func TestUpdateApplicationSetApplicationStatusProgress(t *testing.T) {
appStatuses[i].LastTransitionTime = nil
}

require.NoError(t, err, "expected no errors, but errors occurred")
assert.Equal(t, cc.expectedAppStatus, appStatuses, "expected appStatuses did not match actual")
require.NoErrorf(t, err, "expected no errors, but errors occurred")
assert.Equalf(t, cc.expectedAppStatus, appStatuses, "expected appStatuses did not match actual")
})
}
}
Expand Down Expand Up @@ -6303,8 +6303,8 @@ func TestUpdateResourceStatus(t *testing.T) {

err := r.updateResourcesStatus(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.apps)

require.NoError(t, err, "expected no errors, but errors occurred")
assert.Equal(t, cc.expectedResources, cc.appSet.Status.Resources, "expected resources did not match actual")
require.NoErrorf(t, err, "expected no errors, but errors occurred")
assert.Equalf(t, cc.expectedResources, cc.appSet.Status.Resources, "expected resources did not match actual")
})
}
}
Expand Down Expand Up @@ -6392,15 +6392,15 @@ func TestResourceStatusAreOrdered(t *testing.T) {
}

err := r.updateResourcesStatus(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.apps)
require.NoError(t, err, "expected no errors, but errors occurred")
require.NoErrorf(t, err, "expected no errors, but errors occurred")

err = r.updateResourcesStatus(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.apps)
require.NoError(t, err, "expected no errors, but errors occurred")
require.NoErrorf(t, err, "expected no errors, but errors occurred")

err = r.updateResourcesStatus(context.TODO(), log.NewEntry(log.StandardLogger()), &cc.appSet, cc.apps)
require.NoError(t, err, "expected no errors, but errors occurred")
require.NoErrorf(t, err, "expected no errors, but errors occurred")

assert.Equal(t, cc.expectedResources, cc.appSet.Status.Resources, "expected resources did not match actual")
assert.Equalf(t, cc.expectedResources, cc.appSet.Status.Resources, "expected resources did not match actual")
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion applicationset/generators/pull_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func TestAllowedSCMProviderPullRequest(t *testing.T) {

_, err := pullRequestGenerator.GenerateParams(&applicationSetInfo.Spec.Generators[0], &applicationSetInfo, nil)

require.Error(t, err, "Must return an error")
require.Errorf(t, err, "Must return an error")
var expectedError ErrDisallowedSCMProvider
assert.ErrorAs(t, err, &expectedError)
})
Expand Down
2 changes: 1 addition & 1 deletion applicationset/generators/scm_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func TestAllowedSCMProvider(t *testing.T) {

_, err := scmGenerator.GenerateParams(&applicationSetInfo.Spec.Generators[0], &applicationSetInfo, nil)

require.Error(t, err, "Must return an error")
require.Errorf(t, err, "Must return an error")
var expectedError ErrDisallowedSCMProvider
assert.ErrorAs(t, err, &expectedError)
})
Expand Down
21 changes: 6 additions & 15 deletions applicationset/services/internal/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestClient(t *testing.T) {
Expand All @@ -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.NoErrorf(t, err, "Failed to create client: %v", err)
}

func TestClientDo(t *testing.T) {
Expand Down Expand Up @@ -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.NoErrorf(t, err, "NewClient returned unexpected error: %v", err)

req, err := client.NewRequest("POST", "", cc.params, nil)
if err != nil {
t.Fatalf("NewRequest returned unexpected error: %v", err)
}
require.NoErrorf(t, err, "NewRequest returned unexpected error: %v", err)

var data []map[string]interface{}

Expand All @@ -149,12 +144,8 @@ func TestCheckResponse(t *testing.T) {
}

err := CheckResponse(resp)
if err == nil {
t.Error("Expected an error, got nil")
}
require.Errorf(t, err, "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())
}
assert.Equalf(t, expected, err.Error(), "Expected error '%s', got '%s'", expected, err.Error())
}
13 changes: 4 additions & 9 deletions applicationset/services/plugin/plugin_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestPlugin(t *testing.T) {
Expand All @@ -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.NoErrorf(t, err, "unexpected error: %v", err)

data, err := client.List(context.Background(), nil)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
require.NoErrorf(t, err, "unexpected error: %v", 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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func TestListPullRequestTLS(t *testing.T) {
for _, cert := range ts.TLS.Certificates {
for _, c := range cert.Certificate {
parsedCert, err := x509.ParseCertificate(c)
require.NoError(t, err, "Failed to parse certificate")
require.NoErrorf(t, err, "Failed to parse certificate")
certs = append(certs, pem.EncodeToMemory(&pem.Block{
Type: "CERTIFICATE",
Bytes: parsedCert.Raw,
Expand Down
5 changes: 2 additions & 3 deletions applicationset/services/pull_request/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
assert.Equalf(t, got, c.Expect, "expect: %v, got: %v", c.Expect, got)
})
}
}
Expand Down
11 changes: 4 additions & 7 deletions applicationset/services/pull_request/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -174,7 +171,7 @@ func TestListWithStateTLS(t *testing.T) {
for _, cert := range ts.TLS.Certificates {
for _, c := range cert.Certificate {
parsedCert, err := x509.ParseCertificate(c)
require.NoError(t, err, "Failed to parse certificate")
require.NoErrorf(t, err, "Failed to parse certificate")
certs = append(certs, pem.EncodeToMemory(&pem.Block{
Type: "CERTIFICATE",
Bytes: parsedCert.Raw,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func TestAWSCodeCommitListRepos(t *testing.T) {
assert.Equal(t, originRepo.id, repo.RepositoryId)
assert.Equal(t, originRepo.defaultBranch, repo.Branch)
assert.Equal(t, originRepo.expectedCloneUrl, repo.URL)
assert.Empty(t, repo.SHA, "SHA is always empty")
assert.Emptyf(t, repo.SHA, "SHA is always empty")
}
}
})
Expand Down
2 changes: 1 addition & 1 deletion applicationset/services/scm_provider/azure_devops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func TestGetAzureDevopsRepositories(t *testing.T) {
repositories, err := provider.ListRepos(ctx, "https")

if testCase.getRepositoriesError != nil {
require.Error(t, err, "Expected an error from test case %v", testCase.name)
require.Errorf(t, err, "Expected an error from test case %v", testCase.name)
}

if testCase.expectedNumberOfRepos == 0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ func TestListReposTLS(t *testing.T) {
for _, cert := range ts.TLS.Certificates {
for _, c := range cert.Certificate {
parsedCert, err := x509.ParseCertificate(c)
require.NoError(t, err, "Failed to parse certificate")
require.NoErrorf(t, err, "Failed to parse certificate")
certs = append(certs, pem.EncodeToMemory(&pem.Block{
Type: "CERTIFICATE",
Bytes: parsedCert.Raw,
Expand Down
2 changes: 1 addition & 1 deletion applicationset/services/scm_provider/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@ func TestGetBranchesTLS(t *testing.T) {
for _, cert := range ts.TLS.Certificates {
for _, c := range cert.Certificate {
parsedCert, err := x509.ParseCertificate(c)
require.NoError(t, err, "Failed to parse certificate")
require.NoErrorf(t, err, "Failed to parse certificate")
certs = append(certs, pem.EncodeToMemory(&pem.Block{
Type: "CERTIFICATE",
Bytes: parsedCert.Raw,
Expand Down
Loading

0 comments on commit 001c137

Please sign in to comment.