From b7115183758af64409e30ee6d37831fca0eba744 Mon Sep 17 00:00:00 2001
From: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Date: Tue, 18 Jun 2024 10:55:42 -0400
Subject: [PATCH] Remove unnecessary .nocover files (#5642)

## Which problem is this PR solving?
- Part of #5068
- Looks like test were added but `.nocover` files not removed.

## Description of the changes
- Remove unnecessary .nocover files.

## How was this change tested?
- Check if this affects coverage in CI.

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
---
 cmd/all-in-one/.nocover                       |  1 -
 cmd/all-in-one/all_in_one_test.go             | 66 ++++++++-----------
 cmd/all-in-one/package_test.go                | 14 ++++
 cmd/anonymizer/app/anonymizer/.nocover        |  1 -
 cmd/anonymizer/app/anonymizer/package_test.go | 14 ++++
 cmd/internal/flags/.nocover                   |  2 -
 cmd/internal/flags/admin.go                   |  6 +-
 cmd/internal/flags/admin_test.go              | 14 ++--
 cmd/internal/flags/package_test.go            | 14 ++++
 9 files changed, 81 insertions(+), 51 deletions(-)
 delete mode 100644 cmd/all-in-one/.nocover
 create mode 100644 cmd/all-in-one/package_test.go
 delete mode 100644 cmd/anonymizer/app/anonymizer/.nocover
 create mode 100644 cmd/anonymizer/app/anonymizer/package_test.go
 delete mode 100644 cmd/internal/flags/.nocover
 create mode 100644 cmd/internal/flags/package_test.go

diff --git a/cmd/all-in-one/.nocover b/cmd/all-in-one/.nocover
deleted file mode 100644
index 8ab06f28b67..00000000000
--- a/cmd/all-in-one/.nocover
+++ /dev/null
@@ -1 +0,0 @@
-main is not testable
diff --git a/cmd/all-in-one/all_in_one_test.go b/cmd/all-in-one/all_in_one_test.go
index f5186001976..95b7ea9bac8 100644
--- a/cmd/all-in-one/all_in_one_test.go
+++ b/cmd/all-in-one/all_in_one_test.go
@@ -75,7 +75,10 @@ func healthCheck(t *testing.T) {
 	require.Eventuallyf(
 		t,
 		func() bool {
-			_, err := http.Get(queryAddr + "/")
+			resp, err := http.Get(queryAddr + "/")
+			if err == nil {
+				resp.Body.Close()
+			}
 			return err == nil
 		},
 		10*time.Second,
@@ -85,14 +88,23 @@ func healthCheck(t *testing.T) {
 	t.Logf("Server detected at %s", queryAddr)
 }
 
-func checkWebUI(t *testing.T) {
-	resp, err := http.Get(queryAddr + "/")
+func httpGet(t *testing.T, url string) (*http.Response, []byte) {
+	req, err := http.NewRequest(http.MethodGet, url, nil)
+	require.NoError(t, err)
+	req.Close = true // avoid persistent connections which leak goroutines
+
+	resp, err := httpClient.Do(req)
 	require.NoError(t, err)
-	require.NotNil(t, resp)
 	defer resp.Body.Close()
-	assert.Equal(t, http.StatusOK, resp.StatusCode)
+
 	bodyBytes, err := io.ReadAll(resp.Body)
 	require.NoError(t, err)
+
+	return resp, bodyBytes
+}
+
+func checkWebUI(t *testing.T) {
+	_, bodyBytes := httpGet(t, queryAddr+"/")
 	body := string(bodyBytes)
 	t.Run("Static_files", func(t *testing.T) {
 		pattern := regexp.MustCompile(`<link rel="shortcut icon"[^<]+href="(.*?)"`)
@@ -115,12 +127,7 @@ func checkWebUI(t *testing.T) {
 func createTrace(t *testing.T) {
 	// Since all requests to query service are traces, creating a new trace
 	// is simply a matter of querying one of the endpoints.
-	req, err := http.NewRequest(http.MethodGet, queryAddr+getServicesURL, nil)
-	require.NoError(t, err)
-
-	resp, err := httpClient.Do(req)
-	require.NoError(t, err)
-	defer resp.Body.Close()
+	resp, _ := httpGet(t, queryAddr+getServicesURL)
 	traceResponse := resp.Header.Get("traceresponse")
 	// Expecting: [version] [trace-id] [child-id] [trace-flags]
 	parts := strings.Split(traceResponse, "-")
@@ -134,26 +141,18 @@ type response struct {
 }
 
 func getAPITrace(t *testing.T) {
-	req, err := http.NewRequest(http.MethodGet, queryAddr+getTraceURL+traceID, nil)
-	require.NoError(t, err)
-
 	var queryResponse response
-
 	for i := 0; i < 20; i++ {
-		resp, err := httpClient.Do(req)
-		require.NoError(t, err)
-
-		body, _ := io.ReadAll(resp.Body)
-
-		err = json.Unmarshal(body, &queryResponse)
-		require.NoError(t, err)
-		resp.Body.Close()
+		_, body := httpGet(t, queryAddr+getTraceURL+traceID)
+		require.NoError(t, json.Unmarshal(body, &queryResponse))
 
 		if len(queryResponse.Data) == 1 {
 			break
 		}
+		t.Logf("Trace %s not found (yet)", traceID)
 		time.Sleep(time.Second)
 	}
+
 	require.Len(t, queryResponse.Data, 1)
 	require.Len(t, queryResponse.Data[0].Spans, 1)
 }
@@ -163,35 +162,22 @@ func getSamplingStrategy(t *testing.T) {
 	if os.Getenv("SKIP_SAMPLING") == "true" {
 		t.Skip("skipping sampling strategy check because SKIP_SAMPLING=true is set")
 	}
-	req, err := http.NewRequest(http.MethodGet, agentAddr+getSamplingStrategyURL, nil)
-	require.NoError(t, err)
-
-	resp, err := httpClient.Do(req)
-	require.NoError(t, err)
-
-	body, _ := io.ReadAll(resp.Body)
+	_, body := httpGet(t, agentAddr+getSamplingStrategyURL)
 
 	var queryResponse api_v2.SamplingStrategyResponse
-	err = jsonpb.Unmarshal(bytes.NewReader(body), &queryResponse)
-	require.NoError(t, err)
-	resp.Body.Close()
+	require.NoError(t, jsonpb.Unmarshal(bytes.NewReader(body), &queryResponse))
 
 	assert.NotNil(t, queryResponse.ProbabilisticSampling)
 	assert.EqualValues(t, 1.0, queryResponse.ProbabilisticSampling.SamplingRate)
 }
 
 func getServicesAPIV3(t *testing.T) {
-	req, err := http.NewRequest(http.MethodGet, queryAddr+getServicesAPIV3URL, nil)
-	require.NoError(t, err)
-	resp, err := httpClient.Do(req)
-	require.NoError(t, err)
-	body, _ := io.ReadAll(resp.Body)
+	_, body := httpGet(t, queryAddr+getServicesAPIV3URL)
 
 	var servicesResponse struct {
 		Services []string
 	}
-	err = json.Unmarshal(body, &servicesResponse)
-	require.NoError(t, err)
+	require.NoError(t, json.Unmarshal(body, &servicesResponse))
 	require.Len(t, servicesResponse.Services, 1)
 	assert.Contains(t, servicesResponse.Services[0], "jaeger")
 }
diff --git a/cmd/all-in-one/package_test.go b/cmd/all-in-one/package_test.go
new file mode 100644
index 00000000000..3694f7ca674
--- /dev/null
+++ b/cmd/all-in-one/package_test.go
@@ -0,0 +1,14 @@
+// Copyright (c) 2024 The Jaeger Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"testing"
+
+	"github.com/jaegertracing/jaeger/pkg/testutils"
+)
+
+func TestMain(m *testing.M) {
+	testutils.VerifyGoLeaks(m)
+}
diff --git a/cmd/anonymizer/app/anonymizer/.nocover b/cmd/anonymizer/app/anonymizer/.nocover
deleted file mode 100644
index 5b583b79e93..00000000000
--- a/cmd/anonymizer/app/anonymizer/.nocover
+++ /dev/null
@@ -1 +0,0 @@
-non-critical test utility
diff --git a/cmd/anonymizer/app/anonymizer/package_test.go b/cmd/anonymizer/app/anonymizer/package_test.go
new file mode 100644
index 00000000000..a550d335fdd
--- /dev/null
+++ b/cmd/anonymizer/app/anonymizer/package_test.go
@@ -0,0 +1,14 @@
+// Copyright (c) 2024 The Jaeger Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package anonymizer
+
+import (
+	"testing"
+
+	"github.com/jaegertracing/jaeger/pkg/testutils"
+)
+
+func TestMain(m *testing.M) {
+	testutils.VerifyGoLeaks(m)
+}
diff --git a/cmd/internal/flags/.nocover b/cmd/internal/flags/.nocover
deleted file mode 100644
index 46911e9377b..00000000000
--- a/cmd/internal/flags/.nocover
+++ /dev/null
@@ -1,2 +0,0 @@
-FIXME
-
diff --git a/cmd/internal/flags/admin.go b/cmd/internal/flags/admin.go
index ea4df675a04..974aafdaacb 100644
--- a/cmd/internal/flags/admin.go
+++ b/cmd/internal/flags/admin.go
@@ -170,6 +170,8 @@ func (s *AdminServer) registerPprofHandlers() {
 
 // Close stops the HTTP server
 func (s *AdminServer) Close() error {
-	_ = s.tlsCertWatcherCloser.Close()
-	return s.server.Shutdown(context.Background())
+	return errors.Join(
+		s.tlsCertWatcherCloser.Close(),
+		s.server.Shutdown(context.Background()),
+	)
 }
diff --git a/cmd/internal/flags/admin_test.go b/cmd/internal/flags/admin_test.go
index ba4a0e01e1f..87325346074 100644
--- a/cmd/internal/flags/admin_test.go
+++ b/cmd/internal/flags/admin_test.go
@@ -27,6 +27,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest"
 	"go.uber.org/zap/zaptest/observer"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
@@ -137,10 +138,8 @@ func TestAdminServerTLS(t *testing.T) {
 			v, command := config.Viperize(adminServer.AddFlags)
 			err := command.ParseFlags(test.serverTLSFlags)
 			require.NoError(t, err)
-			zapCore, _ := observer.New(zap.InfoLevel)
-			logger := zap.New(zapCore)
 
-			err = adminServer.initFromViper(v, logger)
+			err = adminServer.initFromViper(v, zaptest.NewLogger(t))
 			require.NoError(t, err)
 
 			adminServer.Serve()
@@ -148,6 +147,7 @@ func TestAdminServerTLS(t *testing.T) {
 
 			clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop())
 			require.NoError(t, err0)
+			defer test.clientTLS.Close()
 			dialer := &net.Dialer{Timeout: 2 * time.Second}
 			conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorAdminHTTP), clientTLSCfg)
 			require.NoError(t, clientError)
@@ -158,9 +158,13 @@ func TestAdminServerTLS(t *testing.T) {
 					TLSClientConfig: clientTLSCfg,
 				},
 			}
-
-			response, requestError := client.Get(fmt.Sprintf("https://localhost:%d", ports.CollectorAdminHTTP))
+			url := fmt.Sprintf("https://localhost:%d", ports.CollectorAdminHTTP)
+			req, err := http.NewRequest(http.MethodGet, url, nil)
+			require.NoError(t, err)
+			req.Close = true // avoid persistent connections which leak goroutines
+			response, requestError := client.Do(req)
 			require.NoError(t, requestError)
+			defer response.Body.Close()
 			require.NotNil(t, response)
 		})
 	}
diff --git a/cmd/internal/flags/package_test.go b/cmd/internal/flags/package_test.go
new file mode 100644
index 00000000000..da12c0031e5
--- /dev/null
+++ b/cmd/internal/flags/package_test.go
@@ -0,0 +1,14 @@
+// Copyright (c) 2024 The Jaeger Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package flags
+
+import (
+	"testing"
+
+	"github.com/jaegertracing/jaeger/pkg/testutils"
+)
+
+func TestMain(m *testing.M) {
+	testutils.VerifyGoLeaks(m)
+}