changes suggested by lint check (#1072)

* changes suggested by lint check

* revert cache namespaces, to be updated with runtime version

* use wrapper func where possible, update delta where not

* replace InDelta in metrics_test with toolchain-common wrapper

controllers/nstemplatetier/nstemplatetier_controller_test.go

@@ -143,7 +143,7 @@ func TestReconcile(t *testing.T) {
 				res, err := r.Reconcile(context.TODO(), req)
 				// then
 				require.Error(t, err)
-				assert.EqualError(t, err, "unable to get the current NSTemplateTier: mock error")
+				require.EqualError(t, err, "unable to get the current NSTemplateTier: mock error")
 				assert.Equal(t, reconcile.Result{}, res) // no explicit requeue
 			})
 		})

controllers/socialevent/socialevent_controller_test.go

@@ -153,7 +153,7 @@ func TestReconcileSocialEvent(t *testing.T) {
 
 			// then
 			require.Error(t, err)
-			assert.EqualError(t, err, "unable to get the 'notfound' NSTemplateTier: mock error")
+			require.EqualError(t, err, "unable to get the 'notfound' NSTemplateTier: mock error")
 			// check the social event status
 			socialeventtest.AssertThatSocialEvent(t, test.HostOperatorNs, event.Name, hostClient).
 				HasConditions(toolchainv1alpha1.Condition{

controllers/spacebindingrequest/spacebinding_spacebindingrequest_mapper_test.go

@@ -28,7 +28,7 @@ func TestMapSpaceBindingToSpaceBindingRequestByLabel(t *testing.T) {
 		requests := spacebindingrequest.MapSpaceBindingToSpaceBindingRequest()(sbnosbr)
 
 		// then
-		require.Len(t, requests, 0)
+		require.Empty(t, requests)
 	})
 
 	t.Run("should return spacebinding associated with spacebindingrequest", func(t *testing.T) {

controllers/spaceprovisionerconfig/mapper_test.go

@@ -105,10 +105,10 @@ func TestMapToolchainClusterToSpaceProvisionerConfigs(t *testing.T) {
 		})
 
 		// then
-		require.Equal(t, reqs, []reconcile.Request{
+		require.Equal(t, []reconcile.Request{
 			requestFromObject(spc0),
 			requestFromObject(spc2),
-		})
+		}, reqs)
 	})
 
 	t.Run("interprets errors as empty result", func(t *testing.T) {

controllers/spaceprovisionerconfig/spaceprovisionerconfig_controller_test.go

@@ -253,7 +253,7 @@ func TestSpaceProvisionerConfigReEnqueing(t *testing.T) {
 		require.NoError(t, cl.Get(context.TODO(), runtimeclient.ObjectKeyFromObject(spc), spcInCluster))
 
 		// then
-		assert.Error(t, reconcileErr)
+		require.Error(t, reconcileErr)
 		AssertThat(t, spcInCluster, Is(NotReadyWithReason(toolchainv1alpha1.SpaceProvisionerConfigToolchainClusterNotFoundReason)))
 		assert.Len(t, spcInCluster.Status.Conditions, 1)
 		assert.Equal(t, "failed to get the referenced ToolchainCluster: "+getErr.Error(), spcInCluster.Status.Conditions[0].Message)
@@ -285,7 +285,7 @@ func TestSpaceProvisionerConfigReEnqueing(t *testing.T) {
 		res, reconcileErr := r.Reconcile(context.TODO(), req)
 
 		// then
-		assert.NoError(t, reconcileErr)
+		require.NoError(t, reconcileErr)
 		assert.False(t, res.Requeue)
 		assert.Empty(t, spc.Status.Conditions)
 	})

controllers/spacerequest/space_spacerequest_mapper_test.go

@@ -27,7 +27,7 @@ func TestMapToSpaceRequestByLabel(t *testing.T) {
 		requests := spacerequest.MapSubSpaceToSpaceRequest()(spacenosr)
 
 		// then
-		require.Len(t, requests, 0)
+		require.Empty(t, requests)
 	})
 
 	t.Run("should return space associated with spacerequest", func(t *testing.T) {

controllers/spacerequest/spacerequest_controller_test.go

@@ -414,7 +414,7 @@ func TestCreateSpaceRequest(t *testing.T) {
 						toolchainv1alpha1.SpaceRequestProvisionedNamespaceLabelKey: "jane-env",
 					})
 					require.NoError(t, err)
-					require.Len(t, secrets.Items, 0)
+					require.Empty(t, secrets.Items)
 
 					// when
 					_, err := ctrl.Reconcile(context.TODO(), requestFor(sr))

controllers/toolchainconfig/configuration_test.go

@@ -3,6 +3,7 @@ package toolchainconfig
 import (
 	"context"
 	"fmt"
+	"github.com/stretchr/testify/require"
 	"testing"
 	"time"
 
@@ -28,7 +29,7 @@ func TestGetToolchainConfig(t *testing.T) {
 		toolchainCfg, err := GetToolchainConfig(cl)
 
 		// then
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, "prod", toolchainCfg.Environment())
 		assert.Empty(t, toolchainCfg.Notifications().MailgunAPIKey())
 	})
@@ -46,7 +47,7 @@ func TestGetToolchainConfig(t *testing.T) {
 		toolchainCfg, err := GetToolchainConfig(cl)
 
 		// then
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, "e2e-tests", toolchainCfg.Environment())
 		assert.Equal(t, "abc123", toolchainCfg.Notifications().MailgunAPIKey())
 		assert.Equal(t, "sandbox", toolchainCfg.Notifications().TemplateSetName())
@@ -55,22 +56,22 @@ func TestGetToolchainConfig(t *testing.T) {
 			// given
 			obj := testconfig.ModifyToolchainConfigObj(t, cl, testconfig.Environment("unit-tests"), testconfig.Notifications().TemplateSetName("appstudio"))
 			err := cl.Update(context.TODO(), obj)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			secret.Data["mailgunAPIKey"] = []byte("def456")
 			err = cl.Update(context.TODO(), secret)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			actual := &toolchainv1alpha1.ToolchainConfig{}
 			err = cl.Get(context.TODO(), types.NamespacedName{Name: "config", Namespace: test.HostOperatorNs}, actual)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, "unit-tests", *actual.Spec.Host.Environment)
 
 			// when
 			toolchainCfg, err = GetToolchainConfig(cl)
 
 			// then
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, "e2e-tests", toolchainCfg.Environment()) // returns cached value
 			assert.Equal(t, "abc123", toolchainCfg.Notifications().MailgunAPIKey())
 			assert.Equal(t, "sandbox", toolchainCfg.Notifications().TemplateSetName())
@@ -90,7 +91,7 @@ func TestGetToolchainConfig(t *testing.T) {
 		toolchainCfg, err := GetToolchainConfig(cl)
 
 		// then
-		assert.EqualError(t, err, "get failed")
+		require.EqualError(t, err, "get failed")
 		assert.Equal(t, "prod", toolchainCfg.Environment())
 		assert.Empty(t, toolchainCfg.Notifications().MailgunAPIKey())
 	})
