[chore]: enable error-nil and nil-compare rules from testifylint (ope…

…n-telemetry#34936)

#### Description

Testifylint is a linter that provides best practices with the use of
testify.

This PR enables
[error-nil](https://github.com/Antonboom/testifylint?tab=readme-ov-file#error-nil)
and
[nil-compare](https://github.com/Antonboom/testifylint?tab=readme-ov-file#nil-compare)
rules from [testifylint](https://github.com/Antonboom/testifylint)


It also adds testifylint as tool to use with a make command

.golangci.yml

@@ -137,14 +137,14 @@ linters-settings:
       - compares
       - empty
       - error-is-as
-      - error-nil
       - expected-actual
       - float-compare
+      - formatter
       - go-require
       - negative-positive
-      - nil-compare
       - require-error
       - suite-dont-use-pkg
+      - suite-subtest-run
       - useless-assert
     enable-all: true
 

Makefile

@@ -147,6 +147,14 @@ golint:
 gogovulncheck:
 	$(MAKE) $(FOR_GROUP_TARGET) TARGET="govulncheck"
 
+.PHONY: gotestifylint
+gotestifylint:
+	$(MAKE) $(FOR_GROUP_TARGET) TARGET="testifylint"
+
+.PHONY: gotestifylint-fix
+gotestifylint-fix:
+	$(MAKE) $(FOR_GROUP_TARGET) TARGET="testifylint-fix"
+
 .PHONY: goporto
 goporto: $(PORTO)
 	$(PORTO) -w --include-internal --skip-dirs "^cmd$$" ./

Makefile.Common

@@ -72,8 +72,10 @@ BUILDER             := $(TOOLS_BIN_DIR)/builder
 GOVULNCHECK         := $(TOOLS_BIN_DIR)/govulncheck
 GCI                 := $(TOOLS_BIN_DIR)/gci
 GOTESTSUM           := $(TOOLS_BIN_DIR)/gotestsum
+TESTIFYLINT         := $(TOOLS_BIN_DIR)/testifylint
 
 GOTESTSUM_OPT?= --rerun-fails=1
+TESTIFYLINT_OPT?= --enable-all --disable=compares,empty,error-is-as,expected-actual,float-compare,formatter,go-require,negative-positive,require-error,suite-dont-use-pkg,suite-subtest-run,useless-assert
 
 # BUILD_TYPE should be one of (dev, release).
 BUILD_TYPE?=release
@@ -220,6 +222,15 @@ misspell-correction: $(TOOLS_BIN_DIR)/misspell
 moddownload:
 	$(GOCMD) mod download
 
+.PHONY: testifylint
+testifylint: $(TESTIFYLINT)
+	@echo "running $(TESTIFYLINT)"
+	$(TESTIFYLINT) $(TESTIFYLINT_OPT) ./...
+
+.PHONY: testifylint-fix
+testifylint-fix:
+	@$(MAKE) testifylint TESTIFYLINT_OPT="${TESTIFYLINT_OPT} --fix"
+
 .PHONY: gci
 gci: $(TOOLS_BIN_DIR)/gci
 	@echo "running $(GCI)"

exporter/alibabacloudlogserviceexporter/trace_exporter_test.go

@@ -31,7 +31,7 @@ func TestNewTracesExporter(t *testing.T) {
 	// This will put trace data to send buffer and return success.
 	err = got.ConsumeTraces(context.Background(), traces)
 	assert.NoError(t, err)
-	assert.Nil(t, got.Shutdown(context.Background()))
+	assert.NoError(t, got.Shutdown(context.Background()))
 }
 
 func TestNewFailsWithEmptyTracesExporterName(t *testing.T) {

exporter/awsemfexporter/emf_exporter_test.go

@@ -314,10 +314,10 @@ func TestPushMetricsDataWithErr(t *testing.T) {
 		metricNames:  []string{"metric_1", "metric_2"},
 		metricValues: [][]float64{{100}, {4}},
 	})
-	assert.NotNil(t, exp.pushMetricsData(ctx, md))
-	assert.NotNil(t, exp.pushMetricsData(ctx, md))
-	assert.Nil(t, exp.pushMetricsData(ctx, md))
-	assert.Nil(t, exp.shutdown(ctx))
+	assert.Error(t, exp.pushMetricsData(ctx, md))
+	assert.Error(t, exp.pushMetricsData(ctx, md))
+	assert.NoError(t, exp.pushMetricsData(ctx, md))
+	assert.NoError(t, exp.shutdown(ctx))
 }
 
 func TestNewExporterWithoutConfig(t *testing.T) {

exporter/awsemfexporter/grouped_metric_test.go

@@ -462,7 +462,7 @@ func BenchmarkAddToGroupedMetric(b *testing.B) {
 		for i := 0; i < numMetrics; i++ {
 			metadata := generateTestMetricMetadata("namespace", int64(1596151098037), "log-group", "log-stream", "cloudwatch-otel", metrics.At(i).Type())
 			err := addToGroupedMetric(metrics.At(i), groupedMetrics, metadata, true, nil, testCfg, emfCalcs)
-			assert.Nil(b, err)
+			assert.NoError(b, err)
 		}
 	}
 }

exporter/awsemfexporter/metric_translator_test.go

@@ -1984,7 +1984,7 @@ func BenchmarkTranslateOtToGroupedMetricWithInstrLibrary(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		groupedMetric := make(map[any]*groupedMetric)
 		err := translator.translateOTelToGroupedMetric(rm, groupedMetric, config)
-		assert.Nil(b, err)
+		assert.NoError(b, err)
 	}
 }
 
@@ -2007,7 +2007,7 @@ func BenchmarkTranslateOtToGroupedMetricWithoutConfigReplacePattern(b *testing.B
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[any]*groupedMetric)
 		err := translator.translateOTelToGroupedMetric(rm, groupedMetrics, config)
-		assert.Nil(b, err)
+		assert.NoError(b, err)
 	}
 }
 
