Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable testifylint and ginkgo linters #6091

Merged
merged 2 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ linters:
- goheader
- gocritic
- forbidigo
- testifylint
- ginkgolinter

linters-settings:
misspell:
Expand Down Expand Up @@ -56,6 +58,10 @@ linters-settings:
- name: use-any
- name: var-declaration
- name: var-naming
testifylint:
enable-all: true
ginkgolinter:
forbid-focus-container: true

issues:
exclude-rules:
Expand Down
30 changes: 15 additions & 15 deletions apis/projectcontour/v1alpha1/accesslog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,25 @@ import (
"testing"

"github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestValidateAccessLogType(t *testing.T) {
assert.Error(t, v1alpha1.AccessLogType("").Validate())
assert.Error(t, v1alpha1.AccessLogType("foo").Validate())
require.Error(t, v1alpha1.AccessLogType("").Validate())
require.Error(t, v1alpha1.AccessLogType("foo").Validate())

assert.NoError(t, v1alpha1.EnvoyAccessLog.Validate())
assert.NoError(t, v1alpha1.JSONAccessLog.Validate())
require.NoError(t, v1alpha1.EnvoyAccessLog.Validate())
require.NoError(t, v1alpha1.JSONAccessLog.Validate())
}

func TestValidateAccessLogLevel(t *testing.T) {
assert.Error(t, v1alpha1.AccessLogLevel("").Validate())
assert.Error(t, v1alpha1.AccessLogLevel("foo").Validate())
require.Error(t, v1alpha1.AccessLogLevel("").Validate())
require.Error(t, v1alpha1.AccessLogLevel("foo").Validate())

assert.NoError(t, v1alpha1.LogLevelInfo.Validate())
assert.NoError(t, v1alpha1.LogLevelError.Validate())
assert.NoError(t, v1alpha1.LogLevelCritical.Validate())
assert.NoError(t, v1alpha1.LogLevelDisabled.Validate())
require.NoError(t, v1alpha1.LogLevelInfo.Validate())
require.NoError(t, v1alpha1.LogLevelError.Validate())
require.NoError(t, v1alpha1.LogLevelCritical.Validate())
require.NoError(t, v1alpha1.LogLevelDisabled.Validate())
}

func TestValidateAccessLogJSONFields(t *testing.T) {
Expand All @@ -59,7 +59,7 @@ func TestValidateAccessLogJSONFields(t *testing.T) {
}

for _, c := range errorCases {
assert.Error(t, v1alpha1.AccessLogJSONFields(c).Validate(), c)
require.Error(t, v1alpha1.AccessLogJSONFields(c).Validate(), c)
}

successCases := [][]string{
Expand All @@ -82,7 +82,7 @@ func TestValidateAccessLogJSONFields(t *testing.T) {
}

for _, c := range successCases {
assert.NoError(t, v1alpha1.AccessLogJSONFields(c).Validate(), c)
require.NoError(t, v1alpha1.AccessLogJSONFields(c).Validate(), c)
}
}

Expand All @@ -103,7 +103,7 @@ func TestAccessLogFormatString(t *testing.T) {
}

for _, c := range errorCases {
assert.Error(t, v1alpha1.AccessLogFormatString(c).Validate(), c)
require.Error(t, v1alpha1.AccessLogFormatString(c).Validate(), c)
}

successCases := []string{
Expand Down Expand Up @@ -135,6 +135,6 @@ func TestAccessLogFormatString(t *testing.T) {
}

for _, c := range successCases {
assert.NoError(t, v1alpha1.AccessLogFormatString(c).Validate(), c)
require.NoError(t, v1alpha1.AccessLogFormatString(c).Validate(), c)
}
}
32 changes: 12 additions & 20 deletions cmd/contour/certgen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/projectcontour/contour/pkg/certs"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
)

Expand All @@ -48,9 +49,7 @@ func TestGeneratedSecretsValid(t *testing.T) {
Lifetime: conf.Lifetime,
Namespace: conf.Namespace,
})
if err != nil {
t.Fatalf("failed to generate certificates: %s", err)
}
require.NoError(t, err, "failed to generate certificates")

