Skip to content

Commit

Permalink
fix: use ErrorContains(t, err instead of Contains(t, err.Error() (a…
Browse files Browse the repository at this point in the history
…rgoproj#20220)

Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 authored Oct 4, 2024
1 parent ba0683c commit 1c6ec19
Show file tree
Hide file tree
Showing 30 changed files with 98 additions and 177 deletions.
3 changes: 1 addition & 2 deletions cmd/argocd/commands/admin/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ admissionregistration.k8s.io/MutatingWebhookConfiguration:
require.NoError(t, err)
assert.Contains(t, summary, tc.containsSummary)
} else if tc.containsError != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.containsError)
assert.ErrorContains(t, err, tc.containsError)
}
})
}
Expand Down
6 changes: 2 additions & 4 deletions cmpserver/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,7 @@ func Test_getParametersAnnouncement_invalid_json(t *testing.T) {
Args: []string{`[`},
}
_, err := getParametersAnnouncement(context.Background(), "", []*repoclient.ParameterAnnouncement{}, command, []*apiclient.EnvEntry{})
require.Error(t, err)
assert.Contains(t, err.Error(), "unexpected end of JSON input")
assert.ErrorContains(t, err, "unexpected end of JSON input")
}

func Test_getParametersAnnouncement_bad_command(t *testing.T) {
Expand All @@ -461,8 +460,7 @@ func Test_getParametersAnnouncement_bad_command(t *testing.T) {
Args: []string{"1"},
}
_, err := getParametersAnnouncement(context.Background(), "", []*repoclient.ParameterAnnouncement{}, command, []*apiclient.EnvEntry{})
require.Error(t, err)
assert.Contains(t, err.Error(), "error executing dynamic parameter output command")
assert.ErrorContains(t, err, "error executing dynamic parameter output command")
}

func Test_getTempDirMustCleanup(t *testing.T) {
Expand Down
3 changes: 1 addition & 2 deletions common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ func TestSetOptionalRedisPasswordFromKubeConfig(t *testing.T) {
}
err := SetOptionalRedisPasswordFromKubeConfig(ctx, kubeClient, tc.namespace, redisOptions)
if tc.expectedErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.expectedErr)
require.ErrorContains(t, err, tc.expectedErr)
} else {
require.NoError(t, err)
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/apis/application/v1alpha1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,7 @@ func TestAppProject_IsDestinationPermitted_PermitOnlyProjectScopedClusters(t *te
_, err := proj.IsDestinationPermitted(ApplicationDestination{Server: "https://my-cluster.123.com", Namespace: "default"}, func(_ string) ([]*Cluster, error) {
return nil, errors.New("some error")
})
require.Error(t, err)
assert.Contains(t, err.Error(), "could not retrieve project clusters")
assert.ErrorContains(t, err, "could not retrieve project clusters")
}

func TestAppProject_IsGroupKindPermitted(t *testing.T) {
Expand Down Expand Up @@ -817,8 +816,7 @@ func TestAppProject_InvalidPolicyRules(t *testing.T) {
for _, bad := range badPolicies {
p.Spec.Roles[0].Policies = []string{bad.policy}
err = p.ValidateProject()
require.Error(t, err)
assert.Contains(t, err.Error(), bad.errmsg)
assert.ErrorContains(t, err, bad.errmsg)
}
}

Expand Down
3 changes: 1 addition & 2 deletions reposerver/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,8 +570,7 @@ func TestUnlockGitReferences(t *testing.T) {

t.Run("Test not locked", func(t *testing.T) {
err := cache.UnlockGitReferences("test-repo", "")
require.Error(t, err)
assert.Contains(t, err.Error(), "key is missing")
assert.ErrorContains(t, err, "key is missing")
})

t.Run("Test unlock", func(t *testing.T) {
Expand Down
26 changes: 9 additions & 17 deletions reposerver/repository/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func Test_GenerateManifests_NoOutOfBoundsAccess(t *testing.T) {
res, err := GenerateManifests(context.Background(), repoDir, "", "", &q, false, &git.NoopCredsStore{}, resource.MustParse("0"), nil)
require.Error(t, err)
assert.NotContains(t, err.Error(), mustNotContain)
assert.Contains(t, err.Error(), "illegal filepath")
require.ErrorContains(t, err, "illegal filepath")
assert.Nil(t, res)
})
}
Expand Down Expand Up @@ -684,8 +684,7 @@ func TestInvalidMetadata(t *testing.T) {
src := argoappv1.ApplicationSource{Path: "./testdata/invalid-metadata", Directory: &argoappv1.ApplicationSourceDirectory{Recurse: true}}
q := apiclient.ManifestRequest{Repo: &argoappv1.Repository{}, ApplicationSource: &src, AppLabelKey: "test", AppName: "invalid-metadata", TrackingMethod: "annotation+label"}
_, err := service.GenerateManifest(context.Background(), &q)
require.Error(t, err)
assert.Contains(t, err.Error(), "contains non-string value in the map under key \"invalid\"")
assert.ErrorContains(t, err, "contains non-string value in the map under key \"invalid\"")
}

func TestNilMetadataAccessors(t *testing.T) {
Expand Down Expand Up @@ -763,8 +762,7 @@ func TestGenerateJsonnetLibOutside(t *testing.T) {
ProjectSourceRepos: []string{"*"},
}
_, err := service.GenerateManifest(context.Background(), &q)
require.Error(t, err)
require.Contains(t, err.Error(), "file '../../../testdata/jsonnet/vendor' resolved to outside repository root")
require.ErrorContains(t, err, "file '../../../testdata/jsonnet/vendor' resolved to outside repository root")
}

func TestManifestGenErrorCacheByNumRequests(t *testing.T) {
Expand Down Expand Up @@ -1141,8 +1139,7 @@ func TestHelmWithMissingValueFiles(t *testing.T) {

// Should fail since we're passing a non-existent values file, and error should indicate that
_, err := service.GenerateManifest(context.Background(), req)
require.Error(t, err)
assert.Contains(t, err.Error(), fmt.Sprintf("%s: no such file or directory", missingValuesFile))
require.ErrorContains(t, err, fmt.Sprintf("%s: no such file or directory", missingValuesFile))

// Should template without error even if defining a non-existent values file
req.ApplicationSource.Helm.IgnoreMissingValueFiles = true
Expand Down Expand Up @@ -1330,8 +1327,7 @@ func TestGenerateHelmWithValuesDirectoryTraversalOutsideRepo(t *testing.T) {
ProjectName: "something",
ProjectSourceRepos: []string{"*"},
})
require.Error(t, err)
assert.Contains(t, err.Error(), "outside repository root")
assert.ErrorContains(t, err, "outside repository root")
})

t.Run("Values file with relative path pointing inside repo root", func(t *testing.T) {
Expand Down Expand Up @@ -1385,8 +1381,7 @@ func TestGenerateHelmWithValuesDirectoryTraversalOutsideRepo(t *testing.T) {
ProjectName: "something",
ProjectSourceRepos: []string{"*"},
})
require.Error(t, err)
assert.Contains(t, err.Error(), "outside repository root")
assert.ErrorContains(t, err, "outside repository root")
})

t.Run("Remote values file from forbidden protocol", func(t *testing.T) {
Expand All @@ -1404,8 +1399,7 @@ func TestGenerateHelmWithValuesDirectoryTraversalOutsideRepo(t *testing.T) {
ProjectName: "something",
ProjectSourceRepos: []string{"*"},
})
require.Error(t, err)
assert.Contains(t, err.Error(), "is not allowed")
assert.ErrorContains(t, err, "is not allowed")
})

