chore: enable compare and nil-compare rules from testifylint linter (a…

…rgoproj#18581) Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
AmadeusITGroup · Jun 16, 2024 · cda7a4e · cda7a4e
1 parent 06bfb64
commit cda7a4e
Show file tree

Hide file tree

Showing 14 changed files with 42 additions and 40 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -27,11 +27,9 @@ linters-settings:
   testifylint:
     enable-all: true
     disable:
-      - compares
       - error-is-as
       - float-compare
       - go-require
-      - nil-compare
       - require-error
 run:
   timeout: 50m
diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go
@@ -1652,7 +1652,7 @@ func TestRemoveFinalizerOnInvalidDestination_DestinationTypes(t *testing.T) {
 
 			finalizerRemoved := len(retrievedApp.Finalizers) == 0
 
-			assert.True(t, c.expectFinalizerRemoved == finalizerRemoved)
+			assert.Equal(t, c.expectFinalizerRemoved, finalizerRemoved)
 
 			bytes, _ := json.MarshalIndent(retrievedApp, "", "  ")
 			t.Log("Contents of app after call:", string(bytes))
@@ -2458,7 +2458,7 @@ func TestReconcilerValidationProjectErrorBehaviour(t *testing.T) {
 	// Verify that on validation error, no error is returned, but the object is requeued
 	res, err := r.Reconcile(context.Background(), req)
 	assert.NoError(t, err)
-	assert.True(t, res.RequeueAfter == ReconcileRequeueOnValidationError)
+	assert.Equal(t, ReconcileRequeueOnValidationError, res.RequeueAfter)
 
 	var app v1alpha1.Application
 
@@ -2546,7 +2546,7 @@ func TestReconcilerCreateAppsRecoveringRenderError(t *testing.T) {
 	// Verify that on generatorsError, no error is returned, but the object is requeued
 	res, err := r.Reconcile(context.Background(), req)
 	assert.NoError(t, err)
-	assert.True(t, res.RequeueAfter == ReconcileRequeueOnValidationError)
+	assert.Equal(t, ReconcileRequeueOnValidationError, res.RequeueAfter)
 
 	var app v1alpha1.Application
 
@@ -2694,7 +2694,7 @@ func applicationsUpdateSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
 	// Verify that on validation error, no error is returned, but the object is requeued
 	resCreate, err := r.Reconcile(context.Background(), req)
 	assert.NoError(t, err)
-	assert.True(t, resCreate.RequeueAfter == 0)
+	assert.Equal(t, time.Duration(0), resCreate.RequeueAfter)
 
 	var app v1alpha1.Application
 
@@ -2723,7 +2723,7 @@ func applicationsUpdateSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
 
 	err = r.Client.Get(context.TODO(), crtclient.ObjectKey{Namespace: "argocd", Name: "good-cluster"}, &app)
 	assert.NoError(t, err)
-	assert.True(t, resUpdate.RequeueAfter == 0)
+	assert.Equal(t, time.Duration(0), resUpdate.RequeueAfter)
 	assert.Equal(t, "good-cluster", app.Name)
 
 	return app
@@ -2858,7 +2858,7 @@ func applicationsDeleteSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
 	// Verify that on validation error, no error is returned, but the object is requeued
 	resCreate, err := r.Reconcile(context.Background(), req)
 	assert.NoError(t, err)
-	assert.True(t, resCreate.RequeueAfter == 0)
+	assert.Equal(t, time.Duration(0), resCreate.RequeueAfter)
 
 	var app v1alpha1.Application
 
@@ -2889,7 +2889,7 @@ func applicationsDeleteSyncPolicyTest(t *testing.T, applicationsSyncPolicy v1alp
 
 	err = r.Client.List(context.TODO(), &apps)
 	assert.NoError(t, err)
-	assert.True(t, resUpdate.RequeueAfter == 0)
+	assert.Equal(t, time.Duration(0), resUpdate.RequeueAfter)
 
 	return apps
 }
@@ -3168,7 +3168,7 @@ func TestPolicies(t *testing.T) {
 			// Check if Application is created
 			res, err := r.Reconcile(context.Background(), req)
 			assert.NoError(t, err)
-			assert.True(t, res.RequeueAfter == 0)
+			assert.Equal(t, time.Duration(0), res.RequeueAfter)
 
 			var app v1alpha1.Application
 			err = r.Client.Get(context.TODO(), crtclient.ObjectKey{Namespace: "argocd", Name: "my-app"}, &app)
@@ -3182,7 +3182,7 @@ func TestPolicies(t *testing.T) {
 
 			res, err = r.Reconcile(context.Background(), req)
 			assert.NoError(t, err)
-			assert.True(t, res.RequeueAfter == 0)
+			assert.Equal(t, time.Duration(0), res.RequeueAfter)
 
 			err = r.Client.Get(context.TODO(), crtclient.ObjectKey{Namespace: "argocd", Name: "my-app"}, &app)
 			assert.NoError(t, err)
@@ -3206,7 +3206,7 @@ func TestPolicies(t *testing.T) {
 
 			res, err = r.Reconcile(context.Background(), req)
 			assert.NoError(t, err)
-			assert.True(t, res.RequeueAfter == 0)
+			assert.Equal(t, time.Duration(0), res.RequeueAfter)
 
 			err = r.Client.Get(context.TODO(), crtclient.ObjectKey{Namespace: "argocd", Name: "my-app"}, &app)
 			assert.NoError(t, err)

diff --git a/applicationset/utils/utils_test.go b/applicationset/utils/utils_test.go
@@ -912,7 +912,7 @@ func TestCheckInvalidGenerators(t *testing.T) {
 		hook := logtest.NewGlobal()
 
 		_ = CheckInvalidGenerators(&c.appSet)
-		assert.True(t, len(hook.Entries) >= 1, c.testName)
+		assert.GreaterOrEqual(t, len(hook.Entries), 1, c.testName)
 		assert.NotNil(t, hook.LastEntry(), c.testName)
 		if hook.LastEntry() != nil {
 			assert.Equal(t, logrus.WarnLevel, hook.LastEntry().Level, c.testName)

diff --git a/cmd/argocd/commands/admin/project_allowlist_test.go b/cmd/argocd/commands/admin/project_allowlist_test.go
@@ -16,5 +16,5 @@ func TestProjectAllowListGen(t *testing.T) {
 
 	globalProj, err := generateProjectAllowList(resourceList, "testdata/test_clusterrole.yaml", "testproj")
 	assert.NoError(t, err)
-	assert.True(t, len(globalProj.Spec.NamespaceResourceWhitelist) > 0)
+	assert.Positive(t, len(globalProj.Spec.NamespaceResourceWhitelist))
 }
diff --git a/cmd/util/app_test.go b/cmd/util/app_test.go
@@ -222,7 +222,7 @@ func Test_setAppSpecOptions(t *testing.T) {
 	})
 	t.Run("RetryLimit", func(t *testing.T) {
 		assert.NoError(t, f.SetFlag("sync-retry-limit", "5"))
-		assert.True(t, f.spec.SyncPolicy.Retry.Limit == 5)
+		assert.Equal(t, int64(5), f.spec.SyncPolicy.Retry.Limit)
 
 		assert.NoError(t, f.SetFlag("sync-retry-limit", "0"))
 		assert.Nil(t, f.spec.SyncPolicy.Retry)

diff --git a/controller/metrics/metrics_test.go b/controller/metrics/metrics_test.go
@@ -2,7 +2,6 @@ package metrics
 
 import (
 	"context"
-	"fmt"
 	"log"
 	"net/http"
 	"net/http/httptest"
@@ -370,7 +369,7 @@ func assertMetricsPrinted(t *testing.T, expectedLines, body string) {
 		if line == "" {
 			continue
 		}
-		assert.Contains(t, body, line, fmt.Sprintf("expected metrics mismatch for line: %s", line))
+		assert.Contains(t, body, line, "expected metrics mismatch for line: %s", line)
 	}
 }
 

diff --git a/reposerver/cache/cache_test.go b/reposerver/cache/cache_test.go
@@ -253,7 +253,7 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
 	retrievedVal = &CachedManifestResponse{}
 	err = repoCache.GetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, retrievedVal, nil)
 
-	assert.True(t, err == cacheutil.ErrCacheMiss)
+	assert.Equal(t, err, cacheutil.ErrCacheMiss)
 
 	// Verify that the hash mismatch item has been deleted
 	items := getInMemoryCacheContents(t, inMemCache)

diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go
@@ -319,7 +319,7 @@ func TestGenerateManifests_K8SAPIResetCache(t *testing.T) {
 	res, err = service.GenerateManifest(context.Background(), &q)
 	assert.NoError(t, err)
 	assert.NotEqual(t, cachedFakeResponse, res)
-	assert.True(t, len(res.Manifests) > 1)
+	assert.Greater(t, len(res.Manifests), 1)
 }
 
 func TestGenerateManifests_EmptyCache(t *testing.T) {
@@ -338,7 +338,7 @@ func TestGenerateManifests_EmptyCache(t *testing.T) {
 
 	res, err := service.GenerateManifest(context.Background(), &q)
 	assert.NoError(t, err)
-	assert.True(t, len(res.Manifests) > 0)
+	assert.Positive(t, len(res.Manifests))
 	mockCache.mockCache.AssertCacheCalledTimes(t, &repositorymocks.CacheCallCounts{
 		ExternalSets:    2,
 		ExternalGets:    2,
@@ -837,9 +837,9 @@ func TestManifestGenErrorCacheByNumRequests(t *testing.T) {
 
 				// Verify invariant: res != nil xor err != nil
 				if err != nil {
-					assert.True(t, res == nil, "both err and res are non-nil res: %v   err: %v", res, err)
+					assert.Nil(t, res, "both err and res are non-nil res: %v   err: %v", res, err)
 				} else {
-					assert.True(t, res != nil, "both err and res are nil")
+					assert.NotNil(t, res, "both err and res are nil")
 				}
 
 				cachedManifestResponse := getRecentCachedEntry(service, manifestRequest)
@@ -854,13 +854,13 @@ func TestManifestGenErrorCacheByNumRequests(t *testing.T) {
 					// nolint:staticcheck
 					assert.Nil(t, cachedManifestResponse.ManifestResponse)
 					// nolint:staticcheck
-					assert.True(t, cachedManifestResponse.FirstFailureTimestamp != 0)
+					assert.NotEqual(t, 0, cachedManifestResponse.FirstFailureTimestamp)
 
 					// Internal cache consec failures value should increase with invocations, cached response should stay the same,
 					// nolint:staticcheck
-					assert.True(t, cachedManifestResponse.NumberOfConsecutiveFailures == adjustedInvocation+1)
+					assert.Equal(t, cachedManifestResponse.NumberOfConsecutiveFailures, adjustedInvocation+1)
 					// nolint:staticcheck
-					assert.True(t, cachedManifestResponse.NumberOfCachedResponsesReturned == 0)
+					assert.Equal(t, 0, cachedManifestResponse.NumberOfCachedResponsesReturned)
 				} else {
 					// GenerateManifest SHOULD return cached errors for the next X responses, where X is the
 					// PauseGenerationOnFailureForRequests constant
@@ -869,13 +869,13 @@ func TestManifestGenErrorCacheByNumRequests(t *testing.T) {
 					// nolint:staticcheck
 					assert.Nil(t, cachedManifestResponse.ManifestResponse)
 					// nolint:staticcheck
-					assert.True(t, cachedManifestResponse.FirstFailureTimestamp != 0)
+					assert.NotEqual(t, 0, cachedManifestResponse.FirstFailureTimestamp)
 
 					// Internal cache values should update correctly based on number of return cache entries, consecutive failures should stay the same
 					// nolint:staticcheck
-					assert.True(t, cachedManifestResponse.NumberOfConsecutiveFailures == service.initConstants.PauseGenerationAfterFailedGenerationAttempts)
+					assert.Equal(t, cachedManifestResponse.NumberOfConsecutiveFailures, service.initConstants.PauseGenerationAfterFailedGenerationAttempts)
 					// nolint:staticcheck
-					assert.True(t, cachedManifestResponse.NumberOfCachedResponsesReturned == (adjustedInvocation-service.initConstants.PauseGenerationAfterFailedGenerationAttempts+1))
+					assert.Equal(t, cachedManifestResponse.NumberOfCachedResponsesReturned, (adjustedInvocation - service.initConstants.PauseGenerationAfterFailedGenerationAttempts + 1))
 				}
 			}
 		})
@@ -931,8 +931,13 @@ func TestManifestGenErrorCacheFileContentsChange(t *testing.T) {
 		fmt.Println("    res: ", res)
 
 		if step < 2 {
-			assert.True(t, (err != nil) == errorExpected, "error return value and error expected did not match")
-			assert.True(t, (res != nil) == !errorExpected, "GenerateManifest return value and expected value did not match")
+			if errorExpected {
+				assert.Error(t, err, "error return value and error expected did not match")
+				assert.Nil(t, res, "GenerateManifest return value and expected value did not match")
+			} else {
+				assert.NoError(t, err, "error return value and error expected did not match")
+				assert.NotNil(t, res, "GenerateManifest return value and expected value did not match")
+			}
 		}
 
 		if step == 2 {

diff --git a/server/extension/extension_test.go b/server/extension/extension_test.go
@@ -231,7 +231,7 @@ func TestRegisterExtensions(t *testing.T) {
 				err := f.manager.RegisterExtensions()
 
 				// then
-				assert.Error(t, err, fmt.Sprintf("expected error in test %s but got nil", tc.name))
+				assert.Error(t, err, "expected error in test %s but got nil", tc.name)
 			})
 		}
 	})

diff --git a/test/e2e/app_management_ns_test.go b/test/e2e/app_management_ns_test.go
@@ -601,7 +601,7 @@ func TestNamespacedManipulateApplicationResources(t *testing.T) {
 					break
 				}
 			}
-			assert.True(t, index > -1)
+			assert.Greater(t, index, -1)
 
 			deployment := resources[index]
 
@@ -1426,7 +1426,7 @@ func TestNamespacedSelfManagedApps(t *testing.T) {
 				lastReconciledAt = reconciledAt
 			}
 
-			assert.True(t, reconciledCount < 3, "Application was reconciled too many times")
+			assert.Less(t, reconciledCount, 3, "Application was reconciled too many times")
 		})
 }
 

diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go
@@ -737,7 +737,7 @@ func TestManipulateApplicationResources(t *testing.T) {
 					break
 				}
 			}
-			assert.True(t, index > -1)
+			assert.Greater(t, index, -1)
 
 			deployment := resources[index]
 
@@ -1853,7 +1853,7 @@ func TestSelfManagedApps(t *testing.T) {
 				lastReconciledAt = reconciledAt
 			}
 
-			assert.True(t, reconciledCount < 3, "Application was reconciled too many times")
+			assert.Less(t, reconciledCount, 3, "Application was reconciled too many times")
 		})
 }
 

diff --git a/test/e2e/jsonnet_test.go b/test/e2e/jsonnet_test.go
@@ -35,7 +35,7 @@ func TestJsonnetAppliedCorrectly(t *testing.T) {
 				}
 			}
 
-			assert.True(t, index > -1)
+			assert.Greater(t, index, -1)
 
 			deployment := resources[index]
 			assert.Equal(t, "jsonnet-guestbook-ui", deployment.GetName())
@@ -65,7 +65,7 @@ func TestJsonnetTlaParameterAppliedCorrectly(t *testing.T) {
 				}
 			}
 
