Skip to content

Commit

Permalink
chore: use testify instead of testing.Fatal or testing.Error in server (
Browse files Browse the repository at this point in the history 
argoproj#20755)

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
  • Loading branch information
@mmorel-35
mmorel-35 authored Nov 12, 2024
1 parent 2a72df2 commit 993d79c
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 171 deletions.
186 changes: 89 additions & 97 deletions server/application/application_test.go
View file

Large diffs are not rendered by default.

16 changes: 4 additions & 12 deletions server/application/terminal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ func TestPodExists(t *testing.T) {
} {
t.Run(tcase.name, func(t *testing.T) {
result := podExists(tcase.treeNodes, tcase.podName, tcase.namespace)
if result != tcase.expectedResult {
t.Errorf("Expected result %v, but got %v", tcase.expectedResult, result)
}
assert.Equalf(t, tcase.expectedResult, result, "Expected result %v, but got %v", tcase.expectedResult, result)
})
}
}
Expand Down Expand Up @@ -110,9 +108,7 @@ func TestIsValidPodName(t *testing.T) {
} {
t.Run(tcase.name, func(t *testing.T) {
result := argo.IsValidPodName(tcase.resourceName)
if result != tcase.expectedResult {
t.Errorf("Expected result %v, but got %v", tcase.expectedResult, result)
}
assert.Equalf(t, tcase.expectedResult, result, "Expected result %v, but got %v", tcase.expectedResult, result)
})
}
}
Expand Down Expand Up @@ -141,9 +137,7 @@ func TestIsValidNamespaceName(t *testing.T) {
} {
t.Run(tcase.name, func(t *testing.T) {
result := argo.IsValidNamespaceName(tcase.resourceName)
if result != tcase.expectedResult {
t.Errorf("Expected result %v, but got %v", tcase.expectedResult, result)
}
assert.Equalf(t, tcase.expectedResult, result, "Expected result %v, but got %v", tcase.expectedResult, result)
})
}
}
Expand Down Expand Up @@ -172,9 +166,7 @@ func TestIsValidContainerNameName(t *testing.T) {
} {
t.Run(tcase.name, func(t *testing.T) {
result := argo.IsValidContainerName(tcase.resourceName)
if result != tcase.expectedResult {
t.Errorf("Expected result %v, but got %v", tcase.expectedResult, result)
}
assert.Equalf(t, tcase.expectedResult, result, "Expected result %v, but got %v", tcase.expectedResult, result)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/application/websocket_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func TestValidateWithoutPermissions(t *testing.T) {
ts.ctx = context.WithValue(context.Background(), "claims", &jwt.MapClaims{"groups": []string{"test"}})
_, err := ts.validatePermissions([]byte{})
require.Error(t, err)
assert.Equal(t, permissionDeniedErr.Error(), err.Error())
assert.EqualError(t, err, permissionDeniedErr.Error())
}

testServerConnection(t, validate, true)
Expand Down
8 changes: 4 additions & 4 deletions server/applicationset/applicationset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func TestCreateAppSetTemplatedProject(t *testing.T) {
Applicationset: testAppSet,
}
_, err := appServer.Create(context.Background(), &createReq)
assert.Equal(t, "error validating ApplicationSets: the Argo CD API does not currently support creating ApplicationSets with templated `project` fields", err.Error())
assert.EqualError(t, err, "error validating ApplicationSets: the Argo CD API does not currently support creating ApplicationSets with templated `project` fields")
}

func TestCreateAppSetWrongNamespace(t *testing.T) {
Expand All @@ -370,7 +370,7 @@ func TestCreateAppSetWrongNamespace(t *testing.T) {
}
_, err := appServer.Create(context.Background(), &createReq)

assert.Equal(t, "namespace 'NOT-ALLOWED' is not permitted", err.Error())
assert.EqualError(t, err, "namespace 'NOT-ALLOWED' is not permitted")
}

func TestCreateAppSetDryRun(t *testing.T) {
Expand Down Expand Up @@ -466,7 +466,7 @@ func TestGetAppSet(t *testing.T) {
appsetQuery := applicationset.ApplicationSetGetQuery{Name: "AppSet1", AppsetNamespace: "NOT-ALLOWED"}

_, err := appSetServer.Get(context.Background(), &appsetQuery)
assert.Equal(t, "namespace 'NOT-ALLOWED' is not permitted", err.Error())
assert.EqualError(t, err, "namespace 'NOT-ALLOWED' is not permitted")
})
}

Expand Down Expand Up @@ -636,6 +636,6 @@ func TestResourceTree(t *testing.T) {
appsetQuery := applicationset.ApplicationSetTreeQuery{Name: "AppSet1", AppsetNamespace: "NOT-ALLOWED"}

_, err := appSetServer.ResourceTree(context.Background(), &appsetQuery)
assert.Equal(t, "namespace 'NOT-ALLOWED' is not permitted", err.Error())
assert.EqualError(t, err, "namespace 'NOT-ALLOWED' is not permitted")
})
}
38 changes: 12 additions & 26 deletions server/cluster/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,7 @@ func TestRotateAuth(t *testing.T) {
}

configMarshal, err := json.Marshal(config)
if err != nil {
t.Errorf("failed to marshal config for test: %v", err)
}
require.NoError(t, err, "failed to marshal config for test")

