Skip to content

Commit

Permalink
chore(tests): assert.Error->require.Error (remainder)
Browse files Browse the repository at this point in the history 
This is part of a series of test tidies started by #13365.

The aim is to enable the testifylint golangci-lint checker.

This commit converts assert.Error checks into require.Error for the
rest of the code base.

In some places checks have been coaleced - in particular the pattern

```go
if assert.Error() {
    assert.Contains(..., "message")
}
```

is now
```go
require.ErrorContains(..., "message")
```

Getting this wrong and missing the Contains is still valid go, so
that's a mistake I may have made.

Signed-off-by: Alan Clucas <[email protected]>
  • Loading branch information
@Joibel
Joibel committed Aug 13, 2024
1 parent a572e71 commit 886146b
Show file tree
Hide file tree
Showing 28 changed files with 896 additions and 1,098 deletions.
2 changes: 1 addition & 1 deletion cmd/argoexec/commands/emissary_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func TestEmissary(t *testing.T) {
go func() {
defer wg.Done()
err := run("sleep 3")
assert.EqualError(t, err, fmt.Sprintf("exit status %d", 128+signal))
require.EqualError(t, err, fmt.Sprintf("exit status %d", 128+signal))
}()
wg.Wait()
}
Expand Down
11 changes: 6 additions & 5 deletions server/artifacts/artifact_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubefake "k8s.io/client-go/kubernetes/fake"

