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

✨ add testify linter and address fixes #1334

Merged
merged 1 commit into from
Oct 4, 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
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ linters:
- nonamedreturns
- prealloc
- stylecheck
- testifylint
- tparallel
- unconvert
- unparam
Expand Down
5 changes: 3 additions & 2 deletions internal/action/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/release"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -130,12 +131,12 @@ func TestActionClientFor(t *testing.T) {

// Test the successful case
actionClient, err := acg.ActionClientFor(ctx, obj)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, actionClient)
assert.IsType(t, &ActionClient{}, actionClient)

// Test the error case
actionClient, err = acg.ActionClientFor(ctx, obj)
assert.Error(t, err)
require.Error(t, err)
assert.Nil(t, actionClient)
}
18 changes: 9 additions & 9 deletions internal/catalogmetadata/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,26 +79,26 @@ func TestFilesystemCachePutAndGet(t *testing.T) {

t.Log("Get empty v1 cache")
actualFSGet, err := c.Get(catalogName, resolvedRef1)
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, actualFSGet)

t.Log("Put v1 content into cache")
actualFSPut, err := c.Put(catalogName, resolvedRef1, defaultContent(), nil)
assert.NoError(t, err)
require.NoError(t, err)
require.NotNil(t, actualFSPut)
assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut))
require.NoError(t, equalFilesystems(defaultFS(), actualFSPut))

t.Log("Get v1 content from cache")
actualFSGet, err = c.Get(catalogName, resolvedRef1)
assert.NoError(t, err)
require.NoError(t, err)
require.NotNil(t, actualFSGet)
assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut))
assert.NoError(t, equalFilesystems(actualFSPut, actualFSGet))

t.Log("Put v1 error into cache")
actualFSPut, err = c.Put(catalogName, resolvedRef1, nil, errors.New("fake put error"))
// Errors do not override previously successfully populated cache
assert.NoError(t, err)
require.NoError(t, err)
require.NotNil(t, actualFSPut)
assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut))
assert.NoError(t, equalFilesystems(actualFSPut, actualFSGet))
Expand All @@ -115,13 +115,13 @@ func TestFilesystemCachePutAndGet(t *testing.T) {

t.Log("Put v2 content into cache")
actualFSPut, err = c.Put(catalogName, resolvedRef2, defaultContent(), nil)
assert.NoError(t, err)
require.NoError(t, err)
require.NotNil(t, actualFSPut)
assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut))
require.NoError(t, equalFilesystems(defaultFS(), actualFSPut))

t.Log("Get v2 content from cache")
actualFSGet, err = c.Get(catalogName, resolvedRef2)
assert.NoError(t, err)
require.NoError(t, err)
require.NotNil(t, actualFSGet)
assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut))
assert.NoError(t, equalFilesystems(actualFSPut, actualFSGet))
Expand All @@ -130,7 +130,7 @@ func TestFilesystemCachePutAndGet(t *testing.T) {
// Cache should be empty and no error because
// Put with a new version overrides the old version
actualFSGet, err = c.Get(catalogName, resolvedRef1)
assert.NoError(t, err)
require.NoError(t, err)
assert.Nil(t, actualFSGet)
}