t.Run("Remote values file from custom allowed protocol", func(t *testing.T) {
Expand All @@ -1423,8 +1417,7 @@ func TestGenerateHelmWithValuesDirectoryTraversalOutsideRepo(t *testing.T) {
ProjectName: "something",
ProjectSourceRepos: []string{"*"},
})
require.Error(t, err)
assert.Contains(t, err.Error(), "s3://my-bucket/my-chart-values.yaml: no such file or directory")
assert.ErrorContains(t, err, "s3://my-bucket/my-chart-values.yaml: no such file or directory")
})
}

Expand Down Expand Up @@ -2857,8 +2850,7 @@ func TestTestRepoOCI(t *testing.T) {
EnableOCI: true,
},
})
require.Error(t, err)
assert.Contains(t, err.Error(), "OCI Helm repository URL should include hostname and port only")
assert.ErrorContains(t, err, "OCI Helm repository URL should include hostname and port only")
}

func Test_getHelmDependencyRepos(t *testing.T) {
Expand Down
14 changes: 5 additions & 9 deletions server/account/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,14 @@ func TestUpdatePassword_DoesNotHavePermissions(t *testing.T) {
})
ctx := adminContext(context.Background())
_, err := accountServer.UpdatePassword(ctx, &account.UpdatePasswordRequest{CurrentPassword: "oldpassword", NewPassword: "newpassword", Name: "anotherUser"})
require.Error(t, err)
assert.Contains(t, err.Error(), "permission denied")
assert.ErrorContains(t, err, "permission denied")
})

t.Run("SSOAccountWithTheSameName", func(t *testing.T) {
accountServer, _ := newTestAccountServerExt(context.Background(), enforcer)
ctx := ssoAdminContext(context.Background(), time.Now())
_, err := accountServer.UpdatePassword(ctx, &account.UpdatePasswordRequest{CurrentPassword: "oldpassword", NewPassword: "newpassword", Name: "admin"})
require.Error(t, err)
assert.Contains(t, err.Error(), "permission denied")
assert.ErrorContains(t, err, "permission denied")
})
}