@@ -136,7 +137,7 @@ func TestForceLoadToolchainConfig(t *testing.T) {
 		toolchainCfg, err := ForceLoadToolchainConfig(cl)
 
 		// then
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, "prod", toolchainCfg.Environment())
 		assert.Empty(t, toolchainCfg.Notifications().MailgunAPIKey())
 	})
@@ -154,30 +155,30 @@ func TestForceLoadToolchainConfig(t *testing.T) {
 		toolchainCfg, err := ForceLoadToolchainConfig(cl)
 
 		// then
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, "e2e-tests", toolchainCfg.Environment())
 		assert.Equal(t, "abc123", toolchainCfg.Notifications().MailgunAPIKey())
 
 		t.Run("config object updated but cache not updated", func(t *testing.T) {
 			// given
 			obj := testconfig.ModifyToolchainConfigObj(t, cl, testconfig.Environment("unit-tests"))
 			err := cl.Update(context.TODO(), obj)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			secret.Data["mailgunAPIKey"] = []byte("def456")
 			err = cl.Update(context.TODO(), secret)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 
 			actual := &toolchainv1alpha1.ToolchainConfig{}
 			err = cl.Get(context.TODO(), types.NamespacedName{Name: "config", Namespace: test.HostOperatorNs}, actual)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, "unit-tests", *actual.Spec.Host.Environment)
 
 			// when
 			toolchainCfg, err = ForceLoadToolchainConfig(cl)
 
 			// then
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, "unit-tests", toolchainCfg.Environment())               // returns actual value
 			assert.Equal(t, "def456", toolchainCfg.Notifications().MailgunAPIKey()) // returns actual value
 		})
@@ -196,7 +197,7 @@ func TestForceLoadToolchainConfig(t *testing.T) {
 		toolchainCfg, err := ForceLoadToolchainConfig(cl)
 
 		// then
-		assert.EqualError(t, err, "get failed")
+		require.EqualError(t, err, "get failed")
 		assert.Equal(t, "prod", toolchainCfg.Environment())
 		assert.Empty(t, toolchainCfg.Notifications().MailgunAPIKey())
 	})
