Skip to content

Commit

Permalink
fix: Use t.Fatal instead of os.Exit in tests (part 2) (#21003) (#22187)
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
  • Loading branch information
@andrii-korotkov-verkada
andrii-korotkov-verkada authored Mar 6, 2025
1 parent a8b76f2 commit 62ec9fe
Show file tree
Hide file tree
Showing 41 changed files with 343 additions and 288 deletions.
43 changes: 22 additions & 21 deletions server/account/account_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
sessionpkg "github.com/argoproj/argo-cd/v3/pkg/apiclient/session"
"github.com/argoproj/argo-cd/v3/server/session"
"github.com/argoproj/argo-cd/v3/test"
"github.com/argoproj/argo-cd/v3/util/errors"
"github.com/argoproj/argo-cd/v3/util/password"
"github.com/argoproj/argo-cd/v3/util/rbac"
sessionutil "github.com/argoproj/argo-cd/v3/util/session"
Expand All @@ -31,15 +30,17 @@ const (
)

// return an AccountServer which returns fake data
func newTestAccountServer(ctx context.Context, opts ...func(cm *corev1.ConfigMap, secret *corev1.Secret)) (*Server, *session.Server) {
return newTestAccountServerExt(ctx, func(_ jwt.Claims, _ ...any) bool {
func newTestAccountServer(t *testing.T, ctx context.Context, opts ...func(cm *corev1.ConfigMap, secret *corev1.Secret)) (*Server, *session.Server) {
t.Helper()
return newTestAccountServerExt(t, ctx, func(_ jwt.Claims, _ ...any) bool {
return true
}, opts...)
}

func newTestAccountServerExt(ctx context.Context, enforceFn rbac.ClaimsEnforcerFunc, opts ...func(cm *corev1.ConfigMap, secret *corev1.Secret)) (*Server, *session.Server) {
func newTestAccountServerExt(t *testing.T, ctx context.Context, enforceFn rbac.ClaimsEnforcerFunc, opts ...func(cm *corev1.ConfigMap, secret *corev1.Secret)) (*Server, *session.Server) {
t.Helper()
bcrypt, err := password.HashPassword("oldpassword")
errors.CheckError(err)
require.NoError(t, err)
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "argocd-cm",
Expand Down Expand Up @@ -104,7 +105,7 @@ func projTokenContext(ctx context.Context) context.Context {
}

func TestUpdatePassword(t *testing.T) {
accountServer, sessionServer := newTestAccountServer(context.Background())
accountServer, sessionServer := newTestAccountServer(t, context.Background())
ctx := adminContext(context.Background())
var err error

Expand Down Expand Up @@ -140,7 +141,7 @@ func TestUpdatePassword(t *testing.T) {
}

func TestUpdatePassword_AdminUpdatesAnotherUser(t *testing.T) {
accountServer, sessionServer := newTestAccountServer(context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, sessionServer := newTestAccountServer(t, context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.anotherUser"] = "login"
})
ctx := adminContext(context.Background())
Expand All @@ -158,7 +159,7 @@ func TestUpdatePassword_DoesNotHavePermissions(t *testing.T) {
}

t.Run("LocalAccountUpdatesAnotherAccount", func(t *testing.T) {
accountServer, _ := newTestAccountServerExt(context.Background(), enforcer, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServerExt(t, context.Background(), enforcer, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.anotherUser"] = "login"
})
ctx := adminContext(context.Background())
Expand All @@ -167,15 +168,15 @@ func TestUpdatePassword_DoesNotHavePermissions(t *testing.T) {
})

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

func TestUpdatePassword_ProjectToken(t *testing.T) {
accountServer, _ := newTestAccountServer(context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.anotherUser"] = "login"
})
ctx := projTokenContext(context.Background())
Expand All @@ -184,7 +185,7 @@ func TestUpdatePassword_ProjectToken(t *testing.T) {
}

func TestUpdatePassword_OldSSOToken(t *testing.T) {
accountServer, _ := newTestAccountServer(context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.anotherUser"] = "login"
})
ctx := ssoAdminContext(context.Background(), time.Now().Add(-2*common.ChangePasswordSSOTokenMaxAge))
Expand All @@ -194,7 +195,7 @@ func TestUpdatePassword_OldSSOToken(t *testing.T) {
}

func TestUpdatePassword_SSOUserUpdatesAnotherUser(t *testing.T) {
accountServer, sessionServer := newTestAccountServer(context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, sessionServer := newTestAccountServer(t, context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.anotherUser"] = "login"
})
ctx := ssoAdminContext(context.Background(), time.Now())
Expand All @@ -209,15 +210,15 @@ func TestUpdatePassword_SSOUserUpdatesAnotherUser(t *testing.T) {
func TestListAccounts_NoAccountsConfigured(t *testing.T) {
ctx := adminContext(context.Background())

accountServer, _ := newTestAccountServer(ctx)
accountServer, _ := newTestAccountServer(t, ctx)
resp, err := accountServer.ListAccounts(ctx, &account.ListAccountRequest{})
require.NoError(t, err)
assert.Len(t, resp.Items, 1)
}

func TestListAccounts_AccountsAreConfigured(t *testing.T) {
ctx := adminContext(context.Background())
accountServer, _ := newTestAccountServer(ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.account1"] = "apiKey"
cm.Data["accounts.account2"] = "login, apiKey"
cm.Data["accounts.account2.enabled"] = "false"
Expand All @@ -235,7 +236,7 @@ func TestListAccounts_AccountsAreConfigured(t *testing.T) {

func TestGetAccount(t *testing.T) {
ctx := adminContext(context.Background())
accountServer, _ := newTestAccountServer(ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.account1"] = "apiKey"
})

Expand All @@ -255,7 +256,7 @@ func TestGetAccount(t *testing.T) {

func TestCreateToken_SuccessfullyCreated(t *testing.T) {
ctx := adminContext(context.Background())
accountServer, _ := newTestAccountServer(ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.account1"] = "apiKey"
})

Expand All @@ -270,7 +271,7 @@ func TestCreateToken_SuccessfullyCreated(t *testing.T) {

func TestCreateToken_DoesNotHaveCapability(t *testing.T) {
ctx := adminContext(context.Background())
accountServer, _ := newTestAccountServer(ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.account1"] = "login"
})

Expand All @@ -280,7 +281,7 @@ func TestCreateToken_DoesNotHaveCapability(t *testing.T) {

func TestCreateToken_UserSpecifiedID(t *testing.T) {
ctx := adminContext(context.Background())
accountServer, _ := newTestAccountServer(ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, ctx, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
cm.Data["accounts.account1"] = "apiKey"
})

Expand All @@ -294,7 +295,7 @@ func TestCreateToken_UserSpecifiedID(t *testing.T) {

func TestDeleteToken_SuccessfullyRemoved(t *testing.T) {
ctx := adminContext(context.Background())
accountServer, _ := newTestAccountServer(ctx, func(cm *corev1.ConfigMap, secret *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, ctx, func(cm *corev1.ConfigMap, secret *corev1.Secret) {
cm.Data["accounts.account1"] = "apiKey"
secret.Data["accounts.account1.tokens"] = []byte(`[{"id":"123","iat":1583789194,"exp":1583789194}]`)
})
Expand All @@ -309,7 +310,7 @@ func TestDeleteToken_SuccessfullyRemoved(t *testing.T) {
}

func TestCanI_GetLogsAllow(t *testing.T) {
accountServer, _ := newTestAccountServer(context.Background(), func(_ *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServer(t, context.Background(), func(_ *corev1.ConfigMap, _ *corev1.Secret) {
})

ctx := projTokenContext(context.Background())
Expand All @@ -323,7 +324,7 @@ func TestCanI_GetLogsDeny(t *testing.T) {
return false
}

accountServer, _ := newTestAccountServerExt(context.Background(), enforcer, func(_ *corev1.ConfigMap, _ *corev1.Secret) {
accountServer, _ := newTestAccountServerExt(t, context.Background(), enforcer, func(_ *corev1.ConfigMap, _ *corev1.Secret) {
})

ctx := projTokenContext(context.Background())
Expand Down
5 changes: 2 additions & 3 deletions server/application/application_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import (
"github.com/argoproj/argo-cd/v3/util/cache"
"github.com/argoproj/argo-cd/v3/util/cache/appstate"
"github.com/argoproj/argo-cd/v3/util/db"
"github.com/argoproj/argo-cd/v3/util/errors"
"github.com/argoproj/argo-cd/v3/util/grpc"
"github.com/argoproj/argo-cd/v3/util/rbac"
"github.com/argoproj/argo-cd/v3/util/settings"
Expand Down Expand Up @@ -188,9 +187,9 @@ func newTestAppServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforcer),
ctx := context.Background()
db := db.NewDB(testNamespace, settings.NewSettingsManager(ctx, kubeclientset, testNamespace), kubeclientset)
_, err := db.CreateRepository(ctx, fakeRepo())
errors.CheckError(err)
require.NoError(t, err)
_, err = db.CreateCluster(ctx, fakeCluster())
errors.CheckError(err)
require.NoError(t, err)

mockRepoClient := &mocks.Clientset{RepoServerServiceClient: fakeRepoServerClient(false)}

Expand Down
Loading

0 comments on commit 62ec9fe

Please sign in to comment.