secrets, errs := certgen.AsSecrets(conf.Namespace, "", certificates)
if len(errs) > 0 {
Expand Down Expand Up @@ -93,7 +92,7 @@ func TestGeneratedSecretsValid(t *testing.T) {
}

pemBlock, _ := pem.Decode(s.Data[corev1.TLSCertKey])
assert.Equal(t, pemBlock.Type, "CERTIFICATE")
assert.Equal(t, "CERTIFICATE", pemBlock.Type)

cert, err := x509.ParseCertificate(pemBlock.Bytes)
if err != nil {
Expand Down Expand Up @@ -127,9 +126,7 @@ func TestSecretNamePrefix(t *testing.T) {
Lifetime: conf.Lifetime,
Namespace: conf.Namespace,
})
if err != nil {
t.Fatalf("failed to generate certificates: %s", err)
}
require.NoError(t, err, "failed to generate certificates")

secrets, errs := certgen.AsSecrets(conf.Namespace, conf.NameSuffix, certificates)
if len(errs) > 0 {
Expand Down Expand Up @@ -172,18 +169,15 @@ func TestSecretNamePrefix(t *testing.T) {
}

pemBlock, _ := pem.Decode(s.Data[corev1.TLSCertKey])
assert.Equal(t, pemBlock.Type, "CERTIFICATE")
assert.Equal(t, "CERTIFICATE", pemBlock.Type)

cert, err := x509.ParseCertificate(pemBlock.Bytes)
if err != nil {
t.Errorf("failed to parse X509 certificate: %s", err)
}
require.NoError(t, err, "failed to parse X509 certificate")

// Check that each certificate contains SAN entries for the right DNS names.
sort.Strings(cert.DNSNames)
sort.Strings(wantedNames[s.Name])
assert.Equal(t, cert.DNSNames, wantedNames[s.Name])

}
}

Expand All @@ -206,9 +200,7 @@ func TestInvalidNamespaceAndName(t *testing.T) {
Lifetime: conf.Lifetime,
Namespace: conf.Namespace,
})
if err != nil {
t.Fatalf("failed to generate certificates: %s", err)
}
require.NoError(t, err, "failed to generate certificates")

secrets, errs := certgen.AsSecrets(conf.Namespace, conf.NameSuffix, certificates)
if len(errs) != 2 {
Expand Down Expand Up @@ -263,32 +255,32 @@ func TestOutputFileMode(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
outputDir, err := os.MkdirTemp("", "")
assert.NoError(t, err)
require.NoError(t, err)
defer os.RemoveAll(outputDir)
tc.cc.OutputDir = outputDir

// Write a file with insecure mode to ensure overwrite works as expected.
if tc.cc.Overwrite {
_, err = os.Create(filepath.Join(outputDir, tc.insecureFile))
assert.NoError(t, err)
require.NoError(t, err)
}

generatedCerts, err := certs.GenerateCerts(
&certs.Configuration{
Lifetime: tc.cc.Lifetime,
Namespace: tc.cc.Namespace,
})
assert.NoError(t, err)
require.NoError(t, err)

assert.NoError(t, OutputCerts(tc.cc, nil, generatedCerts))
require.NoError(t, OutputCerts(tc.cc, nil, generatedCerts))

err = filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error {
if !info.IsDir() {
assert.Equal(t, os.FileMode(0600), info.Mode(), "incorrect mode for file "+path)
}
return nil
})
assert.NoError(t, err)
require.NoError(t, err)
})
}
}
4 changes: 2 additions & 2 deletions internal/annotation/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,8 @@ func TestAnnotationKindValidation(t *testing.T) {
for key := range annotationsByKind[kind] {
t.Run(fmt.Sprintf("%s is known and valid for %s", key, kind),
func(t *testing.T) {
assert.Equal(t, true, IsKnown(key))
assert.Equal(t, true, ValidForKind(kind, key))
assert.True(t, IsKnown(key))
assert.True(t, ValidForKind(kind, key))
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/contourconfig/contourconfiguration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func TestParseTimeoutPolicy(t *testing.T) {
require.Error(t, err, "expected error to be returned")
require.Contains(t, err.Error(), tc.errorMsg)
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.expected, parsed)
}
})
Expand Down
13 changes: 7 additions & 6 deletions internal/dag/accessors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/projectcontour/contour/internal/ref"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -254,11 +255,11 @@ func TestGetSingleListener(t *testing.T) {

got, gotErr := d.GetSingleListener("http")
assert.Equal(t, d.Listeners["http"], got)
assert.NoError(t, gotErr)
require.NoError(t, gotErr)

got, gotErr = d.GetSingleListener("https")
assert.Equal(t, d.Listeners["https"], got)
assert.NoError(t, gotErr)
require.NoError(t, gotErr)
})

