[chore]: enable len and empty rules from testifylint (#6125)

#### Description Testifylint is a linter that provides best practices with the use of testify. This PR enables [empty](https://github.com/Antonboom/testifylint?tab=readme-ov-file#empty) and [len](https://github.com/Antonboom/testifylint?tab=readme-ov-file#len) rules from [testifylint](https://github.com/Antonboom/testifylint) It's linter provided by golangci-lint. Here all available rules are activated except those who require to be fixed. This PR only fixes empty and len so the quantity of changes stays reasonnable for reviewers. --------- Signed-off-by: Matthieu MOREL <[email protected]> Co-authored-by: Damien Mathieu <[email protected]>
open-telemetry · Sep 20, 2024 · d295ae3 · d295ae3
1 parent b077d79
commit d295ae3
Show file tree

Hide file tree

Showing 18 changed files with 49 additions and 35 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -26,6 +26,7 @@ linters:
     - revive
     - staticcheck
     - tenv
+    - testifylint
     - typecheck
     - unconvert
     - unparam
@@ -243,3 +244,16 @@ linters-settings:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#waitgroup-by-value
       - name: waitgroup-by-value
         disabled: false
+  testifylint:
+    enable-all: true
+    disable:
+      - compares
+      - error-is-as
+      - error-nil
+      - expected-actual
+      - float-compare
+      - formatter
+      - go-require
+      - negative-positive
+      - nil-compare
+      - require-error
diff --git a/detectors/aws/ecs/ecs_test.go b/detectors/aws/ecs/ecs_test.go
@@ -187,7 +187,7 @@ func TestDetectCannotReadContainerName(t *testing.T) {
 	res, err := detector.Detect(context.Background())
 
 	assert.Equal(t, errCannotReadContainerName, err)
-	assert.Equal(t, 0, len(res.Attributes()))
+	assert.Empty(t, res.Attributes())
 }
 
 // returns empty resource when process is not running ECS.

diff --git a/detectors/aws/lambda/detector_test.go b/detectors/aws/lambda/detector_test.go
@@ -46,5 +46,5 @@ func TestReturnsIfNoEnvVars(t *testing.T) {
 	res, err := detector.Detect(context.Background())
 
 	assert.Equal(t, errNotOnLambda, err)
-	assert.Equal(t, 0, len(res.Attributes()))
+	assert.Empty(t, res.Attributes())
 }
diff --git a/instrgen/driver/instrgen_test.go b/instrgen/driver/instrgen_test.go
@@ -47,10 +47,10 @@ func TestCommands(t *testing.T) {
 func TestCallGraph(t *testing.T) {
 	cg := makeCallGraph("./testdata/dummy", "./...")
 	dumpCallGraph(cg)
-	assert.Equal(t, len(cg), 0, "callgraph should contain 0 elems")
+	assert.Empty(t, cg, "callgraph should contain 0 elems")
 	rf := makeRootFunctions("./testdata/dummy", "./...")
 	dumpRootFunctions(rf)
-	assert.Equal(t, len(rf), 0, "rootfunctions set should be empty")
+	assert.Empty(t, rf, "rootfunctions set should be empty")
 }
 
 func TestArgs(t *testing.T) {

diff --git a/instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda_test.go b/instrumentation/github.com/aws/aws-lambda-go/otellambda/lambda_test.go
@@ -124,7 +124,7 @@ func TestLambdaHandlerSignatures(t *testing.T) {
 			lambdaHandler := InstrumentHandler(testCase.handler)
 			handler := reflect.ValueOf(lambdaHandler)
 			resp := handler.Call(testCase.args)
-			assert.Equal(t, 2, len(resp))
+			assert.Len(t, resp, 2)
 			assert.Equal(t, testCase.expected, resp[1].Interface())
 		})
 	}
@@ -228,7 +228,7 @@ func TestHandlerInvokes(t *testing.T) {
 				args = append(args, reflect.ValueOf(testCase.input))
 			}
 			response := handler.Call(args)
-			assert.Equal(t, 2, len(response))
+			assert.Len(t, response, 2)
 			if testCase.expected.err != nil {
 				assert.Equal(t, testCase.expected.err, response[handlerType.NumOut()-1].Interface())
 			} else {

diff --git a/instrumentation/github.com/emicklei/go-restful/otelrestful/restful_test.go b/instrumentation/github.com/emicklei/go-restful/otelrestful/restful_test.go
@@ -209,7 +209,7 @@ func TestWithPublicEndpointFn(t *testing.T) {
 			},
 			spansAssert: func(t *testing.T, _ oteltrace.SpanContext, spans []sdktrace.ReadOnlySpan) {
 				require.Len(t, spans, 1)
-				require.Len(t, spans[0].Links(), 0, "should not contain link")
+				require.Empty(t, spans[0].Links(), "should not contain link")
 			},
 		},
 	} {

diff --git a/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go b/instrumentation/github.com/gin-gonic/gin/otelgin/test/gintrace_test.go
@@ -268,7 +268,7 @@ func TestWithFilter(t *testing.T) {
 		w := httptest.NewRecorder()
 
 		router.ServeHTTP(w, r)
-		assert.Len(t, sr.Ended(), 0)
+		assert.Empty(t, sr.Ended())
 	})
 
 	t.Run("custom filter not filtering route", func(t *testing.T) {
@@ -302,7 +302,7 @@ func TestWithGinFilter(t *testing.T) {
 		w := httptest.NewRecorder()
 
 		router.ServeHTTP(w, r)
-		assert.Len(t, sr.Ended(), 0)
+		assert.Empty(t, sr.Ended())
 	})
 
 	t.Run("custom filter not filtering route", func(t *testing.T) {

diff --git a/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go b/instrumentation/github.com/gorilla/mux/otelmux/test/mux_test.go
@@ -243,7 +243,7 @@ func TestWithPublicEndpointFn(t *testing.T) {
 			},
 			spansAssert: func(t *testing.T, _ trace.SpanContext, spans []sdktrace.ReadOnlySpan) {
 				require.Len(t, spans, 1)
-				require.Len(t, spans[0].Links(), 0, "should not contain link")
+				require.Empty(t, spans[0].Links(), "should not contain link")
 			},
 		},
 	}

diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_stats_handler_test.go
@@ -115,25 +115,25 @@ func TestStatsHandler(t *testing.T) {
 				})
 			} else {
 				t.Run("ClientSpans", func(t *testing.T) {
-					require.Len(t, clientSR.Ended(), 0)
+					require.Empty(t, clientSR.Ended())
 				})
 
 				t.Run("ClientMetrics", func(t *testing.T) {
 					rm := metricdata.ResourceMetrics{}
 					err := clientMetricReader.Collect(context.Background(), &rm)
 					assert.NoError(t, err)
-					require.Len(t, rm.ScopeMetrics, 0)
+					require.Empty(t, rm.ScopeMetrics)
 				})
 
 				t.Run("ServerSpans", func(t *testing.T) {
-					require.Len(t, serverSR.Ended(), 0)
+					require.Empty(t, serverSR.Ended())
 				})
 
 				t.Run("ServerMetrics", func(t *testing.T) {
 					rm := metricdata.ResourceMetrics{}
 					err := serverMetricReader.Collect(context.Background(), &rm)
 					assert.NoError(t, err)
-					require.Len(t, rm.ScopeMetrics, 0)
+					require.Empty(t, rm.ScopeMetrics)
 				})
 			}
 		})

