[chore]: enable error-nil and nil-compare rules from testifylint (#11120)

mmorel-35 · web-flow · commit 2720f59f97cc · 2024-09-10T14:34:29.000-07:00
#### 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) Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
diff --git a/.golangci.yml b/.golangci.yml
@@ -126,13 +126,11 @@ linters-settings:
     disable:
       - compares
       - error-is-as
-      - error-nil
       - expected-actual
       - float-compare
       - formatter
       - go-require
       - negative-positive
-      - nil-compare
       - require-error
     enable-all: true   
 
diff --git a/Makefile.Common b/Makefile.Common
@@ -38,7 +38,7 @@ SEMCONVGEN   := $(TOOLS_BIN_DIR)/semconvgen
 SEMCONVKIT   := $(TOOLS_BIN_DIR)/semconvkit
 TESTIFYLINT  := $(TOOLS_BIN_DIR)/testifylint
 
-TESTIFYLINT_OPT?= --enable-all --disable=compares,error-is-as,error-nil,expected-actual,float-compare,formatter,go-require,negative-positive,nil-compare,require-error
+TESTIFYLINT_OPT?= --enable-all --disable=compares,error-is-as,expected-actual,float-compare,formatter,go-require,negative-positive,require-error
 
 .PHONY: install-tools
 install-tools: $(TOOLS_BIN_NAMES)
diff --git a/component/componentstatus/status_test.go b/component/componentstatus/status_test.go
@@ -27,7 +27,7 @@ func TestNewStatusEvent(t *testing.T) {
 		t.Run(fmt.Sprintf("%s without error", status), func(t *testing.T) {
 			ev := NewEvent(status)
 			require.Equal(t, status, ev.Status())
-			require.Nil(t, ev.Err())
+			require.NoError(t, ev.Err())
 			require.False(t, ev.Timestamp().IsZero())
 		})
 	}
diff --git a/config/configtls/configtls_test.go b/config/configtls/configtls_test.go
@@ -679,7 +679,7 @@ func TestConfigValidate(t *testing.T) {
 			err := test.tlsConfig.Validate()
 
 			if test.errorTxt == "" {
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 			} else {
 				assert.EqualError(t, err, test.errorTxt)
 			}
diff --git a/confmap/resolver_test.go b/confmap/resolver_test.go
@@ -389,7 +389,7 @@ func TestResolverShutdownClosesWatch(t *testing.T) {
 	go func() {
 		errW, ok := <-resolver.Watch()
 		// Channel is closed, no exception
-		assert.Nil(t, errW)
+		assert.NoError(t, errW)
 		assert.False(t, ok)
 		watcherWG.Done()
 	}()
diff --git a/exporter/exporterhelper/batch_sender_test.go b/exporter/exporterhelper/batch_sender_test.go
@@ -561,7 +561,7 @@ func TestBatchSender_ShutdownDeadlock(t *testing.T) {
 	doneShutdown := make(chan struct{})
 	go func() {
 		close(startShutdown)
-		require.Nil(t, be.Shutdown(context.Background()))
+		require.NoError(t, be.Shutdown(context.Background()))
 		close(doneShutdown)
 	}()
 	<-startShutdown
diff --git a/exporter/exporterhelper/common_test.go b/exporter/exporterhelper/common_test.go
@@ -69,7 +69,7 @@ func checkStatus(t *testing.T, sd sdktrace.ReadOnlySpan, err error) {
 func TestQueueOptionsWithRequestExporter(t *testing.T) {
 	bs, err := newBaseExporter(exportertest.NewNopSettings(), defaultDataType, newNoopObsrepSender,
 		WithRetry(configretry.NewDefaultBackOffConfig()))
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.Nil(t, bs.marshaler)
 	require.Nil(t, bs.unmarshaler)
 	_, err = newBaseExporter(exportertest.NewNopSettings(), defaultDataType, newNoopObsrepSender,
@@ -90,7 +90,7 @@ func TestBaseExporterLogging(t *testing.T) {
 	rCfg := configretry.NewDefaultBackOffConfig()
 	rCfg.Enabled = false
 	bs, err := newBaseExporter(set, defaultDataType, newNoopObsrepSender, WithRetry(rCfg))
-	require.Nil(t, err)
+	require.NoError(t, err)
 	sendErr := bs.send(context.Background(), newErrorRequest())
 	require.Error(t, sendErr)
 
diff --git a/exporter/exporterhelper/logs_batch_test.go b/exporter/exporterhelper/logs_batch_test.go
@@ -18,7 +18,7 @@ func TestMergeLogs(t *testing.T) {
 	lr1 := &logsRequest{ld: testdata.GenerateLogs(2)}
 	lr2 := &logsRequest{ld: testdata.GenerateLogs(3)}
 	res, err := mergeLogs(context.Background(), lr1, lr2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, 5, res.(*logsRequest).ld.LogRecordCount())
 }
 
@@ -132,7 +132,7 @@ func TestMergeSplitLogs(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			res, err := mergeSplitLogs(context.Background(), tt.cfg, tt.lr1, tt.lr2)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, len(tt.expected), len(res))
 			for i, r := range res {
 				assert.Equal(t, tt.expected[i], r.(*logsRequest))
diff --git a/exporter/exporterhelper/logs_test.go b/exporter/exporterhelper/logs_test.go
@@ -227,7 +227,7 @@ func TestLogsExporter_WithRecordMetrics_ReturnError(t *testing.T) {
 	t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
 
 	le, err := NewLogsExporter(context.Background(), exporter.Settings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()}, &fakeLogsExporterConfig, newPushLogsData(want))
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, le)
 
 	checkRecordedMetricsForLogsExporter(t, tt, le, want)
@@ -241,7 +241,7 @@ func TestLogsRequestExporter_WithRecordMetrics_ExportError(t *testing.T) {
 
 	le, err := NewLogsRequestExporter(context.Background(), exporter.Settings{ID: fakeLogsExporterName, TelemetrySettings: tt.TelemetrySettings(), BuildInfo: component.NewDefaultBuildInfo()},
 		(&fakeRequestConverter{requestError: want}).requestFromLogsFunc)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, le)
 
 	checkRecordedMetricsForLogsExporter(t, tt, le, want)
@@ -280,7 +280,7 @@ func TestLogsExporter_WithSpan(t *testing.T) {
 	defer otel.SetTracerProvider(nooptrace.NewTracerProvider())
 
 	le, err := NewLogsExporter(context.Background(), set, &fakeLogsExporterConfig, newPushLogsData(nil))
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, le)
 	checkWrapSpanForLogsExporter(t, sr, set.TracerProvider.Tracer("test"), le, nil, 1)
 }
@@ -293,7 +293,7 @@ func TestLogsRequestExporter_WithSpan(t *testing.T) {
 	defer otel.SetTracerProvider(nooptrace.NewTracerProvider())
 
 	le, err := NewLogsRequestExporter(context.Background(), set, (&fakeRequestConverter{}).requestFromLogsFunc)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, le)
 	checkWrapSpanForLogsExporter(t, sr, set.TracerProvider.Tracer("test"), le, nil, 1)
 }
@@ -307,7 +307,7 @@ func TestLogsExporter_WithSpan_ReturnError(t *testing.T) {
 
 	want := errors.New("my_error")
 	le, err := NewLogsExporter(context.Background(), set, &fakeLogsExporterConfig, newPushLogsData(want))
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, le)
 	checkWrapSpanForLogsExporter(t, sr, set.TracerProvider.Tracer("test"), le, want, 1)
 }
@@ -321,7 +321,7 @@ func TestLogsRequestExporter_WithSpan_ReturnError(t *testing.T) {
 
 	want := errors.New("my_error")
 	le, err := NewLogsRequestExporter(context.Background(), set, (&fakeRequestConverter{requestError: want}).requestFromLogsFunc)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, le)
 	checkWrapSpanForLogsExporter(t, sr, set.TracerProvider.Tracer("test"), le, want, 1)
 }
@@ -334,7 +334,7 @@ func TestLogsExporter_WithShutdown(t *testing.T) {
 	assert.NotNil(t, le)
 	assert.NoError(t, err)
 
-	assert.Nil(t, le.Shutdown(context.Background()))
+	assert.NoError(t, le.Shutdown(context.Background()))
 	assert.True(t, shutdownCalled)
 }
 
@@ -347,7 +347,7 @@ func TestLogsRequestExporter_WithShutdown(t *testing.T) {
 	assert.NotNil(t, le)
 	assert.NoError(t, err)
 
-	assert.Nil(t, le.Shutdown(context.Background()))
+	assert.NoError(t, le.Shutdown(context.Background()))
 	assert.True(t, shutdownCalled)
 }
 
diff --git a/exporter/exporterhelper/metrics_batch_test.go b/exporter/exporterhelper/metrics_batch_test.go
@@ -18,7 +18,7 @@ func TestMergeMetrics(t *testing.T) {
 	mr1 := &metricsRequest{md: testdata.GenerateMetrics(2)}
 	mr2 := &metricsRequest{md: testdata.GenerateMetrics(3)}
 	res, err := mergeMetrics(context.Background(), mr1, mr2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, 5, res.(*metricsRequest).md.MetricCount())
 }
 
@@ -133,7 +133,7 @@ func TestMergeSplitMetrics(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			res, err := mergeSplitMetrics(context.Background(), tt.cfg, tt.mr1, tt.mr2)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, len(tt.expected), len(res))
 			for i := range res {
 				assert.Equal(t, tt.expected[i], res[i].(*metricsRequest))
diff --git a/exporter/exporterhelper/traces_batch_test.go b/exporter/exporterhelper/traces_batch_test.go
@@ -18,7 +18,7 @@ func TestMergeTraces(t *testing.T) {
 	tr1 := &tracesRequest{td: testdata.GenerateTraces(2)}
 	tr2 := &tracesRequest{td: testdata.GenerateTraces(3)}
 	res, err := mergeTraces(context.Background(), tr1, tr2)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, 5, res.(*tracesRequest).td.SpanCount())
 }
 
@@ -133,7 +133,7 @@ func TestMergeSplitTraces(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			res, err := mergeSplitTraces(context.Background(), tt.cfg, tt.tr1, tt.tr2)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, len(tt.expected), len(res))
 			for i := range res {
 				assert.Equal(t, tt.expected[i], res[i].(*tracesRequest))
diff --git a/exporter/otlpexporter/factory_test.go b/exporter/otlpexporter/factory_test.go
@@ -43,7 +43,7 @@ func TestCreateMetricsExporter(t *testing.T) {
 
 	set := exportertest.NewNopSettings()
 	oexp, err := factory.CreateMetricsExporter(context.Background(), set, cfg)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, oexp)
 }
 
@@ -194,6 +194,6 @@ func TestCreateLogsExporter(t *testing.T) {
 
 	set := exportertest.NewNopSettings()
 	oexp, err := factory.CreateLogsExporter(context.Background(), set, cfg)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, oexp)
 }
diff --git a/exporter/otlphttpexporter/factory_test.go b/exporter/otlphttpexporter/factory_test.go
@@ -46,7 +46,7 @@ func TestCreateMetricsExporter(t *testing.T) {
 
 	set := exportertest.NewNopSettings()
 	oexp, err := factory.CreateMetricsExporter(context.Background(), set, cfg)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, oexp)
 }
 
@@ -205,7 +205,7 @@ func TestCreateLogsExporter(t *testing.T) {
 
 	set := exportertest.NewNopSettings()
 	oexp, err := factory.CreateLogsExporter(context.Background(), set, cfg)
-	require.Nil(t, err)
+	require.NoError(t, err)
 	require.NotNil(t, oexp)
 }
 
diff --git a/exporter/otlphttpexporter/otlp_test.go b/exporter/otlphttpexporter/otlp_test.go
@@ -607,7 +607,7 @@ func TestPartialResponse_missingHeaderAndBody(t *testing.T) {
 					},
 				}
 				err = handlePartialSuccessResponse(resp, tt.handler)
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 			})
 		}
 	}