Expand All @@ -182,8 +180,7 @@ func TestUpdatePassword_ProjectToken(t *testing.T) {
})
ctx := projTokenContext(context.Background())
_, err := accountServer.UpdatePassword(ctx, &account.UpdatePasswordRequest{CurrentPassword: "oldpassword", NewPassword: "newpassword"})
require.Error(t, err)
assert.Contains(t, err.Error(), "password can only be changed for local users")
assert.ErrorContains(t, err, "password can only be changed for local users")
}

func TestUpdatePassword_OldSSOToken(t *testing.T) {
Expand Down Expand Up @@ -291,9 +288,8 @@ func TestCreateToken_UserSpecifiedID(t *testing.T) {
require.NoError(t, err)

_, err = accountServer.CreateToken(ctx, &account.CreateTokenRequest{Name: "account1", Id: "test"})
require.Error(t, err)
assert.Contains(t, err.Error(), "failed to update account with new token:")
assert.Contains(t, err.Error(), "account already has token with id 'test'")
require.ErrorContains(t, err, "failed to update account with new token:")
assert.ErrorContains(t, err, "account already has token with id 'test'")
}

func TestDeleteToken_SuccessfullyRemoved(t *testing.T) {
Expand Down
3 changes: 1 addition & 2 deletions server/application/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2007,7 +2007,7 @@ func TestServer_GetApplicationSyncWindowsState(t *testing.T) {
appServer := newTestAppServer(t, testApp)

active, err := appServer.GetApplicationSyncWindows(context.Background(), &application.ApplicationSyncWindowsQuery{Name: &testApp.Name})
assert.Contains(t, err.Error(), "not exist")
require.ErrorContains(t, err, "not exist")
assert.Nil(t, active)
})
}
Expand Down Expand Up @@ -2718,7 +2718,6 @@ func TestAppNamespaceRestrictions(t *testing.T) {
Name: ptr.To("test-app"),
AppNamespace: ptr.To("argocd-1"),
})
require.Error(t, err)
require.ErrorContains(t, err, "permission denied")
require.Nil(t, app)
})
Expand Down
6 changes: 2 additions & 4 deletions server/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,17 +471,15 @@ func TestRotateAuth(t *testing.T) {
Name: "my-cluster-name",
})

require.Error(t, err)
assert.Contains(t, err.Error(), "Get \"https://my-cluster-name/")
assert.ErrorContains(t, err, "Get \"https://my-cluster-name/")
})

t.Run("RotateAuth by Server - Error from no such host", func(t *testing.T) {
_, err := server.RotateAuth(context.Background(), &clusterapi.ClusterQuery{
Server: "https://my-cluster-name",
})

require.Error(t, err)
assert.Contains(t, err.Error(), "Get \"https://my-cluster-name/")
assert.ErrorContains(t, err, "Get \"https://my-cluster-name/")
})
}

Expand Down
10 changes: 5 additions & 5 deletions server/project/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ p, role:admin, projects, update, *, allow`)
projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projWithRole), enforcer, sync.NewKeyLock(), nil, nil, projInformer, settingsMgr, argoDB, testEnableEventList)
request := &project.ProjectUpdateRequest{Project: projWithRole}
_, err := projectServer.Update(context.Background(), request)
assert.Contains(t, err.Error(), "object must be of form 'test/*', 'test[/<NAMESPACE>]/<APPNAME>' or 'test/<APPNAME>'")
assert.ErrorContains(t, err, "object must be of form 'test/*', 'test[/<NAMESPACE>]/<APPNAME>' or 'test/<APPNAME>'")
})

t.Run("TestValidateProjectIncorrectProjectInRoleFailure", func(t *testing.T) {
Expand All @@ -602,7 +602,7 @@ p, role:admin, projects, update, *, allow`)
projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projWithRole), enforcer, sync.NewKeyLock(), nil, nil, projInformer, settingsMgr, argoDB, testEnableEventList)
request := &project.ProjectUpdateRequest{Project: projWithRole}
_, err := projectServer.Update(context.Background(), request)
assert.Contains(t, err.Error(), "policy subject must be: 'proj:test:testRole'")
assert.ErrorContains(t, err, "policy subject must be: 'proj:test:testRole'")
})