-			assert.True(t, index > -1)
+			assert.Greater(t, index, -1)
 
 			deployment := resources[index]
 			assert.Equal(t, "testing-tla", deployment.GetName())

diff --git a/util/cache/redis_test.go b/util/cache/redis_test.go
@@ -108,7 +108,7 @@ func TestRedisSetCacheCompressed(t *testing.T) {
 
 	compressedData, err := redisClient.Get(context.Background(), "my-key.gz").Bytes()
 	assert.NoError(t, err)
-	assert.True(t, len(compressedData) > len([]byte(testValue)), "compressed data is bigger than uncompressed")
+	assert.Greater(t, len(compressedData), len([]byte(testValue)), "compressed data is bigger than uncompressed")
 
 	var result string
 	assert.NoError(t, client.Get("my-key", &result))

diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go
@@ -127,14 +127,14 @@ func TestInClusterServerAddressEnabled(t *testing.T) {
 	})
 	argoCDCM, err := settingsManager.getConfigMap()
 	assert.NoError(t, err)
-	assert.True(t, argoCDCM.Data[inClusterEnabledKey] == "true")
+	assert.Equal(t, "true", argoCDCM.Data[inClusterEnabledKey])
 
 	_, settingsManager = fixtures(map[string]string{
 		"cluster.inClusterEnabled": "false",
 	})
 	argoCDCM, err = settingsManager.getConfigMap()
 	assert.NoError(t, err)
-	assert.False(t, argoCDCM.Data[inClusterEnabledKey] == "true")
+	assert.NotEqual(t, "true", argoCDCM.Data[inClusterEnabledKey])
 }
 
 func TestInClusterServerAddressEnabledByDefault(t *testing.T) {