diff --git a/pdata/pcommon/value_test.go b/pdata/pcommon/value_test.go
@@ -255,7 +255,7 @@ func TestValue_CopyTo(t *testing.T) {
 	av := NewValueEmpty()
 	destVal := otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_IntValue{}}
 	av.CopyTo(newValue(&destVal, &state))
-	assert.EqualValues(t, nil, destVal.Value)
+	assert.Nil(t, destVal.Value)
 }
 
 func TestSliceWithNilValues(t *testing.T) {
diff --git a/processor/processorhelper/logs_test.go b/processor/processorhelper/logs_test.go
@@ -61,7 +61,7 @@ func TestNewLogsProcessor_ProcessLogError(t *testing.T) {
 func TestNewLogsProcessor_ProcessLogsErrSkipProcessingData(t *testing.T) {
 	lp, err := NewLogsProcessor(context.Background(), processortest.NewNopSettings(), &testLogsCfg, consumertest.NewNop(), newTestLProcessor(ErrSkipProcessingData))
 	require.NoError(t, err)
-	assert.Equal(t, nil, lp.ConsumeLogs(context.Background(), plog.NewLogs()))
+	assert.NoError(t, lp.ConsumeLogs(context.Background(), plog.NewLogs()))
 }
 
 func newTestLProcessor(retError error) ProcessLogsFunc {
diff --git a/processor/processorhelper/metrics_test.go b/processor/processorhelper/metrics_test.go
@@ -61,7 +61,7 @@ func TestNewMetricsProcessor_ProcessMetricsError(t *testing.T) {
 func TestNewMetricsProcessor_ProcessMetricsErrSkipProcessingData(t *testing.T) {
 	mp, err := NewMetricsProcessor(context.Background(), processortest.NewNopSettings(), &testMetricsCfg, consumertest.NewNop(), newTestMProcessor(ErrSkipProcessingData))
 	require.NoError(t, err)
-	assert.Equal(t, nil, mp.ConsumeMetrics(context.Background(), pmetric.NewMetrics()))
+	assert.NoError(t, mp.ConsumeMetrics(context.Background(), pmetric.NewMetrics()))
 }
 
 func newTestMProcessor(retError error) ProcessMetricsFunc {
diff --git a/processor/processorhelper/traces_test.go b/processor/processorhelper/traces_test.go
@@ -61,7 +61,7 @@ func TestNewTracesProcessor_ProcessTraceError(t *testing.T) {
 func TestNewTracesProcessor_ProcessTracesErrSkipProcessingData(t *testing.T) {
 	tp, err := NewTracesProcessor(context.Background(), processortest.NewNopSettings(), &testTracesCfg, consumertest.NewNop(), newTestTProcessor(ErrSkipProcessingData))
 	require.NoError(t, err)
-	assert.Equal(t, nil, tp.ConsumeTraces(context.Background(), ptrace.NewTraces()))
+	assert.NoError(t, tp.ConsumeTraces(context.Background(), ptrace.NewTraces()))
 }
 
 func newTestTProcessor(retError error) ProcessTracesFunc {

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ func TestNewStatusEvent(t *testing.T) {`
`27`	`27`	`t.Run(fmt.Sprintf("%s without error", status), func(t *testing.T) {`
`28`	`28`	`ev := NewEvent(status)`
`29`	`29`	`require.Equal(t, status, ev.Status())`
`30`		`- require.Nil(t, ev.Err())`
	`30`	`+ require.NoError(t, ev.Err())`
`31`	`31`	`require.False(t, ev.Timestamp().IsZero())`
`32`	`32`	`})`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -679,7 +679,7 @@ func TestConfigValidate(t *testing.T) {`
`679`	`679`	`err := test.tlsConfig.Validate()`
`680`	`680`
`681`	`681`	`if test.errorTxt == "" {`
`682`		`- assert.Nil(t, err)`
	`682`	`+ assert.NoError(t, err)`
`683`	`683`	`} else {`
`684`	`684`	`assert.EqualError(t, err, test.errorTxt)`
`685`	`685`	`}`