diff --git a/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/test/clienttrace_test.go
@@ -263,7 +263,7 @@ func TestEndBeforeStartWithoutSubSpansDoesNotPanic(t *testing.T) {
 
 	// no spans created because we were just using background context without span
 	// and Start wasn't called which would have started a span
-	require.Len(t, sr.Ended(), 0)
+	require.Empty(t, sr.Ended())
 }
 
 type clientTraceTestFixture struct {
@@ -306,7 +306,7 @@ func TestWithoutSubSpans(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, resp.Body.Close())
 	// no spans created because we were just using background context without span
-	require.Len(t, fixture.SpanRecorder.Ended(), 0)
+	require.Empty(t, fixture.SpanRecorder.Ended())
 
 	// Start again with a "real" span in the context, now tracing should add
 	// events and annotations.
@@ -443,7 +443,7 @@ func TestWithoutHeaders(t *testing.T) {
 	recSpan := fixture.SpanRecorder.Ended()[0]
 
 	gotAttributes := recSpan.Attributes()
-	require.Len(t, gotAttributes, 0)
+	require.Empty(t, gotAttributes)
 }
 
 func TestWithInsecureHeaders(t *testing.T) {

diff --git a/instrumentation/net/http/otelhttp/test/client_test.go b/instrumentation/net/http/otelhttp/test/client_test.go
@@ -68,7 +68,7 @@ func TestConvenienceWrappers(t *testing.T) {
 	res.Body.Close()
 
 	spans := sr.Ended()
-	require.Equal(t, 4, len(spans))
+	require.Len(t, spans, 4)
 	assert.Equal(t, "HTTP GET", spans[0].Name())
 	assert.Equal(t, "HTTP HEAD", spans[1].Name())
 	assert.Equal(t, "HTTP POST", spans[2].Name())
@@ -100,7 +100,7 @@ func TestClientWithTraceContext(t *testing.T) {
 	span.End()
 
 	spans := sr.Ended()
-	require.Equal(t, 2, len(spans))
+	require.Len(t, spans, 2)
 	assert.Equal(t, "HTTP GET", spans[0].Name())
 	assert.Equal(t, "http requests", spans[1].Name())
 	assert.NotEmpty(t, spans[0].Parent().SpanID())

diff --git a/instrumentation/net/http/otelhttp/test/handler_test.go b/instrumentation/net/http/otelhttp/test/handler_test.go
@@ -432,7 +432,7 @@ func TestWithPublicEndpointFn(t *testing.T) {
 			},
 			spansAssert: func(t *testing.T, _ trace.SpanContext, spans []sdktrace.ReadOnlySpan) {
 				require.Len(t, spans, 1)
-				require.Len(t, spans[0].Links(), 0, "should not contain link")
+				require.Empty(t, spans[0].Links(), "should not contain link")
 			},
 		},
 	} {

diff --git a/processors/minsev/minsev_test.go b/processors/minsev/minsev_test.go
@@ -123,7 +123,7 @@ func TestLogProcessorOnEmit(t *testing.T) {
 			r.SetSeverity(sev)
 			assert.NoError(t, p.OnEmit(ctx, r), sev.String())
 
-			if !assert.Lenf(t, wrapped.OnEmitCalls, 0, "Record with severity %s passed-through", sev) {
+			if !assert.Emptyf(t, wrapped.OnEmitCalls, "Record with severity %s passed-through", sev) {
 				wrapped.Reset()
 			}
 		}
@@ -159,7 +159,7 @@ func TestLogProcessorEnabled(t *testing.T) {
 			param.SetSeverity(sev)
 			assert.False(t, p.Enabled(ctx, param), sev.String())
 
-			if !assert.Lenf(t, wrapped.EnabledCalls, 0, "Record with severity %s passed-through", sev) {
+			if !assert.Emptyf(t, wrapped.EnabledCalls, "Record with severity %s passed-through", sev) {
 				wrapped.Reset()
 			}
 		}
@@ -182,7 +182,7 @@ func TestLogProcessorEnabled(t *testing.T) {
 		params.SetSeverity(api.SeverityError)
 		assert.True(t, p.Enabled(ctx, *params))
 
-		assert.Len(t, wrapped.EnabledCalls, 0)
+		assert.Empty(t, wrapped.EnabledCalls)
 	})
 }
 

diff --git a/samplers/aws/xray/internal/manifest_test.go b/samplers/aws/xray/internal/manifest_test.go
@@ -476,7 +476,7 @@ func TestRefreshManifestMissingRuleName(t *testing.T) {
 	require.NoError(t, err)
 
 	// assert on rule not added
-	require.Len(t, m.Rules, 0)
+	require.Empty(t, m.Rules)
 }
 
 // assert that rule with version greater than one does not update to the manifest.
@@ -520,7 +520,7 @@ func TestRefreshManifestIncorrectVersion(t *testing.T) {
 	require.NoError(t, err)
 
 	// assert on rule not added
-	require.Len(t, m.Rules, 0)
+	require.Empty(t, m.Rules)
 }
 
 // assert that 1 valid and 1 invalid rule update only valid rule gets stored to the manifest.

diff --git a/samplers/probability/consistent/sampler_test.go b/samplers/probability/consistent/sampler_test.go
@@ -212,9 +212,9 @@ func TestSamplerBehavior(t *testing.T) {
 						}
 
 						if test.hasErrors {
-							require.Less(t, 0, len(handler.Errors()))
+							require.NotEmpty(t, handler.Errors())
 						} else {
-							require.Equal(t, 0, len(handler.Errors()))
+							require.Empty(t, handler.Errors())
 						}
 					}
 				})