@@ -281,7 +282,7 @@ func TestAutomaticApprovalConfig(t *testing.T) {
 		toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{})
 
 		assert.True(t, toolchainCfg.AutomaticApproval().IsEnabled())
-		assert.Equal(t, len(toolchainCfg.AutomaticApproval().Domains()), 0)
+		assert.Empty(t, toolchainCfg.AutomaticApproval().Domains())
 	})
 }
 
@@ -540,22 +541,22 @@ func TestPublicViewer(t *testing.T) {
 		cfg := commonconfig.NewToolchainConfigObjWithReset(t)
 		toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{})
 
-		assert.Equal(t, false, toolchainCfg.PublicViewer().Enabled())
+		assert.False(t, toolchainCfg.PublicViewer().Enabled())
 	})
 
 	t.Run("disabled", func(t *testing.T) {
 		cfg := commonconfig.NewToolchainConfigObjWithReset(t)
 		cfg.Spec.Host.PublicViewerConfig = &toolchainv1alpha1.PublicViewerConfiguration{Enabled: false}
 		toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{})
 
-		assert.Equal(t, false, toolchainCfg.PublicViewer().Enabled())
+		assert.False(t, toolchainCfg.PublicViewer().Enabled())
 	})
 
 	t.Run("enabled", func(t *testing.T) {
 		cfg := commonconfig.NewToolchainConfigObjWithReset(t)
 		cfg.Spec.Host.PublicViewerConfig = &toolchainv1alpha1.PublicViewerConfiguration{Enabled: true}
 		toolchainCfg := newToolchainConfig(cfg, map[string]map[string]string{})
 
-		assert.Equal(t, true, toolchainCfg.PublicViewer().Enabled())
+		assert.True(t, toolchainCfg.PublicViewer().Enabled())
 	})
 }

controllers/toolchainconfig/mapper_test.go

@@ -39,6 +39,6 @@ func TestSecretToToolchainConfigMapper(t *testing.T) {
 		req := MapSecretToToolchainConfig()(pod)
 
 		// then
-		require.Len(t, req, 0)
+		require.Empty(t, req)
 	})
 }

controllers/toolchainconfig/sync_test.go

@@ -82,7 +82,7 @@ func TestSyncMemberConfigs(t *testing.T) {
 
 			// then
 			require.Len(t, syncErrors, 1)
-			assert.Equal(t, syncErrors["member2"], "client error")
+			assert.Equal(t, "client error", syncErrors["member2"])
 		})
 
 		t.Run("sync to multiple members failed", func(t *testing.T) {
@@ -108,8 +108,8 @@ func TestSyncMemberConfigs(t *testing.T) {
 
 			// then
 			require.Len(t, syncErrors, 2)
-			assert.Equal(t, syncErrors["member1"], "client error")
-			assert.Equal(t, syncErrors["member2"], "client2 error")
+			assert.Equal(t, "client error", syncErrors["member1"])
+			assert.Equal(t, "client2 error", syncErrors["member2"])
 		})
 
 		t.Run("specific memberoperatorconfig exists but member cluster not found", func(t *testing.T) {
@@ -128,7 +128,7 @@ func TestSyncMemberConfigs(t *testing.T) {
 
 			// then
 			require.Len(t, syncErrors, 1)
-			assert.Equal(t, syncErrors["member2"], "specific member configuration exists but no matching toolchaincluster was found")
+			assert.Equal(t, "specific member configuration exists but no matching toolchaincluster was found", syncErrors["member2"])
 		})
 	})
 }

controllers/toolchainconfig/toolchainconfig_controller_test.go

@@ -53,12 +53,12 @@ func TestReconcile(t *testing.T) {
 
 			// check member1 config
 			_, err = getMemberConfig(member1)
-			assert.Error(t, err)
+			require.Error(t, err)
 			assert.True(t, errors.IsNotFound(err))
 
 			// check member2 config
 			_, err = getMemberConfig(member2)
-			assert.Error(t, err)
+			require.Error(t, err)
 			assert.True(t, errors.IsNotFound(err))
 		})
 