t.Run("TestValidateProjectIncorrectTokenInRoleFailure", func(t *testing.T) {
Expand All @@ -621,7 +621,7 @@ p, role:admin, projects, update, *, allow`)
projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projWithRole), enforcer, sync.NewKeyLock(), nil, nil, projInformer, settingsMgr, argoDB, testEnableEventList)
request := &project.ProjectUpdateRequest{Project: projWithRole}
_, err := projectServer.Update(context.Background(), request)
assert.Contains(t, err.Error(), "policy subject must be: 'proj:test:testRole'")
assert.ErrorContains(t, err, "policy subject must be: 'proj:test:testRole'")
})

t.Run("TestValidateProjectInvalidEffectFailure", func(t *testing.T) {
Expand All @@ -639,7 +639,7 @@ p, role:admin, projects, update, *, allow`)
projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projWithRole), enforcer, sync.NewKeyLock(), nil, nil, projInformer, settingsMgr, argoDB, testEnableEventList)
request := &project.ProjectUpdateRequest{Project: projWithRole}
_, err := projectServer.Update(context.Background(), request)
assert.Contains(t, err.Error(), "effect must be: 'allow' or 'deny'")
assert.ErrorContains(t, err, "effect must be: 'allow' or 'deny'")
})

t.Run("TestNormalizeProjectRolePolicies", func(t *testing.T) {
Expand Down Expand Up @@ -685,7 +685,7 @@ p, role:admin, projects, update, *, allow`)
argoDB := db.NewDB("default", settingsMgr, kubeclientset)
projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(projectWithSyncWindows), enforcer, sync.NewKeyLock(), sessionMgr, nil, projInformer, settingsMgr, argoDB, testEnableEventList)
res, err := projectServer.GetSyncWindowsState(ctx, &project.SyncWindowsQuery{Name: "incorrect"})
assert.Contains(t, err.Error(), "not found")
require.ErrorContains(t, err, "not found")
assert.Nil(t, res)
})

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app_deletion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestDeletingAppByLabel(t *testing.T) {
// delete is unsuccessful since no selector match
AndCLIOutput(
func(output string, err error) {
assert.Contains(t, err.Error(), "no apps match selector foo=baz")
assert.ErrorContains(t, err, "no apps match selector foo=baz")
},
).
When().
Expand Down
15 changes: 5 additions & 10 deletions test/e2e/app_management_ns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ func TestNamespacedGetLogsDenySwitchOn(t *testing.T) {
Expect(HealthIs(health.HealthStatusHealthy)).
And(func(app *Application) {
_, err := RunCliWithRetry(5, "app", "logs", ctx.AppQualifiedName(), "--kind", "Deployment", "--group", "", "--name", "guestbook-ui")
require.Error(t, err)
assert.Contains(t, err.Error(), "permission denied")
assert.ErrorContains(t, err, "permission denied")
})
}

Expand Down Expand Up @@ -667,8 +666,7 @@ func TestNamespacedAppWithSecrets(t *testing.T) {
_, err = RunCli("app", "patch-resource", ctx.AppQualifiedName(), "--resource-name", "test-secret",
"--kind", "Secret", "--patch", `{"op": "add", "path": "/data", "value": "hello"}'`,
"--patch-type", "application/json-patch+json")
require.Error(t, err)
assert.Contains(t, err.Error(), fmt.Sprintf("failed to patch Secret %s/test-secret", DeploymentNamespace()))
require.ErrorContains(t, err, fmt.Sprintf("failed to patch Secret %s/test-secret", DeploymentNamespace()))
assert.NotContains(t, err.Error(), "username")
assert.NotContains(t, err.Error(), "password")

Expand Down Expand Up @@ -973,8 +971,7 @@ func TestNamespacedSyncResourceByLabel(t *testing.T) {
Expect(SyncStatusIs(SyncStatusCodeSynced)).
And(func(app *Application) {
_, err := RunCli("app", "sync", ctx.AppQualifiedName(), "--label", "this-label=does-not-exist")
require.Error(t, err)
assert.Contains(t, err.Error(), "level=fatal")
assert.ErrorContains(t, err, "level=fatal")
})
}

Expand Down Expand Up @@ -1045,8 +1042,7 @@ func TestNamespacedNoLocalSyncWithAutosyncEnabled(t *testing.T) {
require.NoError(t, err)

_, err = RunCli("app", "sync", app.QualifiedName(), "--local", guestbookPathLocal)
require.Error(t, err)
assert.Contains(t, err.Error(), "Cannot use local sync")
assert.ErrorContains(t, err, "Cannot use local sync")
})
}

Expand Down Expand Up @@ -1093,8 +1089,7 @@ func assertNSResourceActions(t *testing.T, appName string, successful bool) {
if successful {
require.NoError(t, err)
} else {
require.Error(t, err)
assert.Contains(t, err.Error(), message)
assert.ErrorContains(t, err, message)
}
}

Expand Down
Loading

0 comments on commit 1c6ec19

Please sign in to comment.