Enable testifylint linter and fix various issues

.golangci.yaml

@@ -35,6 +35,7 @@ linters:
     - unused
     - usestdlibvars
     - whitespace
+    - testifylint
   # TODO(vincepri): Figure out if we want to enable or remove the following linters:
   # - predeclared
   # - goconst

pkg/reconciler/topology/partitionset/partitioning_test.go

@@ -118,19 +118,19 @@ func TestPartition(t *testing.T) {
 	}
 
 	matchLabelsMap := partition(shards, []string{}, nil)
-	require.Equal(t, 0, len(matchLabelsMap), "No label selector expected when no dimension is provided, got: %v", matchLabelsMap)
+	require.Empty(t, matchLabelsMap, "No label selector expected when no dimension is provided, got: %v", matchLabelsMap)
 
 	matchLabelsMap = partition(shards, []string{"doesnotexist"}, nil)
-	require.Equal(t, 0, len(matchLabelsMap), "No label selector expected when no shard with the dimension, got: %v", matchLabelsMap)
+	require.Empty(t, matchLabelsMap, "No label selector expected when no shard with the dimension, got: %v", matchLabelsMap)
 
 	matchLabelsMap = partition(shards, []string{"region"}, nil)
-	require.Equal(t, 2, len(matchLabelsMap), "2 label selectors for region: Europe and Asia expected, got: %v", matchLabelsMap)
+	require.Len(t, matchLabelsMap, 2, "2 label selectors for region: Europe and Asia expected, got: %v", matchLabelsMap)
 
 	matchLabelsMap = partition(shards, []string{"region", "cloud"}, nil)
-	require.Equal(t, 3, len(matchLabelsMap), "3 label selectors for: Asia/Azure, Europe/AWS and Europe/Azure expected, got: %v", matchLabelsMap)
+	require.Len(t, matchLabelsMap, 3, "3 label selectors for: Asia/Azure, Europe/AWS and Europe/Azure expected, got: %v", matchLabelsMap)
 
 	matchLabelsMap = partition(shards, []string{"region", "cloud"}, map[string]string{"environment": "prod"})
-	require.Equal(t, 3, len(matchLabelsMap), "3 label selectors for: Asia/Azure, Europe/AWS, Europe/Azure expected, got: %v", matchLabelsMap)
+	require.Len(t, matchLabelsMap, 3, "3 label selectors for: Asia/Azure, Europe/AWS, Europe/Azure expected, got: %v", matchLabelsMap)
 	for _, v := range matchLabelsMap {
 		require.Equal(t, "prod", v["environment"], "Expected that all partitions have a label selector for environment = prod")
 	}

pkg/server/apiextensions_test.go

@@ -32,7 +32,7 @@ import (
 )
 
 func TestSystemCRDsLogicalClusterName(t *testing.T) {
-	require.Equal(t, SystemCRDClusterName.String(), reservedcrdgroups.SystemCRDLogicalClusterName, "reservedcrdgroups admission check should match SystemCRDLogicalCluster")
+	require.Equal(t, reservedcrdgroups.SystemCRDLogicalClusterName, SystemCRDClusterName.String(), "reservedcrdgroups admission check should match SystemCRDLogicalCluster")
 }
 
 func TestDecorateCRDWithBinding(t *testing.T) {

pkg/virtual/apiexport/authorizer/binding_test.go

@@ -712,7 +712,7 @@ func TestBoundAPIAuthorizer(t *testing.T) {
 			if err != nil {
 				errString = err.Error()
 			}
-			require.Equal(t, errString, tc.expectedErr)
+			require.Equal(t, tc.expectedErr, errString)
 			require.Equal(t, tc.expectedDecision, dec)
 			require.Equal(t, tc.expectedReason, reason)
 		})

pkg/virtual/apiexport/authorizer/maximal_permission_policy_test.go

@@ -326,7 +326,7 @@ func TestMaximalPermissionPolicyAuthorizer(t *testing.T) {
 			if err != nil {
 				errString = err.Error()
 			}
-			require.Equal(t, errString, tc.expectedErr)
+			require.Equal(t, tc.expectedErr, errString)
 			require.Equal(t, tc.expectedDecision, dec)
 			require.Equal(t, tc.expectedReason, reason)
 		})

pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_controller_test.go

@@ -67,7 +67,7 @@ func TestEnqueueAPIResourceSchema(t *testing.T) {
 	logger, _ := ktesting.NewTestContext(t)
 	c.enqueueAPIResourceSchema(schema, logger)
 
-	require.Equal(t, c.queue.Len(), 2)
+	require.Equal(t, 2, c.queue.Len())
 
 	// get the queue keys
 	actual := sets.New[string]()

pkg/virtual/apiexport/schemas/builtin/builtin_test.go

@@ -49,5 +49,5 @@ func TestInit(t *testing.T) {
 		require.Truef(t, versionFound, "could not find version %s in API resource schema %s", api.GroupVersion.String(), schema.Name)
 	}
 
-	require.Equal(t, len(builtInAPIResourceSchemas), len(visitedResourceSchemas))
+	require.Len(t, visitedResourceSchemas, len(builtInAPIResourceSchemas))
 }

test/e2e/apibinding/apibinding_deletion_test.go

@@ -220,7 +220,7 @@ func TestAPIBindingDeletion(t *testing.T) {
 		},
 	}
 	_, err = cowboyClient.Create(t.Context(), cowboyDenied, metav1.CreateOptions{})
-	require.Equal(t, apierrors.IsForbidden(err), true)
+	require.True(t, apierrors.IsForbidden(err))
 
 	t.Logf("Clean finalizer to remove the cowboy")
 	err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {

test/e2e/apibinding/apibinding_test.go

@@ -331,7 +331,7 @@ func TestAPIBinding(t *testing.T) {
 		cowboyClient := wildwestClusterClient.Cluster(consumerWorkspace).WildwestV1alpha1().Cowboys("default")
 		cowboys, err := cowboyClient.List(t.Context(), metav1.ListOptions{})
 		require.NoError(t, err, "error listing cowboys inside %q", consumerWorkspace)
-		require.Zero(t, len(cowboys.Items), "expected 0 cowboys inside %q", consumerWorkspace)
+		require.Empty(t, cowboys.Items, "expected 0 cowboys inside %q", consumerWorkspace)
 
 		t.Logf("Create a cowboy CR in consumer workspace %q", consumerWorkspace)
 		cowboyName := fmt.Sprintf("cowboy-%s", consumerWorkspace.Base())
@@ -347,7 +347,7 @@ func TestAPIBinding(t *testing.T) {
 		t.Logf("Make sure there is 1 cowboy in consumer workspace %q", consumerWorkspace)
 		cowboys, err = cowboyClient.List(t.Context(), metav1.ListOptions{})
 		require.NoError(t, err, "error listing cowboys in %q", consumerWorkspace)
-		require.Equal(t, 1, len(cowboys.Items), "expected 1 cowboy in %q", consumerWorkspace)
+		require.Len(t, cowboys.Items, 1, "expected 1 cowboy in %q", consumerWorkspace)
 		require.Equal(t, cowboyName, cowboys.Items[0].Name, "unexpected name for cowboy in %q", consumerWorkspace)
 
 		t.Logf("Create an APIBinding in consumer workspace %q that points to the today-cowboys export from serviceProvider2 (which should conflict)", consumerWorkspace)
@@ -479,13 +479,13 @@ func TestAPIBinding(t *testing.T) {
 				listErrs = append(listErrs, err)
 				continue
 			}
-			require.Equal(t, 1, len(list.Items), "unexpected # of cowboys through virtual workspace with explicit workspace")
+			require.Len(t, list.Items, 1, "unexpected # of cowboys through virtual workspace with explicit workspace")
 			foundOnShards++
 
 			t.Logf("Listing %s|%s cowboys via virtual workspace wildcard list", provider2Path, exportName)
 			list, err = vw2ClusterClient.Resource(gvr).List(t.Context(), metav1.ListOptions{})
 			require.NoError(t, err, "error listing through virtual workspace wildcard")
-			require.Equal(t, 1, len(list.Items), "unexpected # of cowboys through virtual workspace with wildcard")
+			require.Len(t, list.Items, 1, "unexpected # of cowboys through virtual workspace with wildcard")
 		}
 
 		if foundOnShards == 0 {

test/e2e/apibinding/maximalpermissionpolicy_authorizer_test.go

@@ -236,7 +236,7 @@ func TestMaximalPermissionPolicyAuthorizer(t *testing.T) {
 		t.Logf("Make sure there is 1 cowboy in consumer workspace %q", consumer)
 		cowboys, err := cowboyclient.List(t.Context(), metav1.ListOptions{})
 		require.NoError(t, err, "error listing cowboys in consumer workspace %q", consumer)
-		require.Equal(t, 1, len(cowboys.Items), "expected 1 cowboy in consumer workspace %q", consumer)
+		require.Len(t, cowboys.Items, 1, "expected 1 cowboy in consumer workspace %q", consumer)
 		if serviceProvider == rbacServiceProviderPath {
 			t.Logf("Make sure that the status of cowboy can not be updated in workspace %q", consumer)
 			kcptestinghelpers.Eventually(t, func() (bool, string) {
@@ -294,7 +294,7 @@ func TestMaximalPermissionPolicyAuthorizer(t *testing.T) {
 			t.Logf("User 2 can list cowboys in consumer workspace %q before deleting APIExport", consumer)
 			user2Cowboys, err := user2Client.Cluster(consumer).WildwestV1alpha1().Cowboys("default").List(t.Context(), metav1.ListOptions{})
 			require.NoError(t, err)
-			require.Equal(t, 3, len(user2Cowboys.Items), "expected 3 cowboys in consumer")
+			require.Len(t, user2Cowboys.Items, 3, "expected 3 cowboys in consumer")
 
 			t.Logf("User 2 gets errors trying to delete an existing cowboy in consumer workspace %q", consumer)
 			err = user2Client.Cluster(consumer).WildwestV1alpha1().Cowboys(cowboy2.ObjectMeta.Namespace).Delete(t.Context(), cowboy2.ObjectMeta.Name, metav1.DeleteOptions{})
@@ -328,7 +328,7 @@ func TestMaximalPermissionPolicyAuthorizer(t *testing.T) {
 			t.Logf("Admin can list the cowboys in consumer workspace %q", consumer)
 			cowboysAfterDelete, err := wildwestClusterClient.Cluster(consumer).WildwestV1alpha1().Cowboys("default").List(t.Context(), metav1.ListOptions{})
 			require.NoError(t, err, "error listing cowboys in consumer workspace %q", consumer)
-			require.Equal(t, 3, len(cowboysAfterDelete.Items), "expected 3 cowboy in consumer")
+			require.Len(t, cowboysAfterDelete.Items, 3, "expected 3 cowboy in consumer")
 
 			t.Logf("Admin can delete an existing cowboy in consumer workspace %q", consumer)
 			err = wildwestClusterClient.Cluster(consumer).WildwestV1alpha1().Cowboys(cowboy2.ObjectMeta.Namespace).Delete(t.Context(), cowboy2.ObjectMeta.Name, metav1.DeleteOptions{})
@@ -490,7 +490,7 @@ func testCRUDOperations(t *testing.T, consumer1Workspace logicalcluster.Path, wi
 		cowboys, err = cowboyClient.List(t.Context(), metav1.ListOptions{})
 		return err == nil
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected to be able to list ")
-	require.Zero(t, len(cowboys.Items), "expected 0 cowboys inside consumer workspace %q", consumer1Workspace)
+	require.Empty(t, cowboys.Items, "expected 0 cowboys inside consumer workspace %q", consumer1Workspace)
 
 	t.Logf("Create a cowboy CR in consumer workspace %q", consumer1Workspace)
 	cowboyName := fmt.Sprintf("cowboy-%s", consumer1Workspace.Base())

test/e2e/authorizer/serviceaccounts_test.go

@@ -204,7 +204,7 @@ func TestServiceAccounts(t *testing.T) {
 
 				t.Log("Accessing workspace with the service account")
 				obj, err := saKubeClusterClient.Cluster(otherPath).CoreV1().ConfigMaps(namespace.Name).List(ctx, metav1.ListOptions{})
-				require.Error(t, err, fmt.Sprintf("expected error accessing workspace with the service account, got: %v", obj))
+				require.Error(t, err, "expected error accessing workspace with the service account, got: %v", obj)
 
 				t.Log("Giving the access to configmaps in the other workspace")
 				_, err = kubeClusterClient.Cluster(otherPath).RbacV1().ClusterRoles().Create(ctx, &rbacv1.ClusterRole{
@@ -295,7 +295,7 @@ func TestServiceAccounts(t *testing.T) {
 
 				t.Log("Accessing workspace with the service account")
 				obj, err := saKubeClusterClient.Cluster(otherPath).CoreV1().ConfigMaps(namespace.Name).List(ctx, metav1.ListOptions{})
-				require.Error(t, err, fmt.Sprintf("expected error accessing workspace with the service account, got: %v", obj))
+				require.Error(t, err, "expected error accessing workspace with the service account, got: %v", obj)
 			})
 
 			t.Run("A service account is allowed to escalate permissions implicitly", func(t *testing.T) {

test/e2e/reconciler/apiexportendpointslice/apiexportendpointslice_test.go

@@ -107,7 +107,7 @@ func TestAPIExportEndpointSliceWithPartition(t *testing.T) {
 	require.True(t, apierrors.IsForbidden(err), "no error creating APIExportEndpointSlice (admission should have declined it)")
 	sliceList, err := sliceClient.Cluster(partitionClusterPath).List(ctx, metav1.ListOptions{})
 	require.NoError(t, err, "error listing APIExportEndpointSlice")
-	require.True(t, len(sliceList.Items) == 0, "not expecting any APIExportEndpointSlice")
+	require.Empty(t, sliceList.Items, "not expecting any APIExportEndpointSlice")
 
 	t.Logf("Creating the missing APIExport")
 	exportClient := kcpClusterClient.ApisV1alpha2().APIExports()
@@ -155,7 +155,7 @@ func TestAPIExportEndpointSliceWithPartition(t *testing.T) {
 
 		return false, spew.Sdump(slice.Status.Conditions)
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected missing Partition")
-	require.True(t, len(slice.Status.APIExportEndpoints) == 0, "not expecting any endpoint")
+	require.Empty(t, slice.Status.APIExportEndpoints, "not expecting any endpoint")
 	t.Logf("Creating the missing Partition")
 	partitionClient := kcpClusterClient.TopologyV1alpha1().Partitions()
 	_, err = partitionClient.Cluster(partitionClusterPath).Create(ctx, partition, metav1.CreateOptions{})
@@ -172,7 +172,7 @@ func TestAPIExportEndpointSliceWithPartition(t *testing.T) {
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected valid Partition")
 
 	t.Logf("Checking that no endpoint has been populated")
-	require.True(t, len(slice.Status.APIExportEndpoints) == 0, "not expecting any endpoint")
+	require.Empty(t, slice.Status.APIExportEndpoints, "not expecting any endpoint")
 }
 
 func TestAPIBindingEndpointSlicesSharded(t *testing.T) {

test/e2e/reconciler/cache/helpers.go

@@ -28,6 +28,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -85,7 +86,7 @@ func StartStandaloneCacheServer(ctx context.Context, t *testing.T, dataDir strin
 	start := time.Now()
 	t.Logf("Starting the cache server")
 	go func() {
-		require.NoError(t, preparedCachedServer.Run(ctx))
+		assert.NoError(t, preparedCachedServer.Run(ctx))
 	}()
 
 	cacheServerCertificatePath := path.Join(dataDir, "cache", "apiserver.crt")

test/e2e/reconciler/partitionset/partitionset_test.go

@@ -89,7 +89,7 @@ func TestPartitionSet(t *testing.T) {
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected valid partitionSet")
 	partitions, err = partitionClient.Cluster(partitionClusterPath).List(ctx, metav1.ListOptions{})
 	require.NoError(t, err, "error retrieving partitions")
-	require.Equal(t, 0, len(partitions.Items), "no partition expected, got: %d", len(partitions.Items))
+	require.Empty(t, partitions.Items, "no partition expected, got: %d", len(partitions.Items))
 
 	// Newly added shards are annotated to avoid side effects on other e2e tests.
 	t.Logf("Creating a shard matching the partitionSet")
@@ -395,6 +395,6 @@ func TestPartitionSetAdmission(t *testing.T) {
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected 1 partition")
 	expectedName := partitionSet.Name + "-" + strings.Join(labelValues, "-")
 	expectedName = expectedName[:validation.DNS1123LabelMaxLength-5]
-	require.EqualValues(t, expectedName, partitions.Items[0].Name[:len(partitions.Items[0].Name)-5],
+	require.Equal(t, expectedName, partitions.Items[0].Name[:len(partitions.Items[0].Name)-5],
 		"partition name not as expected, got: %s", partitions.Items[0].Name)
 }

test/e2e/reconciler/workspacedeletion/controller_test.go

@@ -184,11 +184,11 @@ func TestWorkspaceDeletion(t *testing.T) {
 
 				nslist, err := rootShardKubeClusterClient.Cluster(workspaceCluster).CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
 				require.NoError(t, err, "failed to list namespaces in workspace %s", workspace.Name)
-				require.Equal(t, 0, len(nslist.Items))
+				require.Empty(t, nslist.Items)
 
 				cmlist, err := rootShardKubeClusterClient.Cluster(workspaceCluster).CoreV1().ConfigMaps(metav1.NamespaceAll).List(ctx, metav1.ListOptions{})
 				require.NoError(t, err, "failed to list configmaps in workspace %s", workspace.Name)
-				require.Equal(t, 0, len(cmlist.Items))
+				require.Empty(t, cmlist.Items)
 			},
 		},
 		{

test/e2e/server/url_test.go

@@ -95,7 +95,7 @@ func TestURLs(t *testing.T) {
 			if !assert.Equal(t, testPath.expectedStatusCode, resp.StatusCode) {
 				t.Logf("Expected status code %d, got %d", testPath.expectedStatusCode, resp.StatusCode)
 				b, err := io.ReadAll(resp.Body)
-				assert.NoError(t, err)
+				assert.NoError(t, err) //nolint:testifylint // using assert here to still get the response body as far as possible
 				t.Logf("Response body: %s", string(b))
 			}
 		})

test/e2e/virtual/apiexport/binding_test.go

@@ -347,7 +347,7 @@ func TestAPIBindingPermissionClaimsVerbs(t *testing.T) {
 	t.Logf("Make sure secrets list shows nothing to start")
 	secrets, err := kubeClusterClient.Cluster(consumerClusterName.Path()).CoreV1().Secrets("default").List(t.Context(), metav1.ListOptions{})
 	require.NoError(t, err, "error listing secrets inside %q", consumerPath)
-	require.Zero(t, len(secrets.Items), "expected 0 secrets inside %q", consumerPath)
+	require.Empty(t, secrets.Items, "expected 0 secrets inside %q", consumerPath)
 
 	t.Logf("Create a secret in consumer workspace %q before allowing create", consumerPath)
 	secretName := fmt.Sprintf("secret-%s", consumerPath.Base())

test/e2e/virtual/apiexport/virtualworkspace_test.go

@@ -119,7 +119,7 @@ func TestAPIExportVirtualWorkspace(t *testing.T) {
 	require.NoError(t, err)
 	cowboysProjected, err := wildwestVCClusterClient.WildwestV1alpha1().Cowboys().List(t.Context(), metav1.ListOptions{})
 	require.NoError(t, err)
-	require.Equal(t, 1, len(cowboysProjected.Items))
+	require.Len(t, cowboysProjected.Items, 1)
 
 	t.Logf("Verify that the virtual workspace includes apibindings")
 	discoveryVCClusterClient, err := kcpdiscovery.NewForConfig(apiExportVWCfg)
@@ -310,7 +310,7 @@ func TestAPIExportVirtualWorkspace(t *testing.T) {
 		t.Logf("Verify that deleted cowboy is gone")
 		cowboys, err := wwUser1VC.WildwestV1alpha1().Cowboys().List(t.Context(), metav1.ListOptions{})
 		require.NoError(t, err)
-		require.Equal(t, 2, len(cowboys.Items))
+		require.Len(t, cowboys.Items, 2)
 		names := make([]string, 0, len(cowboys.Items))
 		for _, c := range cowboys.Items {
 			names = append(names, c.Name)
@@ -324,7 +324,7 @@ func TestAPIExportVirtualWorkspace(t *testing.T) {
 		t.Logf("Verify that all cowboys are gone")
 		cowboys, err = wwUser1VC.WildwestV1alpha1().Cowboys().List(t.Context(), metav1.ListOptions{})
 		require.NoError(t, err)
-		require.Equal(t, 0, len(cowboys.Items))
+		require.Empty(t, cowboys.Items)
 	}
 }
 
@@ -636,7 +636,7 @@ func TestAPIExportPermissionClaims(t *testing.T) {
 	require.NoError(t, err)
 	cowboys, err := wildwestVCClients.WildwestV1alpha1().Cowboys().List(t.Context(), metav1.ListOptions{})
 	require.NoError(t, err)
-	require.Equal(t, 1, len(cowboys.Items))
+	require.Len(t, cowboys.Items, 1)
 
 	t.Logf("Verify that we get empty lists for all claimed resources (other than apibindings) because the claims have not been accepted yet")
 	dynamicVWClusterClient, err := kcpdynamic.NewForConfig(consumer1VWCfg)
@@ -666,7 +666,7 @@ func TestAPIExportPermissionClaims(t *testing.T) {
 		require.NoError(t, err, "error listing %q", gvr)
 		if gvr == apisv1alpha2.SchemeGroupVersion.WithResource("apibindings") {
 			// for this one we always see the reflexive objects
-			require.Equal(t, 1, len(list.Items), "expected to find 1 apibinding, got %#v", list.Items)
+			require.Len(t, list.Items, 1, "expected to find 1 apibinding, got %#v", list.Items)
 			for _, binding := range list.Items {
 				require.Equal(t, "cowboys", binding.GetName(), "expected binding name to be \"cowboys\"")
 			}
@@ -1107,7 +1107,7 @@ func createCowboyInConsumer(t *testing.T, consumer1Workspace logicalcluster.Path
 		cowboys, err = cowboyClusterClient.List(t.Context(), metav1.ListOptions{})
 		return err == nil
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected to be able to list ")
-	require.Zero(t, len(cowboys.Items), "expected 0 cowboys inside consumer workspace %q", consumer1Workspace)
+	require.Empty(t, cowboys.Items, "expected 0 cowboys inside consumer workspace %q", consumer1Workspace)
 
 	t.Logf("Create a cowboy CR in consumer workspace %q", consumer1Workspace)
 	cowboyName := fmt.Sprintf("cowboy-%s", consumer1Workspace.Base())

test/e2e/virtual/initializingworkspaces/virtualworkspace_test.go

@@ -453,7 +453,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) {
 				if logicalcluster.From(evt.Object.(metav1.Object)).String() != ws.Spec.Cluster {
 					continue
 				}
-				require.Equal(t, evt.Type, watch.Added)
+				require.Equal(t, watch.Added, evt.Type)
 			case <-time.Tick(wait.ForeverTestTimeout):
 				t.Fatalf("never saw a watche event for the %s initializer", initializer)
 			}
@@ -555,7 +555,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) {
 				if logicalcluster.From(evt.Object.(metav1.Object)).String() != ws.Spec.Cluster {
 					continue
 				}
-				require.Equal(t, evt.Type, watch.Deleted)
+				require.Equal(t, watch.Deleted, evt.Type)
 			case <-time.Tick(wait.ForeverTestTimeout):
 				t.Fatalf("never saw a watch event for the %s initializer", initializer)
 			}