Expand Down Expand Up @@ -664,14 +665,14 @@ func TestArtifactServer_GetArtifactByUIDInvalidRequestPath(t *testing.T) {
// make sure there is no index out of bounds error
assert.Equal(t, 400, recorder.Result().StatusCode)
output, err := io.ReadAll(recorder.Result().Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Contains(t, string(output), "Bad Request")

recorder = httptest.NewRecorder()
s.GetOutputArtifactByUID(recorder, r)
assert.Equal(t, 400, recorder.Result().StatusCode)
output, err = io.ReadAll(recorder.Result().Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Contains(t, string(output), "Bad Request")
}

Expand All @@ -682,7 +683,7 @@ func TestArtifactServer_httpBadRequestError(t *testing.T) {

assert.Equal(t, http.StatusBadRequest, recorder.Result().StatusCode)
output, err := io.ReadAll(recorder.Result().Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Contains(t, string(output), "Bad Request")
}

Expand All @@ -695,7 +696,7 @@ func TestArtifactServer_httpFromError(t *testing.T) {

assert.Equal(t, http.StatusInternalServerError, recorder.Result().StatusCode)
output, err := io.ReadAll(recorder.Result().Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, "Internal Server Error\n", string(output))

recorder = httptest.NewRecorder()
Expand All @@ -705,7 +706,7 @@ func TestArtifactServer_httpFromError(t *testing.T) {

assert.Equal(t, http.StatusUnauthorized, recorder.Result().StatusCode)
output, err = io.ReadAll(recorder.Result().Body)
assert.NoError(t, err)
require.NoError(t, err)
assert.Contains(t, string(output), "Unauthorized")

recorder = httptest.NewRecorder()
Expand Down
6 changes: 3 additions & 3 deletions server/auth/authorizer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
authorizationv1 "k8s.io/api/authorization/v1"
"k8s.io/apimachinery/pkg/runtime"
kubefake "k8s.io/client-go/kubernetes/fake"
Expand All @@ -22,9 +23,8 @@ func TestAuthorizer_CanI(t *testing.T) {
ctx := context.WithValue(context.Background(), KubeKey, kubeClient)
t.Run("CanI", func(t *testing.T) {
allowed, err := CanI(ctx, "", "", "", "")
if assert.NoError(t, err) {
assert.True(t, allowed)
}
require.NoError(t, err)
assert.True(t, allowed)
})
kubeClient.AddReactor("create", "selfsubjectrulesreviews", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &authorizationv1.SelfSubjectRulesReview{
Expand Down
194 changes: 88 additions & 106 deletions server/auth/gatekeeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/metadata"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -111,56 +112,50 @@ func TestServer_GetWFClient(t *testing.T) {
clients := &servertypes.Clients{Workflow: wfClient, Kubernetes: kubeClient}
t.Run("None", func(t *testing.T) {
_, err := NewGatekeeper(Modes{}, clients, nil, nil, clientForAuthorization, "", "", true, resourceCache)
assert.Error(t, err)
require.Error(t, err)
})
t.Run("Invalid", func(t *testing.T) {
g, err := NewGatekeeper(Modes{Client: true}, clients, nil, nil, clientForAuthorization, "", "", true, resourceCache)
if assert.NoError(t, err) {
_, err := g.Context(x("invalid"))
assert.Error(t, err)
}
require.NoError(t, err)
_, err = g.Context(x("invalid"))
require.Error(t, err)
})
t.Run("NotAllowed", func(t *testing.T) {
g, err := NewGatekeeper(Modes{SSO: true}, clients, nil, nil, clientForAuthorization, "", "", true, resourceCache)
if assert.NoError(t, err) {
_, err := g.Context(x("Bearer "))
assert.Error(t, err)
}
require.NoError(t, err)
_, err = g.Context(x("Bearer "))
require.Error(t, err)
})
t.Run("Client", func(t *testing.T) {
g, err := NewGatekeeper(Modes{Client: true}, clients, &rest.Config{Username: "my-username"}, nil, clientForAuthorization, "", "", true, resourceCache)
assert.NoError(t, err)
require.NoError(t, err)
ctx, err := g.Context(x("Bearer "))
if assert.NoError(t, err) {
assert.NotEqual(t, wfClient, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
assert.Nil(t, GetClaims(ctx))
}
require.NoError(t, err)
assert.NotEqual(t, wfClient, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
assert.Nil(t, GetClaims(ctx))
})
t.Run("Server", func(t *testing.T) {
g, err := NewGatekeeper(Modes{Server: true}, clients, &rest.Config{Username: "my-username"}, nil, clientForAuthorization, "", "", true, resourceCache)
assert.NoError(t, err)
require.NoError(t, err)
ctx, err := g.Context(x(""))
if assert.NoError(t, err) {
assert.Equal(t, wfClient, GetWfClient(ctx))
assert.Equal(t, kubeClient, GetKubeClient(ctx))
assert.NotNil(t, GetClaims(ctx))
}
require.NoError(t, err)
assert.Equal(t, wfClient, GetWfClient(ctx))
assert.Equal(t, kubeClient, GetKubeClient(ctx))
assert.NotNil(t, GetClaims(ctx))
})
t.Run("SSO", func(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Claims: jwt.Claims{Subject: "my-sub"}}, nil)
ssoIf.On("IsRBACEnabled").Return(false)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.Context(x("Bearer v2:whatever"))
if assert.NoError(t, err) {
assert.Equal(t, wfClient, GetWfClient(ctx))
assert.Equal(t, kubeClient, GetKubeClient(ctx))
if assert.NotNil(t, GetClaims(ctx)) {
assert.Equal(t, "my-sub", GetClaims(ctx).Subject)
}
}
require.NoError(t, err)
ctx, err := g.Context(x("Bearer v2:whatever"))
require.NoError(t, err)
assert.Equal(t, wfClient, GetWfClient(ctx))
assert.Equal(t, kubeClient, GetKubeClient(ctx))
if assert.NotNil(t, GetClaims(ctx)) {
assert.Equal(t, "my-sub", GetClaims(ctx).Subject)
}
})
hook := &test.Hook{}
Expand All @@ -171,126 +166,113 @@ func TestServer_GetWFClient(t *testing.T) {
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.Context(x("Bearer v2:whatever"))
if assert.NoError(t, err) {
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
}
require.NoError(t, err)
ctx, err := g.Context(x("Bearer v2:whatever"))
require.NoError(t, err)
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
})
t.Run("SSO+RBAC, Namespace delegation ON, precedence=2, Delegated", func(t *testing.T) {
t.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user1-ns"))
if assert.NoError(t, err) {
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "user1-sa", claims.ServiceAccountName)
assert.Equal(t, "user1-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "user1-sa", hook.LastEntry().Data["serviceAccount"])
}
require.NoError(t, err)
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user1-ns"))
require.NoError(t, err)
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "user1-sa", claims.ServiceAccountName)
assert.Equal(t, "user1-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "user1-sa", hook.LastEntry().Data["serviceAccount"])
})
t.Run("SSO+RBAC, Namespace delegation OFF, precedence=2, Not Delegated", func(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user1-ns"))
if assert.NoError(t, err) {
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
}
require.NoError(t, err)
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user1-ns"))
require.NoError(t, err)
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
})
t.Run("SSO+RBAC, Namespace delegation ON, precedence=0, Not delegated", func(t *testing.T) {
t.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user2-ns"))
if assert.NoError(t, err) {
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
}
require.NoError(t, err)
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user2-ns"))
require.NoError(t, err)
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
})
t.Run("SSO+RBAC, Namespace delegation ON, precedence=1, Not delegated", func(t *testing.T) {
t.Setenv("SSO_DELEGATE_RBAC_TO_NAMESPACE", "true")
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"my-group", "other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", false, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user3-ns"))
if assert.NoError(t, err) {
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
}
require.NoError(t, err)
ctx, err := g.ContextWithRequest(x("Bearer v2:whatever"), servertypes.NamespaceHolder("user3-ns"))
require.NoError(t, err)
assert.NotEqual(t, clients, GetWfClient(ctx))
assert.NotEqual(t, kubeClient, GetKubeClient(ctx))
claims := GetClaims(ctx)
if assert.NotNil(t, claims) {
assert.Equal(t, []string{"my-group", "other-group"}, claims.Groups)
assert.Equal(t, "my-sa", claims.ServiceAccountName)
assert.Equal(t, "my-ns", claims.ServiceAccountNamespace)
}
assert.Equal(t, "my-sa", hook.LastEntry().Data["serviceAccount"])
})
t.Run("SSO+RBAC,precedence=0", func(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{Groups: []string{"other-group"}}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
ctx, err := g.Context(x("Bearer v2:whatever"))
if assert.NoError(t, err) {
assert.Equal(t, "my-other-sa", hook.LastEntry().Data["serviceAccount"])
assert.Equal(t, "my-other-sa", GetClaims(ctx).ServiceAccountName)
}
}
require.NoError(t, err)
ctx, err := g.Context(x("Bearer v2:whatever"))
require.NoError(t, err)
assert.Equal(t, "my-other-sa", hook.LastEntry().Data["serviceAccount"])
assert.Equal(t, "my-other-sa", GetClaims(ctx).ServiceAccountName)
})
t.Run("SSO+RBAC,denied", func(t *testing.T) {
ssoIf := &ssomocks.Interface{}
ssoIf.On("Authorize", mock.Anything, mock.Anything).Return(&types.Claims{}, nil)
ssoIf.On("IsRBACEnabled").Return(true)
g, err := NewGatekeeper(Modes{SSO: true}, clients, &rest.Config{Username: "my-username"}, ssoIf, clientForAuthorization, "my-ns", "my-ns", true, resourceCache)
if assert.NoError(t, err) {
_, err := g.Context(x("Bearer v2:whatever"))
assert.EqualError(t, err, "rpc error: code = PermissionDenied desc = not allowed")
}
require.NoError(t, err)
_, err = g.Context(x("Bearer v2:whatever"))
require.EqualError(t, err, "rpc error: code = PermissionDenied desc = not allowed")
})
}

Expand Down
Loading

0 comments on commit 886146b

Please sign in to comment.