clientset := getClientset(nil, testNamespace,
&corev1.Secret{
Expand Down Expand Up @@ -716,9 +714,7 @@ func TestListCluster(t *testing.T) {
t.Errorf("Server.List() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Server.List() = %v, want %v", got, tt.want)
}
assert.Truef(t, reflect.DeepEqual(got, tt.want), "Server.List() = %v, want %v", got, tt.want)
})
}
}
Expand Down Expand Up @@ -801,14 +797,12 @@ func TestNoClusterEnumeration(t *testing.T) {
_, err := server.Get(context.Background(), &clusterapi.ClusterQuery{
Name: "cluster-not-exists",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
require.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")

_, err = server.Get(context.Background(), &clusterapi.ClusterQuery{
Name: "test/ing",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
assert.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
})

t.Run("Update", func(t *testing.T) {
Expand All @@ -817,57 +811,49 @@ func TestNoClusterEnumeration(t *testing.T) {
Name: "cluster-not-exists",
},
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
require.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")

_, err = server.Update(context.Background(), &clusterapi.ClusterUpdateRequest{
Cluster: &v1alpha1.Cluster{
Name: "test/ing",
},
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
assert.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
})

t.Run("Delete", func(t *testing.T) {
_, err := server.Delete(context.Background(), &clusterapi.ClusterQuery{
Server: "https://127.0.0.2",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
require.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")

_, err = server.Delete(context.Background(), &clusterapi.ClusterQuery{
Server: "https://127.0.0.1",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
assert.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
})

t.Run("RotateAuth", func(t *testing.T) {
_, err := server.RotateAuth(context.Background(), &clusterapi.ClusterQuery{
Server: "https://127.0.0.2",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
require.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")

_, err = server.RotateAuth(context.Background(), &clusterapi.ClusterQuery{
Server: "https://127.0.0.1",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
assert.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
})

t.Run("InvalidateCache", func(t *testing.T) {
_, err := server.InvalidateCache(context.Background(), &clusterapi.ClusterQuery{
Server: "https://127.0.0.2",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
require.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")

_, err = server.InvalidateCache(context.Background(), &clusterapi.ClusterQuery{
Server: "https://127.0.0.1",
})
require.Error(t, err)
assert.Equal(t, common.PermissionDeniedAPIError.Error(), err.Error(), "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
assert.ErrorIs(t, err, common.PermissionDeniedAPIError, "error message must be _only_ the permission error, to avoid leaking information about cluster existence")
})
}
36 changes: 9 additions & 27 deletions server/extension/extension_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will build RequestResources successfully", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "namespace:app-name")
r.Header.Add(extension.HeaderArgoCDProjectName, "project-name")

Expand All @@ -47,9 +45,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will return error if application is malformatted", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "no-namespace")

// when
Expand All @@ -62,9 +58,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will return error if application header is missing", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDProjectName, "project-name")

// when
Expand All @@ -77,9 +71,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will return error if project header is missing", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "namespace:app-name")

// when
Expand All @@ -92,9 +84,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will return error if invalid namespace", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "bad%namespace:app-name")
r.Header.Add(extension.HeaderArgoCDProjectName, "project-name")

Expand All @@ -108,9 +98,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will return error if invalid app name", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "namespace:bad@app")
r.Header.Add(extension.HeaderArgoCDProjectName, "project-name")

Expand All @@ -124,9 +112,7 @@ func TestValidateHeaders(t *testing.T) {
t.Run("will return error if invalid project name", func(t *testing.T) {
// given
r, err := http.NewRequest(http.MethodGet, "http://null", nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "namespace:app")
r.Header.Add(extension.HeaderArgoCDProjectName, "bad^project")

Expand Down Expand Up @@ -379,9 +365,7 @@ func TestCallExtension(t *testing.T) {
startTestServer := func(t *testing.T, f *fixture) *httptest.Server {
t.Helper()
err := f.manager.RegisterExtensions()
if err != nil {
t.Fatalf("error starting test server: %s", err)
}
require.NoError(t, err, "error starting test server")
return httptest.NewServer(f.mux)
}

Expand All @@ -396,9 +380,7 @@ func TestCallExtension(t *testing.T) {
newExtensionRequest := func(t *testing.T, method, url string) *http.Request {
t.Helper()
r, err := http.NewRequest(method, url, nil)
if err != nil {
t.Fatalf("error initializing request: %s", err)
}
require.NoError(t, err, "error initializing request")
r.Header.Add(extension.HeaderArgoCDApplicationName, "namespace:app-name")
r.Header.Add(extension.HeaderArgoCDProjectName, defaultProjectName)
return r
Expand Down
7 changes: 4 additions & 3 deletions server/project/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package project
import (
"reflect"
"testing"

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

func TestUnique(t *testing.T) {
Expand Down Expand Up @@ -39,9 +41,8 @@ func TestUnique(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := unique(tt.slice); !reflect.DeepEqual(got, tt.want) {
t.Errorf("unique() = %v, want %v", got, tt.want)
}
got := unique(tt.slice)
assert.Truef(t, reflect.DeepEqual(got, tt.want), "unique() = %v, want %v", got, tt.want)
})
}
}
2 changes: 1 addition & 1 deletion server/repository/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ func TestRepositoryServer(t *testing.T) {
Repo: url,
})
assert.Nil(t, repo)
assert.Equal(t, "rpc error: code = NotFound desc = repo 'https://test' not found", err.Error())
assert.EqualError(t, err, "rpc error: code = NotFound desc = repo 'https://test' not found")
})

t.Run("Test_GetRepoIsSanitized", func(t *testing.T) {
Expand Down

0 comments on commit 993d79c

Please sign in to comment.