t.Run("one HTTP listener, no HTTPS listener", func(t *testing.T) {
Expand All @@ -273,11 +274,11 @@ func TestGetSingleListener(t *testing.T) {

got, gotErr := d.GetSingleListener("http")
assert.Equal(t, d.Listeners["http"], got)
assert.NoError(t, gotErr)
require.NoError(t, gotErr)

got, gotErr = d.GetSingleListener("https")
assert.Nil(t, got)
assert.EqualError(t, gotErr, "no HTTPS listener configured")
require.EqualError(t, gotErr, "no HTTPS listener configured")
})

t.Run("many HTTP listeners, one HTTPS listener", func(t *testing.T) {
Expand All @@ -304,11 +305,11 @@ func TestGetSingleListener(t *testing.T) {

got, gotErr := d.GetSingleListener("http")
assert.Nil(t, got)
assert.EqualError(t, gotErr, "more than one HTTP listener configured")
require.EqualError(t, gotErr, "more than one HTTP listener configured")

got, gotErr = d.GetSingleListener("https")
assert.Equal(t, d.Listeners["https-1"], got)
assert.NoError(t, gotErr)
require.NoError(t, gotErr)
})
}

Expand Down
8 changes: 4 additions & 4 deletions internal/dag/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1835,9 +1835,9 @@ func TestLookupService(t *testing.T) {
switch {
case tc.wantErr != nil:
require.Error(t, gotErr)
assert.EqualError(t, tc.wantErr, gotErr.Error())
require.EqualError(t, tc.wantErr, gotErr.Error())
default:
assert.Nil(t, gotErr)
require.NoError(t, gotErr)
assert.Equal(t, tc.wantSvc, gotSvc)
assert.Equal(t, tc.wantPort, gotPort)
}
Expand Down Expand Up @@ -2732,9 +2732,9 @@ func TestLookupUpstreamValidation(t *testing.T) {
switch {
case tc.wantErr != nil:
require.Error(t, gotErr)
assert.EqualError(t, tc.wantErr, gotErr.Error())
require.EqualError(t, tc.wantErr, gotErr.Error())
default:
assert.Nil(t, gotErr)
require.NoError(t, gotErr)
assert.Equal(t, tc.wantPvc, gotPvc)
}
})
Expand Down
9 changes: 5 additions & 4 deletions internal/dag/conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestPathMatchCondition(t *testing.T) {
Expand Down Expand Up @@ -741,11 +742,11 @@ func TestValidateHeaderMatchConditions(t *testing.T) {
gotErr := headerMatchConditionsValid(tc.matchconditions)

if !tc.wantErr {
assert.NoError(t, gotErr)
require.NoError(t, gotErr)
}

if tc.wantErr {
assert.Error(t, gotErr)
require.Error(t, gotErr)
}
})
}
Expand Down Expand Up @@ -871,11 +872,11 @@ func TestValidateQueryParameterMatchConditions(t *testing.T) {
gotErr := queryParameterMatchConditionsValid(tc.matchconditions)

if !tc.wantErr {
assert.NoError(t, gotErr)
require.NoError(t, gotErr)
}

if tc.wantErr {
assert.Error(t, gotErr)
require.Error(t, gotErr)
}
})
}
Expand Down
17 changes: 9 additions & 8 deletions internal/dag/dag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
)
Expand Down Expand Up @@ -89,12 +90,12 @@ func TestPeerValidationContext(t *testing.T) {
pvc2 := PeerValidationContext{}
var pvc3 *PeerValidationContext

assert.Equal(t, pvc1.GetSubjectNames()[0], "subject")
assert.Equal(t, pvc1.GetCACertificate(), []byte("cacert"))
assert.Equal(t, pvc2.GetSubjectNames(), []string(nil))
assert.Equal(t, pvc2.GetCACertificate(), []byte(nil))
assert.Equal(t, pvc3.GetSubjectNames(), []string(nil))
assert.Equal(t, pvc3.GetCACertificate(), []byte(nil))
assert.Equal(t, "subject", pvc1.GetSubjectNames()[0])
assert.Equal(t, []byte("cacert"), pvc1.GetCACertificate())
assert.Equal(t, []string(nil), pvc2.GetSubjectNames())
assert.Equal(t, []byte(nil), pvc2.GetCACertificate())
assert.Equal(t, []string(nil), pvc3.GetSubjectNames())
assert.Equal(t, []byte(nil), pvc3.GetCACertificate())
}

func TestObserverFunc(t *testing.T) {
Expand All @@ -104,7 +105,7 @@ func TestObserverFunc(t *testing.T) {
// Ensure the given function gets called.
result := false
ObserverFunc(func(*DAG) { result = true }).OnChange(nil)
assert.Equal(t, true, result)
require.True(t, result)
}

func TestServiceClusterValid(t *testing.T) {
Expand All @@ -117,7 +118,7 @@ func TestServiceClusterValid(t *testing.T) {
}

for _, c := range invalid {
assert.Errorf(t, c.Validate(), "invalid cluster %#v", c)
require.Errorf(t, c.Validate(), "invalid cluster %#v", c)
}
}

Expand Down
Loading
Loading