@@ -2030,7 +2030,7 @@ func BenchmarkTranslateOtToGroupedMetricWithConfigReplaceWithResource(b *testing
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[any]*groupedMetric)
 		err := translator.translateOTelToGroupedMetric(rm, groupedMetrics, config)
-		assert.Nil(b, err)
+		assert.NoError(b, err)
 	}
 }
 
@@ -2053,7 +2053,7 @@ func BenchmarkTranslateOtToGroupedMetricWithConfigReplaceWithLabel(b *testing.B)
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[any]*groupedMetric)
 		err := translator.translateOTelToGroupedMetric(rm, groupedMetrics, config)
-		assert.Nil(b, err)
+		assert.NoError(b, err)
 	}
 }
 
@@ -2071,7 +2071,7 @@ func BenchmarkTranslateOtToGroupedMetricWithoutInstrLibrary(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		groupedMetrics := make(map[any]*groupedMetric)
 		err := translator.translateOTelToGroupedMetric(rm, groupedMetrics, config)
-		assert.Nil(b, err)
+		assert.NoError(b, err)
 	}
 }
 
@@ -2140,7 +2140,7 @@ func BenchmarkTranslateGroupedMetricToCWMetricWithFiltering(b *testing.B) {
 	}
 	logger := zap.NewNop()
 	err := m.init(logger)