Expand Down
11 changes: 6 additions & 5 deletions internal/catalogmetadata/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"testing/fstest"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
Expand Down Expand Up @@ -77,7 +78,7 @@ func TestClientGetPackage(t *testing.T) {
pkgName: "pkg-missing",
cache: &fakeCache{getFS: testFS},
assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) {
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, &declcfg.DeclarativeConfig{}, fbc)
},
},
Expand All @@ -89,7 +90,7 @@ func TestClientGetPackage(t *testing.T) {
"invalid-pkg-present/olm.package/invalid-pkg-present.json": &fstest.MapFile{Data: []byte(`{"schema": "olm.package","name": 12345}`)},
}},
assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) {
assert.ErrorContains(t, err, `error loading package "invalid-pkg-present"`)
require.ErrorContains(t, err, `error loading package "invalid-pkg-present"`)
assert.Nil(t, fbc)
},
},
Expand All @@ -99,7 +100,7 @@ func TestClientGetPackage(t *testing.T) {
pkgName: "pkg-present",
cache: &fakeCache{getFS: testFS},
assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) {
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Schema: declcfg.SchemaPackage, Name: "pkg-present"}}}, fbc)
},
},
Expand All @@ -111,7 +112,7 @@ func TestClientGetPackage(t *testing.T) {
return testFS, nil
}},
assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) {
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Schema: declcfg.SchemaPackage, Name: "pkg-present"}}}, fbc)
},
},
Expand Down Expand Up @@ -170,7 +171,7 @@ func TestClientPopulateCache(t *testing.T) {
}, nil
},
assert: func(t *testing.T, fs fs.FS, err error) {
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, testFS, fs)
},
putFuncConstructor: func(t *testing.T) func(source string, errToCache error) (fs.FS, error) {
Expand Down
3 changes: 2 additions & 1 deletion internal/catalogmetadata/filter/bundle_predicates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

mmsemver "github.com/Masterminds/semver/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/operator-framework/operator-registry/alpha/declcfg"
"github.com/operator-framework/operator-registry/alpha/property"
Expand Down Expand Up @@ -40,7 +41,7 @@ func TestInMastermindsSemverRange(t *testing.T) {
}

vRange, err := mmsemver.NewConstraint(">=1.0.0")
assert.NoError(t, err)
require.NoError(t, err)

f := filter.InMastermindsSemverRange(vRange)

Expand Down
6 changes: 3 additions & 3 deletions internal/catalogmetadata/filter/successors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing.
} {
t.Run(tt.name, func(t *testing.T) {
successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName])
assert.NoError(t, err)
require.NoError(t, err)

allBundles := make([]declcfg.Bundle, 0, len(bundleSet))
for _, bundle := range bundleSet {
Expand Down Expand Up @@ -328,7 +328,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing
} {
t.Run(tt.name, func(t *testing.T) {
successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName])
assert.NoError(t, err)
require.NoError(t, err)

allBundles := make([]declcfg.Bundle, 0, len(bundleSet))
for _, bundle := range bundleSet {
Expand Down Expand Up @@ -380,7 +380,7 @@ func TestLegacySuccessor(t *testing.T) {
emptyBundle := declcfg.Bundle{}

f, err := legacySuccessor(installedBundle, fakeChannel)
assert.NoError(t, err)
require.NoError(t, err)

assert.True(t, f(b2))
assert.False(t, f(b3))
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/clusterextension_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {

isTerminal := errors.Is(err, reconcile.TerminalError(nil))
assert.Equal(t, tc.expectTerminal, isTerminal, "expected terminal error: %v, got: %v", tc.expectTerminal, isTerminal)
assert.ErrorContains(t, err, tc.unpackErr.Error())
require.ErrorContains(t, err, tc.unpackErr.Error())

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
Expand Down
8 changes: 4 additions & 4 deletions internal/resolve/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) {
gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil)
require.Error(t, err)
// We will not make a decision on which catalog to use
assert.ErrorContains(t, err, "in multiple catalogs with the same priority [b c]")
require.ErrorContains(t, err, "in multiple catalogs with the same priority [b c]")
assert.Nil(t, gotBundle)
assert.Nil(t, gotVersion)
assert.Nil(t, gotDeprecation)
Expand All @@ -408,7 +408,7 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) {
gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil)
require.Error(t, err)
// We will not make a decision on which catalog to use
assert.ErrorContains(t, err, "in multiple catalogs with the same priority [d f]")
require.ErrorContains(t, err, "in multiple catalogs with the same priority [d f]")
assert.Nil(t, gotBundle)
assert.Nil(t, gotVersion)
assert.Nil(t, gotDeprecation)
Expand Down Expand Up @@ -635,7 +635,7 @@ func TestCatalogWalker(t *testing.T) {
seenCatalogs = append(seenCatalogs, cat.Name)
return nil
}
assert.NoError(t, w(context.Background(), "", walkFunc))
require.NoError(t, w(context.Background(), "", walkFunc))
assert.Equal(t, []string{"a", "b"}, seenCatalogs)
})
}
Expand Down Expand Up @@ -936,7 +936,7 @@ func TestMultiplePriority(t *testing.T) {
ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.1", ocv1alpha1.UpgradeConstraintPolicyCatalogProvided)
gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil)
require.Error(t, err)
assert.ErrorContains(t, err, "in multiple catalogs with the same priority [a b c]")
require.ErrorContains(t, err, "in multiple catalogs with the same priority [a b c]")
assert.Nil(t, gotBundle)
assert.Nil(t, gotVersion)
assert.Nil(t, gotDeprecation)
Expand Down
20 changes: 10 additions & 10 deletions internal/rukpak/source/containers_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ func TestUnpackValidInsecure(t *testing.T) {
result, err := unpacker.Unpack(context.Background(), bundleSource)
require.NoError(t, err)
require.NotNil(t, result)
assert.Equal(t, result.State, source.StateUnpacked)
assert.Equal(t, source.StateUnpacked, result.State)

require.NoDirExists(t, oldBundlePath)

unpackedFile, err := fs.ReadFile(result.Bundle, testFileName)
assert.NoError(t, err)
require.NoError(t, err)
// Ensure the unpacked file matches the source content
assert.Equal(t, []byte(testFileContents), unpackedFile)
assert.NoError(t, unpacker.Cleanup(context.Background(), bundleSource))
Expand Down Expand Up @@ -87,9 +87,9 @@ func TestUnpackValidUsesCache(t *testing.T) {

// Attempt to pull and unpack the image
result, err := unpacker.Unpack(context.Background(), bundleSource)
assert.NoError(t, err)
require.NoError(t, err)
require.NotNil(t, result)
assert.Equal(t, result.State, source.StateUnpacked)
assert.Equal(t, source.StateUnpacked, result.State)

// Make sure the original contents of the cache are still present. If the cached contents
// were not used, we would expect the original contents to be removed.
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestUnpackNameOnlyImageReference(t *testing.T) {

// Attempt to pull and unpack the image
_, err := unpacker.Unpack(context.Background(), bundleSource)
assert.ErrorContains(t, err, "tag or digest is needed")
require.ErrorContains(t, err, "tag or digest is needed")
assert.ErrorIs(t, err, reconcile.TerminalError(nil))
}

Expand Down Expand Up @@ -226,8 +226,8 @@ func TestUnpackInvalidNilImage(t *testing.T) {
// Attempt to unpack
result, err := unpacker.Unpack(context.Background(), bundleSource)
assert.Nil(t, result)
assert.ErrorContains(t, err, "nil image source")
assert.ErrorIs(t, err, reconcile.TerminalError(nil))
require.ErrorContains(t, err, "nil image source")
require.ErrorIs(t, err, reconcile.TerminalError(nil))
assert.NoDirExists(t, filepath.Join(unpacker.BaseCachePath, bundleSource.Name))
}

Expand All @@ -245,8 +245,8 @@ func TestUnpackInvalidImageRef(t *testing.T) {
// Attempt to unpack
result, err := unpacker.Unpack(context.Background(), bundleSource)
assert.Nil(t, result)
assert.ErrorContains(t, err, "error parsing image reference")
assert.ErrorIs(t, err, reconcile.TerminalError(nil))
require.ErrorContains(t, err, "error parsing image reference")
require.ErrorIs(t, err, reconcile.TerminalError(nil))
assert.NoDirExists(t, filepath.Join(unpacker.BaseCachePath, bundleSource.Name))
}

Expand Down Expand Up @@ -322,7 +322,7 @@ func TestCleanup(t *testing.T) {

// Clean up the bundle
err := unpacker.Cleanup(context.Background(), bundleSource)
assert.NoError(t, err)
require.NoError(t, err)
assert.NoDirExists(t, bundleDir)
}

Expand Down
Loading