diff --git a/samplers/probability/consistent/statistical_test.go b/samplers/probability/consistent/statistical_test.go
@@ -256,7 +256,7 @@ func sampleTrials(t *testing.T, prob float64, degrees testDegrees, upperP pValue
 	//
 	// The test specification ensures the test ensures there are
 	// at least 20 expected items per category in these tests.
-	require.NotEqual(t, 0, len(counts))
+	require.NotEmpty(t, counts)
 
 	if degrees == 2 {
 		// Note: because the test is probabilistic, we can't be

diff --git a/zpages/bucket_test.go b/zpages/bucket_test.go
@@ -89,5 +89,5 @@ func TestBucketZeroCapacity(t *testing.T) {
 	assert.Equal(t, 0, bkt.len())
 	bkt.add(&testSpan{endTime: time.Unix(1, 0)})
 	assert.Equal(t, 0, bkt.len())
-	assert.Len(t, bkt.spans(), 0)
+	assert.Empty(t, bkt.spans())
 }
diff --git a/zpages/spanprocessor_test.go b/zpages/spanprocessor_test.go
@@ -52,12 +52,12 @@ func TestSpanProcessor(t *testing.T) {
 		assert.Same(t, spans[i], activeSpans[i])
 	}
 	// No ended spans so there will be no error, no latency samples.
-	assert.Len(t, zsp.errorSpans(spanName), 0)
+	assert.Empty(t, zsp.errorSpans(spanName))
 	for i := 0; i < defaultBoundaries.numBuckets(); i++ {
-		assert.Len(t, zsp.spansByLatency(spanName, i), 0)
+		assert.Empty(t, zsp.spansByLatency(spanName, i))
 	}
 	spansPM := zsp.spansPerMethod()
-	require.Equal(t, 1, len(spansPM))
+	require.Len(t, spansPM, 1)
 	assert.Equal(t, numSpans, spansPM[spanName].activeSpans)
 
 	// End all Spans, they will end pretty fast, so we can only check that we have at least one in
@@ -66,7 +66,7 @@ func TestSpanProcessor(t *testing.T) {
 		s.End()
 	}
 	// Test that no more active spans.
-	assert.Len(t, zsp.activeSpans(spanName), 0)
+	assert.Empty(t, zsp.activeSpans(spanName))
 	assert.LessOrEqual(t, 1, len(zsp.errorSpans(spanName)))
 	numLatencySamples := 0
 	for i := 0; i < defaultBoundaries.numBuckets(); i++ {
@@ -158,13 +158,13 @@ func TestSpanProcessorNegativeLatency(t *testing.T) {
 	zsp.OnStart(context.Background(), ts)
 
 	spansPM := zsp.spansPerMethod()
-	require.Equal(t, 1, len(spansPM))
+	require.Len(t, spansPM, 1)
 	assert.Equal(t, 1, spansPM["test"].activeSpans)
 
 	zsp.OnEnd(ts)
 
 	spansPM = zsp.spansPerMethod()
-	require.Equal(t, 1, len(spansPM))
+	require.Len(t, spansPM, 1)
 	assert.Equal(t, 1, spansPM["test"].latencySpans[0])
 }