-	assert.Nil(b, err)
+	assert.NoError(b, err)
 	config := &Config{
 		MetricDeclarations:    []*MetricDeclaration{m},
 		DimensionRollupOption: zeroAndSingleDimensionRollup,

exporter/awsxrayexporter/internal/translator/segment_test.go

@@ -409,7 +409,7 @@ func TestFixSegmentName(t *testing.T) {
 
 func TestFixAnnotationKey(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validKey := "Key_1"
 	fixedKey := fixAnnotationKey(validKey)
@@ -424,7 +424,7 @@ func TestFixAnnotationKey(t *testing.T) {
 
 func TestFixAnnotationKeyWithAllowDot(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", true)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validKey := "Key_1"
 	fixedKey := fixAnnotationKey(validKey)
@@ -585,7 +585,7 @@ func TestSpanWithAttributesSegmentMetadata(t *testing.T) {
 
 func TestResourceAttributesCanBeIndexed(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "/api/locations"
 	parentSpanID := newSegmentID()
@@ -618,7 +618,7 @@ func TestResourceAttributesCanBeIndexed(t *testing.T) {
 
 func TestResourceAttributesCanBeIndexedWithAllowDot(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", true)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "/api/locations"
 	parentSpanID := newSegmentID()
@@ -672,7 +672,7 @@ func TestResourceAttributesNotIndexedIfSubsegment(t *testing.T) {
 
 func TestSpanWithSpecialAttributesAsListed(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "/api/locations"
 	parentSpanID := newSegmentID()
@@ -692,7 +692,7 @@ func TestSpanWithSpecialAttributesAsListed(t *testing.T) {
 
 func TestSpanWithSpecialAttributesAsListedWithAllowDot(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", true)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "/api/locations"
 	parentSpanID := newSegmentID()
@@ -712,7 +712,7 @@ func TestSpanWithSpecialAttributesAsListedWithAllowDot(t *testing.T) {
 
 func TestSpanWithSpecialAttributesAsListedAndIndexAll(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "/api/locations"
 	parentSpanID := newSegmentID()
@@ -731,7 +731,7 @@ func TestSpanWithSpecialAttributesAsListedAndIndexAll(t *testing.T) {
 
 func TestSpanWithSpecialAttributesAsListedAndIndexAllWithAllowDot(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", true)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "/api/locations"
 	parentSpanID := newSegmentID()
@@ -1142,7 +1142,7 @@ func TestAwsSdkSpanWithAwsRemoteServiceName(t *testing.T) {
 	jsonStr, err := MakeSegmentDocumentString(span, resource, nil, false, nil, false)
 
 	assert.NotNil(t, jsonStr)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.True(t, strings.Contains(jsonStr, "DynamoDb"))
 	assert.False(t, strings.Contains(jsonStr, "DynamoDb.PutItem"))
 	assert.False(t, strings.Contains(jsonStr, user))
@@ -1336,7 +1336,7 @@ func addSpanLink(span ptrace.Span) {
 
 func TestLocalRootConsumer(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "destination operation"
 	resource := getBasicResource()
@@ -1352,7 +1352,7 @@ func TestLocalRootConsumer(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validateLocalRootDependencySubsegment(t, segments[0], span, *segments[1].ID)
 	assert.Nil(t, segments[0].Links)
@@ -1383,7 +1383,7 @@ func TestNonLocalRootConsumerProcess(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	tempTraceID := span.TraceID()
 	expectedTraceID := "1-" + fmt.Sprintf("%x", tempTraceID[0:4]) + "-" + fmt.Sprintf("%x", tempTraceID[4:16])
@@ -1429,15 +1429,15 @@ func TestLocalRootConsumerAWSNamespace(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Ensure that AWS namespace is not overwritten to remote
 	assert.Equal(t, "aws", *segments[0].Namespace)
 }
 
 func TestLocalRootClient(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "SQS Get"
 	resource := getBasicResource()
@@ -1455,7 +1455,7 @@ func TestLocalRootClient(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validateLocalRootDependencySubsegment(t, segments[0], span, *segments[1].ID)
 	assert.Len(t, segments[0].Links, 1)
@@ -1492,7 +1492,7 @@ func TestLocalRootClientAwsServiceMetrics(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	subsegment := segments[0]
 
@@ -1516,7 +1516,7 @@ func TestLocalRootProducer(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validateLocalRootDependencySubsegment(t, segments[0], span, *segments[1].ID)
 	assert.Len(t, segments[0].Links, 1)
@@ -1577,7 +1577,7 @@ func validateLocalRootWithoutDependency(t *testing.T, segment *awsxray.Segment,
 
 func TestLocalRootServer(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "MyService"
 	resource := getBasicResource()
@@ -1593,14 +1593,14 @@ func TestLocalRootServer(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validateLocalRootWithoutDependency(t, segments[0], span)
 }
 
 func TestLocalRootInternal(t *testing.T) {
 	err := featuregate.GlobalRegistry().Set("exporter.xray.allowDot", false)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	spanName := "MyInternalService"
 	resource := getBasicResource()
@@ -1616,7 +1616,7 @@ func TestLocalRootInternal(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	validateLocalRootWithoutDependency(t, segments[0], span)
 }
@@ -1637,7 +1637,7 @@ func TestNotLocalRootInternal(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Validate segment
 	assert.Equal(t, "subsegment", *segments[0].Type)
@@ -1661,7 +1661,7 @@ func TestNotLocalRootConsumer(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Validate segment
 	assert.Equal(t, "subsegment", *segments[0].Type)
@@ -1685,7 +1685,7 @@ func TestNotLocalRootClient(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Validate segment
 	assert.Equal(t, "subsegment", *segments[0].Type)
@@ -1709,7 +1709,7 @@ func TestNotLocalRootProducer(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Validate segment
 	assert.Equal(t, "subsegment", *segments[0].Type)
@@ -1735,7 +1735,7 @@ func TestNotLocalRootServer(t *testing.T) {
 
 	assert.NotNil(t, segments)
 	assert.Len(t, segments, 1)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Validate segment
 	assert.Nil(t, segments[0].Type)

exporter/awsxrayexporter/internal/translator/span_links_test.go

@@ -78,11 +78,11 @@ func TestOldSpanLinkError(t *testing.T) {
 
 	_, error1 := MakeSegment(span, resource, nil, false, nil, false)
 
-	assert.NotNil(t, error1)
+	assert.Error(t, error1)
 
 	_, error2 := MakeSegmentDocumentString(span, resource, nil, false, nil, false)
 
-	assert.NotNil(t, error2)
+	assert.Error(t, error2)
 }
 
 func TestTwoSpanLinks(t *testing.T) {