@@ -91,12 +91,12 @@ func TestReconcile(t *testing.T) {
 
 			// check member1 config
 			member1Cfg, err := getMemberConfig(member1)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, "10s", *member1Cfg.Spec.MemberStatus.RefreshPeriod)
 
 			// check member2 config
 			member2Cfg, err := getMemberConfig(member2)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, "5s", *member2Cfg.Spec.MemberStatus.RefreshPeriod)
 
 			t.Run("cache updated with new version", func(t *testing.T) {
@@ -126,12 +126,12 @@ func TestReconcile(t *testing.T) {
 
 				// check member1 config is unchanged
 				member1Cfg, err := getMemberConfig(member1)
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.Equal(t, "10s", *member1Cfg.Spec.MemberStatus.RefreshPeriod)
 
 				// check member2 config is updated
 				member2Cfg, err := getMemberConfig(member2)
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.Equal(t, "20s", *member2Cfg.Spec.MemberStatus.RefreshPeriod)
 			})
 
@@ -157,12 +157,12 @@ func TestReconcile(t *testing.T) {
 
 				// check member1 config is unchanged
 				member1Cfg, err := getMemberConfig(member1)
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.Equal(t, "10s", *member1Cfg.Spec.MemberStatus.RefreshPeriod)
 
 				// check member2 config is unchanged
 				member2Cfg, err := getMemberConfig(member2)
-				assert.NoError(t, err)
+				require.NoError(t, err)
 				assert.Equal(t, "20s", *member2Cfg.Spec.MemberStatus.RefreshPeriod)
 			})
 		})

controllers/toolchainstatus/toolchainstatus_controller_test.go

@@ -1220,8 +1220,8 @@ func TestToolchainStatusNotifications(t *testing.T) {
 						require.True(t, strings.HasPrefix(notification.ObjectMeta.Name, "toolchainstatus-unready-"))
 
 						require.NotNil(t, notification)
-						require.Equal(t, notification.Spec.Subject, "ToolchainStatus has been in an unready status for an extended period for host-cluster")
-						require.Equal(t, notification.Spec.Recipient, email)
+						require.Equal(t, "ToolchainStatus has been in an unready status for an extended period for host-cluster", notification.Spec.Subject)
+						require.Equal(t, email, notification.Spec.Recipient)
 
 						t.Run("Toolchain status now ok again, notification should be removed", func(t *testing.T) {
 							hostOperatorDeployment := newDeploymentWithConditions(defaultHostOperatorDeploymentName,

controllers/usersignup/usersignup_controller_test.go

@@ -664,7 +664,7 @@ func TestUpdateOfApprovedLabelFails(t *testing.T) {
 
 	// then
 	require.Error(t, err)
-	assert.EqualError(t, err, "unable to update state label at UserSignup resource: some error")
+	require.EqualError(t, err, "unable to update state label at UserSignup resource: some error")
 
 	// Lookup the user signup again
 	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: userSignup.Name, Namespace: req.Namespace}, userSignup)
@@ -2508,7 +2508,7 @@ func TestUserSignupFailedToCreateDeactivationNotification(t *testing.T) {
 		notificationList := &toolchainv1alpha1.NotificationList{}
 		err = r.Client.List(context.TODO(), notificationList)
 		require.NoError(t, err)
-		require.Equal(t, 0, len(notificationList.Items))
+		require.Empty(t, notificationList.Items)
 	})
 }
 
@@ -3674,7 +3674,7 @@ func TestDeathBy100Signups(t *testing.T) {
 
 			// then
 			require.Error(t, err)
-			assert.EqualError(t, err, fmt.Sprintf("Error generating compliant username for %s: unable to transform username [%s] even after 100 attempts", testusername.username, testusername.username))
+			require.EqualError(t, err, fmt.Sprintf("Error generating compliant username for %s: unable to transform username [%s] even after 100 attempts", testusername.username, testusername.username))
 			require.Equal(t, reconcile.Result{}, res)
 
 			// Lookup the user signup again
@@ -3854,7 +3854,7 @@ func TestUserSignupWithMultipleExistingMURNotOK(t *testing.T) {
 	_, err := r.Reconcile(context.TODO(), req)
 
 	// then
-	assert.EqualError(t, err, "Multiple MasterUserRecords found: multiple matching MasterUserRecord resources found")
+	require.EqualError(t, err, "Multiple MasterUserRecords found: multiple matching MasterUserRecord resources found")
 
 	key := types.NamespacedName{
 		Namespace: test.HostOperatorNs,
@@ -3912,7 +3912,7 @@ func TestApprovedManuallyUserSignupWhenNoMembersAvailable(t *testing.T) {
 	_, err := r.Reconcile(context.TODO(), req)
 
 	// then
-	assert.EqualError(t, err, "no target clusters available: no suitable member cluster found - capacity was reached")
+	require.EqualError(t, err, "no target clusters available: no suitable member cluster found - capacity was reached")
 	AssertThatCountersAndMetrics(t).
 		HaveMasterUserRecordsPerDomain(toolchainv1alpha1.Metric{
 			string(metrics.External): 1,