golangci-lint: enable and fix thelper linter

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

.golangci.yml

@@ -102,6 +102,9 @@ linters:
     # Checks usage of github.com/stretchr/testify.
     - testifylint
 
+    # Thelper detects tests helpers which is not start with t.Helper() method.
+    - thelper
+
     # TODO consider adding more linters, cf. https://olegk.dev/go-linters-configuration-the-right-version
 
 linters-settings:

cmd/agent/app/agent_test.go

@@ -72,6 +72,7 @@ func TestAgentMetricsEndpoint(t *testing.T) {
 }
 
 func withRunningAgent(t *testing.T, testcase func(string, chan error)) {
+	t.Helper()
 	resetDefaultPrometheusRegistry()
 	cfg := Builder{
 		Processors: []ProcessorConfiguration{

cmd/agent/app/configmanager/grpc/manager_test.go

@@ -56,6 +56,7 @@ func (*mockSamplingHandler) GetSamplingStrategy(context.Context, *api_v2.Samplin
 }
 
 func initializeGRPCTestServer(t *testing.T, beforeServe func(server *grpc.Server)) (*grpc.Server, net.Addr) {
+	t.Helper()
 	server := grpc.NewServer()
 	lis, err := net.Listen("tcp", "localhost:0")
 	require.NoError(t, err)

cmd/agent/app/processors/thrift_processor_test.go

@@ -44,6 +44,7 @@ var (
 )
 
 func createProcessor(t *testing.T, mFactory metrics.Factory, tFactory thrift.TProtocolFactory, handler AgentProcessor) (string, Processor) {
+	t.Helper()
 	transport, err := thriftudp.NewTUDPServerTransport("127.0.0.1:0")
 	require.NoError(t, err)
 
@@ -70,6 +71,7 @@ func createProcessor(t *testing.T, mFactory metrics.Factory, tFactory thrift.TPr
 
 // revive:disable-next-line function-result-limit
 func initCollectorAndReporter(t *testing.T) (*metricstest.Factory, *testutils.GrpcCollector, reporter.Reporter, *grpc.ClientConn) {
+	t.Helper()
 	grpcCollector := testutils.StartGRPCCollector(t)
 	conn, err := grpc.NewClient(grpcCollector.Listener().Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
@@ -177,6 +179,7 @@ func TestJaegerProcessor(t *testing.T) {
 }
 
 func assertJaegerProcessorCorrectness(t *testing.T, collector *testutils.GrpcCollector, metricsFactory *metricstest.Factory) {
+	t.Helper()
 	sizeF := func() int {
 		return len(collector.GetJaegerBatches())
 	}
@@ -187,6 +190,7 @@ func assertJaegerProcessorCorrectness(t *testing.T, collector *testutils.GrpcCol
 }
 
 func assertZipkinProcessorCorrectness(t *testing.T, collector *testutils.GrpcCollector, metricsFactory *metricstest.Factory) {
+	t.Helper()
 	sizeF := func() int {
 		return len(collector.GetJaegerBatches())
 	}
@@ -205,6 +209,7 @@ func assertCollectorReceivedData(
 	nameF func() string,
 	format string,
 ) {
+	t.Helper()
 	require.Eventually(t,
 		func() bool { return sizeF() == 1 },
 		5*time.Second,

cmd/agent/app/reporter/client_metrics_test.go

@@ -28,6 +28,7 @@ type clientMetricsTest struct {
 }
 
 func (tr *clientMetricsTest) assertLog(t *testing.T, msg, clientUUID string) {
+	t.Helper()
 	logs := tr.logs.FilterMessageSnippet(msg)
 	if clientUUID == "" {
 		assert.Equal(t, 0, logs.Len(), "not expecting log '%s", msg)

cmd/agent/app/reporter/grpc/builder_test.go

@@ -365,6 +365,7 @@ func (f *fakeInterceptor) intercept(ctx context.Context, method string, req, rep
 }
 
 func (f *fakeInterceptor) assertCalled(t *testing.T) {
+	t.Helper()
 	assert.True(t, f.isCalled)
 }
 

cmd/agent/app/reporter/grpc/collector_proxy_test.go

@@ -63,6 +63,7 @@ func TestMultipleCollectors(t *testing.T) {
 }
 
 func initializeGRPCTestServer(t *testing.T, beforeServe func(server *grpc.Server), opts ...grpc.ServerOption) (*grpc.Server, net.Addr) {
+	t.Helper()
 	server := grpc.NewServer(opts...)
 	lis, err := net.Listen("tcp", "localhost:0")
 	require.NoError(t, err)

cmd/agent/app/servers/thriftudp/transport_test.go

@@ -219,6 +219,7 @@ func TestResetInFlush(t *testing.T) {
 }
 
 func withLocalServer(t *testing.T, f func(addr string)) {
+	t.Helper()
 	conn, err := net.ListenUDP(localListenAddr.Network(), localListenAddr)
 	require.NoError(t, err, "ListenUDP failed")
 

cmd/agent/app/testutils/mock_grpc_collector.go

@@ -28,6 +28,7 @@ var _ api_v2.CollectorServiceServer = (*GrpcCollector)(nil)
 
 // StartGRPCCollector starts GRPC collector on a random port
 func StartGRPCCollector(t *testing.T) *GrpcCollector {
+	t.Helper()
 	server := grpc.NewServer()
 	lis, err := net.Listen("tcp", "localhost:0")
 	require.NoError(t, err)

cmd/all-in-one/all_in_one_test.go

@@ -65,6 +65,7 @@ func TestAllInOne(t *testing.T) {
 }
 
 func healthCheck(t *testing.T) {
+	t.Helper()
 	require.Eventuallyf(t,
 		func() bool {
 			resp, err := http.Get(queryAddr + "/")
@@ -100,6 +101,7 @@ func healthCheckV2(t *testing.T) {
 }
 
 func httpGet(t *testing.T, url string) (*http.Response, []byte) {
+	t.Helper()
 	t.Logf("Executing HTTP GET %s", url)
 	req, err := http.NewRequest(http.MethodGet, url, nil)
 	require.NoError(t, err)

cmd/anonymizer/app/query/query_test.go

@@ -54,6 +54,7 @@ type testServer struct {
 }
 
 func newTestServer(t *testing.T) *testServer {
+	t.Helper()
 	spanReader := &spanstoremocks.Reader{}
 	metricsReader, err := disabled.NewMetricsReader()
 	require.NoError(t, err)

cmd/anonymizer/app/uiconv/extractor_test.go

@@ -90,6 +90,7 @@ func TestExtractorTraceScanError(t *testing.T) {
 }
 
 func loadJSON(t *testing.T, fileName string, i any) {
+	t.Helper()
 	b, err := os.ReadFile(fileName)
 	require.NoError(t, err)
 	err = json.Unmarshal(b, i)

cmd/collector/app/handler/grpc_handler_test.go

@@ -85,6 +85,7 @@ func (*mockSpanProcessor) Close() error {
 }
 
 func initializeGRPCTestServer(t *testing.T, beforeServe func(s *grpc.Server)) (*grpc.Server, net.Addr) {
+	t.Helper()
 	server := grpc.NewServer()
 	beforeServe(server)
 	lis, err := net.Listen("tcp", "localhost:0")
@@ -97,6 +98,7 @@ func initializeGRPCTestServer(t *testing.T, beforeServe func(s *grpc.Server)) (*
 }
 
 func newClient(t *testing.T, addr net.Addr) (api_v2.CollectorServiceClient, *grpc.ClientConn) {
+	t.Helper()
 	conn, err := grpc.NewClient(addr.String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
 	require.NoError(t, err)
 	return api_v2.NewCollectorServiceClient(conn), conn
@@ -203,7 +205,7 @@ func TestPostSpansWithError(t *testing.T) {
 }
 
 // withMetadata returns a Context with metadata for outbound (client) calls
-func withMetadata(ctx context.Context, headerName, headerValue string, t *testing.T) context.Context {
+func withMetadata(ctx context.Context, t *testing.T, headerName, headerValue string) context.Context {
 	t.Helper()
 
 	md := metadata.New(map[string]string{headerName: headerValue})
@@ -228,15 +230,15 @@ func TestPostTenantedSpans(t *testing.T) {
 	client, conn := newClient(t, addr)
 	defer conn.Close()
 
-	ctxWithTenant := withMetadata(context.Background(), tenantHeader, dummyTenant, t)
+	ctxWithTenant := withMetadata(context.Background(), t, tenantHeader, dummyTenant)
 	ctxNoTenant := context.Background()
 	mdTwoTenants := metadata.Pairs()
 	mdTwoTenants.Set(tenantHeader, "a", "b")
 	ctxTwoTenants := metadata.NewOutgoingContext(context.Background(), mdTwoTenants)
-	ctxBadTenant := withMetadata(context.Background(), tenantHeader, "invalid-tenant", t)
+	ctxBadTenant := withMetadata(context.Background(), t, tenantHeader, "invalid-tenant")
 
-	withMetadata(context.Background(),
-		tenantHeader, dummyTenant, t)
+	withMetadata(context.Background(), t,
+		tenantHeader, dummyTenant)
 
 	tests := []struct {
 		name            string
@@ -304,7 +306,7 @@ func TestPostTenantedSpans(t *testing.T) {
 }
 
 // withIncomingMetadata returns a Context with metadata for a server to receive
-func withIncomingMetadata(ctx context.Context, headerName, headerValue string, t *testing.T) context.Context {
+func withIncomingMetadata(ctx context.Context, t *testing.T, headerName, headerValue string) context.Context {
 	t.Helper()
 
 	md := metadata.New(map[string]string{headerName: headerValue})
@@ -327,7 +329,7 @@ func TestGetTenant(t *testing.T) {
 	}{
 		{
 			name:     "valid tenant",
-			ctx:      withIncomingMetadata(context.TODO(), tenantHeader, "acme", t),
+			ctx:      withIncomingMetadata(context.TODO(), t, tenantHeader, "acme"),
 			mustFail: false,
 			tenant:   "acme",
 		},
@@ -345,7 +347,7 @@ func TestGetTenant(t *testing.T) {
 		},
 		{
 			name:     "invalid tenant",
-			ctx:      withIncomingMetadata(context.TODO(), tenantHeader, "an-invalid-tenant", t),
+			ctx:      withIncomingMetadata(context.TODO(), t, tenantHeader, "an-invalid-tenant"),
 			mustFail: true,
 			tenant:   "",
 		},

cmd/collector/app/span_processor_test.go

@@ -71,6 +71,7 @@ func TestBySvcMetrics(t *testing.T) {
 	}
 
 	testFn := func(t *testing.T, test TestCase) {
+		t.Helper()
 		mb := metricstest.NewFactory(time.Hour)
 		defer mb.Backend.Stop()
 		logger := zap.NewNop()

cmd/es-index-cleaner/app/index_filter_test.go

@@ -21,6 +21,7 @@ func TestIndexFilter_prefix(t *testing.T) {
 }
 
 func testIndexFilter(t *testing.T, prefix string) {
+	t.Helper()
 	time20200807 := time.Date(2020, time.August, 0o6, 0, 0, 0, 0, time.UTC).AddDate(0, 0, 1)
 	indices := []client.Index{
 		{

cmd/ingester/app/consumer/consumer_test.go

@@ -81,6 +81,7 @@ func newConsumer(
 	processor processor.SpanProcessor,
 	consumer consumer.Consumer,
 ) *Consumer {
+	t.Helper()
 	logger, _ := zap.NewDevelopment()
 	consumerParams := Params{
 		MetricsFactory:   metricsFactory,

cmd/internal/flags/service_test.go

@@ -93,6 +93,7 @@ func TestStartErrors(t *testing.T) {
 }
 
 func waitForEqual(t *testing.T, expected any, getter func() any) {
+	t.Helper()
 	for i := 0; i < 1000; i++ {
 		value := getter()
 		if reflect.DeepEqual(value, expected) {

cmd/internal/printconfig/command_test.go

@@ -54,6 +54,7 @@ func addFlags(flagSet *flag.FlagSet) {
 }
 
 func setConfig(t *testing.T) *viper.Viper {
+	t.Helper()
 	v, command := config.Viperize(addFlags, tenancy.AddFlags)
 	err := command.ParseFlags([]string{
 		"--test-plugin.binary=noop-test-plugin",
@@ -67,7 +68,8 @@ func setConfig(t *testing.T) *viper.Viper {
 	return v
 }
 
-func runPrintConfigCommand(v *viper.Viper, t *testing.T, allFlag bool) string {
+func runPrintConfigCommand(t *testing.T, v *viper.Viper, allFlag bool) string {
+	t.Helper()
 	buf := new(bytes.Buffer)
 	printCmd := Command(v)
 	printCmd.SetOut(buf)
@@ -105,7 +107,7 @@ func TestAllFlag(t *testing.T) {
 `
 
 	v := setConfig(t)
-	actual := runPrintConfigCommand(v, t, true)
+	actual := runPrintConfigCommand(t, v, true)
 	assert.Equal(t, expected, actual)
 }
 
@@ -124,7 +126,7 @@ func TestPrintConfigCommand(t *testing.T) {
 -----------------------------------------------------------------
 `
 	v := setConfig(t)
-	actual := runPrintConfigCommand(v, t, false)
+	actual := runPrintConfigCommand(t, v, false)
 	assert.Equal(t, expected, actual)
 }
 

cmd/jaeger/internal/exporters/storageexporter/exporter_test.go

@@ -163,6 +163,7 @@ func TestExporter(t *testing.T) {
 }
 
 func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
+	t.Helper()
 	telemetrySettings := component.TelemetrySettings{
 		Logger:         zaptest.NewLogger(t),
 		TracerProvider: nooptrace.NewTracerProvider(),

cmd/jaeger/internal/extension/jaegerstorage/config_test.go

@@ -15,6 +15,7 @@ import (
 )
 
 func loadConf(t *testing.T, config string) *confmap.Conf {
+	t.Helper()
 	d := t.TempDir()
 	f := filepath.Join(d, "config.yaml")
 	require.NoError(t, os.WriteFile(f, []byte(config), 0o644))

cmd/jaeger/internal/extension/jaegerstorage/extension_test.go

@@ -197,6 +197,7 @@ func TestMetricsStorageStartError(t *testing.T) {
 }
 
 func testElasticsearchOrOpensearch(t *testing.T, cfg Backend) {
+	t.Helper()
 	ext := makeStorageExtenion(t, &Config{
 		Backends: map[string]Backend{
 			"foo": cfg,
@@ -264,6 +265,7 @@ func noopTelemetrySettings() component.TelemetrySettings {
 }
 
 func makeStorageExtenion(t *testing.T, config *Config) component.Component {
+	t.Helper()
 	extensionFactory := NewFactory()
 	ctx := context.Background()
 	ext, err := extensionFactory.CreateExtension(ctx,
@@ -279,6 +281,7 @@ func makeStorageExtenion(t *testing.T, config *Config) component.Component {
 }
 
 func startStorageExtension(t *testing.T, memstoreName string, promstoreName string) component.Component {
+	t.Helper()
 	config := &Config{
 		Backends: map[string]Backend{
 			memstoreName: {

cmd/jaeger/internal/extension/remotesampling/extension_test.go

@@ -36,6 +36,7 @@ import (
 )
 
 func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
+	t.Helper()
 	telemetrySettings := component.TelemetrySettings{
 		Logger:         zaptest.NewLogger(t),
 		TracerProvider: nooptrace.NewTracerProvider(),
@@ -66,6 +67,7 @@ func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
 }
 
 func makeRemoteSamplingExtension(t *testing.T, cfg component.Config) component.Host {
+	t.Helper()
 	extensionFactory := NewFactory()
 	samplingExtension, err := extensionFactory.CreateExtension(
 		context.Background(),

cmd/jaeger/internal/integration/e2e_integration.go

@@ -61,6 +61,7 @@ type Binary struct {
 }
 
 func (b *Binary) Start(t *testing.T) {
+	t.Helper()
 	outFile, err := os.OpenFile(
 		filepath.Join(t.TempDir(), "output_logs.txt"),
 		os.O_CREATE|os.O_WRONLY,
@@ -113,6 +114,7 @@ func (b *Binary) Start(t *testing.T) {
 // it also initialize the SpanWriter and SpanReader below.
 // This function should be called before any of the tests start.
 func (s *E2EStorageIntegration) e2eInitialize(t *testing.T, storage string) {
+	t.Helper()
 	// set environment variable overrides
 	for key, value := range s.EnvVarOverrides {
 		t.Setenv(key, value)
@@ -160,6 +162,7 @@ func (s *E2EStorageIntegration) e2eInitialize(t *testing.T, storage string) {
 }
 
 func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {
+	t.Helper()
 	healthCheckEndpoint := s.HealthCheckEndpoint
 	if healthCheckEndpoint == "" {
 		healthCheckEndpoint = "http://localhost:13133/status"
@@ -212,11 +215,13 @@ func (s *E2EStorageIntegration) doHealthCheck(t *testing.T) bool {
 // e2eCleanUp closes the SpanReader and SpanWriter gRPC connection.
 // This function should be called after all the tests are finished.
 func (s *E2EStorageIntegration) e2eCleanUp(t *testing.T) {
+	t.Helper()
 	require.NoError(t, s.SpanReader.(io.Closer).Close())
 	require.NoError(t, s.SpanWriter.(io.Closer).Close())
 }
 
 func createStorageCleanerConfig(t *testing.T, configFile string, storage string) string {
+	t.Helper()
 	data, err := os.ReadFile(configFile)
 	require.NoError(t, err)
 	var config map[string]any
@@ -271,6 +276,7 @@ func createStorageCleanerConfig(t *testing.T, configFile string, storage string)
 }
 
 func purge(t *testing.T) {
+	t.Helper()
 	addr := fmt.Sprintf("http://0.0.0.0:%s%s", storagecleaner.Port, storagecleaner.URL)
 	t.Logf("Purging storage via %s", addr)
 	r, err := http.NewRequestWithContext(context.Background(), http.MethodPost, addr, nil)

cmd/jaeger/internal/integration/grpc_test.go

@@ -18,10 +18,12 @@ type GRPCStorageIntegration struct {
 }
 
 func (s *GRPCStorageIntegration) initialize(t *testing.T) {
+	t.Helper()
 	s.remoteStorage = integration.StartNewRemoteMemoryStorage(t)
 }
 
 func (s *GRPCStorageIntegration) cleanUp(t *testing.T) {
+	t.Helper()
 	s.remoteStorage.Close(t)
 	s.initialize(t)
 }