Error checking on unit tests (#2959)

* Error checking on unit tests

* Add back license text

* Retrigger CI

* Wrap defer calls with `func`
Arguments are evaluated immediately on defer calls

* Update config/configauth/oidc_server_test.go

* Use t.Cleanup

* Change ioutil.ReadAll by resp.Body.Close()

* Retrigger CI (flaky tests)

config/configauth/oidc_server_test.go

@@ -49,14 +49,21 @@ func newOIDCServer() (*oidcServer, error) {
 
 	mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, req *http.Request) {
 		w.Header().Set("Content-Type", "application/json; charset=utf-8")
-		json.NewEncoder(w).Encode(map[string]interface{}{
+		err := json.NewEncoder(w).Encode(map[string]interface{}{
 			"issuer":   server.URL,
 			"jwks_uri": fmt.Sprintf("%s/.well-known/jwks.json", server.URL),
 		})
+		if err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
 	})
 	mux.HandleFunc("/.well-known/jwks.json", func(w http.ResponseWriter, req *http.Request) {
 		w.Header().Set("Content-Type", "application/json; charset=utf-8")
-		json.NewEncoder(w).Encode(jwks)
+		if err := json.NewEncoder(w).Encode(jwks); err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
 	})
 
 	privateKey, err := createPrivateKey()

exporter/exporterhelper/queued_retry_test.go

@@ -327,7 +327,7 @@ func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
 	require.NoError(t, be.Start(context.Background(), componenttest.NewNopHost()))
 
 	for i := 0; i < 7; i++ {
-		be.sender.send(newErrorRequest(context.Background()))
+		require.NoError(t, be.sender.send(newErrorRequest(context.Background())))
 	}
 	checkValueForProducer(t, defaultExporterTags, int64(7), "exporter/queue_size")
 

exporter/jaegerexporter/exporter_test.go

@@ -237,7 +237,7 @@ func TestMutualTLS(t *testing.T) {
 	require.NoError(t, err)
 	err = exporter.Start(context.Background(), nil)
 	require.NoError(t, err)
-	defer exporter.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, exporter.Shutdown(context.Background())) })
 
 	traceID := pdata.NewTraceID([16]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15})
 	spanID := pdata.NewSpanID([8]byte{0, 1, 2, 3, 4, 5, 6, 7})
@@ -282,8 +282,8 @@ func TestConnectionStateChange(t *testing.T) {
 		wg.Done()
 	})
 
-	sender.start(context.Background(), componenttest.NewNopHost())
-	defer sender.shutdown(context.Background())
+	require.NoError(t, sender.start(context.Background(), componenttest.NewNopHost()))
+	t.Cleanup(func() { require.NoError(t, sender.shutdown(context.Background())) })
 	wg.Wait() // wait for the initial state to be propagated
 
 	// test

exporter/prometheusexporter/collector_test.go

@@ -152,7 +152,7 @@ func TestCollectMetricsLabelSanitize(t *testing.T) {
 		require.Contains(t, m.Desc().String(), "variableLabels: [label_1 label_2]")
 
 		pbMetric := io_prometheus_client.Metric{}
-		m.Write(&pbMetric)
+		require.NoError(t, m.Write(&pbMetric))
 
 		labelsKeys := map[string]string{"label_1": "1", "label_2": "2"}
 		for _, l := range pbMetric.Label {
@@ -331,7 +331,7 @@ func TestCollectMetrics(t *testing.T) {
 					require.Contains(t, m.Desc().String(), "variableLabels: [label_1 label_2]")
 
 					pbMetric := io_prometheus_client.Metric{}
-					m.Write(&pbMetric)
+					require.NoError(t, m.Write(&pbMetric))
 
 					labelsKeys := map[string]string{"label_1": "1", "label_2": "2"}
 					for _, l := range pbMetric.Label {
@@ -460,7 +460,7 @@ func TestAccumulateHistograms(t *testing.T) {
 					require.Contains(t, m.Desc().String(), "variableLabels: [label_1 label_2]")
 
 					pbMetric := io_prometheus_client.Metric{}
-					m.Write(&pbMetric)
+					require.NoError(t, m.Write(&pbMetric))
 
 					labelsKeys := map[string]string{"label_1": "1", "label_2": "2"}
 					for _, l := range pbMetric.Label {
@@ -566,7 +566,7 @@ func TestAccumulateSummary(t *testing.T) {
 					require.Contains(t, m.Desc().String(), "variableLabels: [label_1 label_2]")
 
 					pbMetric := io_prometheus_client.Metric{}
-					m.Write(&pbMetric)
+					require.NoError(t, m.Write(&pbMetric))
 
 					labelsKeys := map[string]string{"label_1": "1", "label_2": "2"}
 					for _, l := range pbMetric.Label {

exporter/prometheusexporter/end_to_end_test.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	promconfig "github.com/prometheus/prometheus/config"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	"gopkg.in/yaml.v2"
 
@@ -49,7 +50,8 @@ func TestEndToEndSummarySupport(t *testing.T) {
 			return
 		case waitForScrape <- true:
 			// Serve back the metrics as if they were from DropWizard.
-			rw.Write([]byte(dropWizardResponse))
+			_, err := rw.Write([]byte(dropWizardResponse))
+			require.NoError(t, err)
 		}
 	}))
 	defer dropWizardServer.Close()
@@ -80,7 +82,7 @@ func TestEndToEndSummarySupport(t *testing.T) {
 	if err = exporter.Start(ctx, nil); err != nil {
 		t.Fatalf("Failed to start the Prometheus receiver: %v", err)
 	}
-	defer exporter.Shutdown(ctx)
+	t.Cleanup(func() { require.NoError(t, exporter.Shutdown(ctx)) })
 
 	// 3. Create the Prometheus receiver scraping from the DropWizard mock server and
 	// it'll feed scraped and converted metrics then pass them to the Prometheus exporter.
@@ -118,7 +120,7 @@ func TestEndToEndSummarySupport(t *testing.T) {
 	if err = prometheusReceiver.Start(ctx, nil); err != nil {
 		t.Fatalf("Failed to start the Prometheus receiver: %v", err)
 	}
-	defer prometheusReceiver.Shutdown(ctx)
+	t.Cleanup(func() { require.NoError(t, prometheusReceiver.Shutdown(ctx)) })
 
 	// 4. Scrape from the Prometheus exporter to ensure that we export summary metrics
 	// We shall let the Prometheus exporter scrape the DropWizard mock server, at least 9 times.

exporter/prometheusexporter/prometheus_test.go

@@ -119,7 +119,8 @@ func TestPrometheusExporter_endToEnd(t *testing.T) {
 	t.Cleanup(func() {
 		require.NoError(t, exp.Shutdown(context.Background()))
 		// trigger a get so that the server cleans up our keepalive socket
-		http.Get("http://localhost:7777/metrics")
+		_, err = http.Get("http://localhost:7777/metrics")
+		require.NoError(t, err)
 	})
 
 	assert.NotNil(t, exp)
@@ -196,7 +197,8 @@ func TestPrometheusExporter_endToEndWithTimestamps(t *testing.T) {
 	t.Cleanup(func() {
 		require.NoError(t, exp.Shutdown(context.Background()))
 		// trigger a get so that the server cleans up our keepalive socket
-		http.Get("http://localhost:7777/metrics")
+		_, err = http.Get("http://localhost:7777/metrics")
+		require.NoError(t, err)
 	})
 
 	assert.NotNil(t, exp)

exporter/zipkinexporter/zipkin_test.go

@@ -88,7 +88,7 @@ func TestZipkinExporter_roundtripJSON(t *testing.T) {
 	require.NotNil(t, zi)
 
 	require.NoError(t, zi.Start(context.Background(), componenttest.NewNopHost()))
-	defer zi.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, zi.Shutdown(context.Background())) })
 
 	// Let the receiver receive "uploaded Zipkin spans from a Java client application"
 	req, _ := http.NewRequest("POST", "https://tld.org/", strings.NewReader(zipkinSpansJSONJavaLibrary))
@@ -301,7 +301,7 @@ func TestZipkinExporter_roundtripProto(t *testing.T) {
 	buf := new(bytes.Buffer)
 	var contentType string
 	cst := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		io.Copy(buf, r.Body)
+		io.Copy(buf, r.Body) // nolint:errcheck
 		contentType = r.Header.Get("Content-Type")
 		r.Body.Close()
 	}))
@@ -337,7 +337,7 @@ func TestZipkinExporter_roundtripProto(t *testing.T) {
 
 	err = zi.Start(context.Background(), componenttest.NewNopHost())
 	require.NoError(t, err)
-	defer zi.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, zi.Shutdown(context.Background())) })
 
 	// Let the receiver receive "uploaded Zipkin spans from a Java client application"
 	req, _ := http.NewRequest("POST", "https://tld.org/", strings.NewReader(zipkinSpansJSONJavaLibrary))

extension/healthcheckextension/healthcheckextension_test.go

@@ -42,7 +42,7 @@ func TestHealthCheckExtensionUsage(t *testing.T) {
 	require.NotNil(t, hcExt)
 
 	require.NoError(t, hcExt.Start(context.Background(), componenttest.NewNopHost()))
-	defer hcExt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, hcExt.Shutdown(context.Background())) })
 
 	// Give a chance for the server goroutine to run.
 	runtime.Gosched()
@@ -55,13 +55,13 @@ func TestHealthCheckExtensionUsage(t *testing.T) {
 
 	require.Equal(t, http.StatusServiceUnavailable, resp0.StatusCode)
 
-	hcExt.Ready()
+	require.NoError(t, hcExt.Ready())
 	resp1, err := client.Get(url)
 	require.NoError(t, err)
 	defer resp1.Body.Close()
 	require.Equal(t, http.StatusOK, resp1.StatusCode)
 
-	hcExt.NotReady()
+	require.NoError(t, hcExt.NotReady())
 	resp2, err := client.Get(url)
 	require.NoError(t, err)
 	defer resp2.Body.Close()
@@ -102,7 +102,7 @@ func TestHealthCheckMultipleStarts(t *testing.T) {
 
 	mh := newAssertNoErrorHost(t)
 	require.NoError(t, hcExt.Start(context.Background(), mh))
-	defer hcExt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, hcExt.Shutdown(context.Background())) })
 
 	require.Error(t, hcExt.Start(context.Background(), mh))
 }

extension/pprofextension/pprofextension_test.go

@@ -44,7 +44,7 @@ func TestPerformanceProfilerExtensionUsage(t *testing.T) {
 	require.NotNil(t, pprofExt)
 
 	require.NoError(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
-	defer pprofExt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, pprofExt.Shutdown(context.Background())) })
 
 	// Give a chance for the server goroutine to run.
 	runtime.Gosched()
@@ -88,7 +88,7 @@ func TestPerformanceProfilerMultipleStarts(t *testing.T) {
 	require.NotNil(t, pprofExt)
 
 	require.NoError(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))
-	defer pprofExt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, pprofExt.Shutdown(context.Background())) })
 
 	// The instance is already active it will fail.
 	require.Error(t, pprofExt.Start(context.Background(), componenttest.NewNopHost()))

extension/zpagesextension/zpagesextension_test.go

@@ -40,7 +40,7 @@ func TestZPagesExtensionUsage(t *testing.T) {
 	require.NotNil(t, zpagesExt)
 
 	require.NoError(t, zpagesExt.Start(context.Background(), componenttest.NewNopHost()))
-	defer zpagesExt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, zpagesExt.Shutdown(context.Background())) })
 
 	// Give a chance for the server goroutine to run.
 	runtime.Gosched()
@@ -84,7 +84,7 @@ func TestZPagesMultipleStarts(t *testing.T) {
 	require.NotNil(t, zpagesExt)
 
 	require.NoError(t, zpagesExt.Start(context.Background(), componenttest.NewNopHost()))
-	defer zpagesExt.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, zpagesExt.Shutdown(context.Background())) })
 
 	// Try to start it again, it will fail since it is on the same endpoint.
 	require.Error(t, zpagesExt.Start(context.Background(), componenttest.NewNopHost()))

internal/middleware/compression_test.go

@@ -85,7 +85,8 @@ func TestHTTPClientCompression(t *testing.T) {
 			res, err := client.Do(req)
 			require.NoError(t, err)
 
-			ioutil.ReadAll(res.Body)
+			_, err = ioutil.ReadAll(res.Body)
+			require.NoError(t, err)
 			require.NoError(t, res.Body.Close(), "failed to close request body: %v", err)
 			require.NoError(t, srv.Close())
 		})

processor/batchprocessor/batch_processor_test.go

@@ -96,7 +96,7 @@ func TestBatchProcessorSpansDeliveredEnforceBatchSize(t *testing.T) {
 
 	// Added to test logic that check for empty resources.
 	td := testdata.GenerateTraceDataEmpty()
-	batcher.ConsumeTraces(context.Background(), td)
+	require.NoError(t, batcher.ConsumeTraces(context.Background(), td))
 
 	// wait for all spans to be reported
 	for {

receiver/jaegerreceiver/jaeger_agent_test.go

@@ -67,7 +67,7 @@ func TestJaegerAgentUDP_ThriftCompact_InvalidPort(t *testing.T) {
 
 	assert.Error(t, jr.Start(context.Background(), componenttest.NewNopHost()), "should not have been able to startTraceReception")
 
-	jr.Shutdown(context.Background())
+	require.NoError(t, jr.Shutdown(context.Background()))
 }
 
 func TestJaegerAgentUDP_ThriftBinary(t *testing.T) {
@@ -91,7 +91,7 @@ func TestJaegerAgentUDP_ThriftBinary_PortInUse(t *testing.T) {
 	jr := newJaegerReceiver(jaegerAgent, config, nil, params)
 
 	assert.NoError(t, jr.startAgent(componenttest.NewNopHost()), "Start failed")
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	l, err := net.Listen("udp", fmt.Sprintf("localhost:%d", port))
 	assert.Error(t, err, "should not have been able to listen to the port")
@@ -112,7 +112,7 @@ func TestJaegerAgentUDP_ThriftBinary_InvalidPort(t *testing.T) {
 
 	assert.Error(t, jr.Start(context.Background(), componenttest.NewNopHost()), "should not have been able to startTraceReception")
 
-	jr.Shutdown(context.Background())
+	require.NoError(t, jr.Shutdown(context.Background()))
 }
 
 func initializeGRPCTestServer(t *testing.T, beforeServe func(server *grpc.Server), opts ...grpc.ServerOption) (*grpc.Server, net.Addr) {
@@ -152,7 +152,7 @@ func TestJaegerHTTP(t *testing.T) {
 	}
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerAgent, config, nil, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	assert.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()), "Start failed")
 
@@ -190,7 +190,7 @@ func testJaegerAgent(t *testing.T, agentEndpoint string, receiverConfig *configu
 	sink := new(consumertest.TracesSink)
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerAgent, receiverConfig, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	assert.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()), "Start failed")
 

receiver/jaegerreceiver/trace_receiver_test.go

@@ -136,7 +136,7 @@ func TestReception(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	t.Log("Starting")
 
@@ -169,7 +169,7 @@ func TestPortsNotOpen(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	require.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()))
 
@@ -198,7 +198,7 @@ func TestGRPCReception(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	require.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()))
 
@@ -254,7 +254,7 @@ func TestGRPCReceptionWithTLS(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	require.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()))
 
@@ -328,7 +328,7 @@ func expectedTraceData(t1, t2, t3 time.Time) pdata.Traces {
 
 func grpcFixture(t1 time.Time, d1, d2 time.Duration) *api_v2.PostSpansRequest {
 	traceID := model.TraceID{}
-	traceID.Unmarshal([]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80})
+	traceID.Unmarshal([]byte{0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF, 0x80}) // nolint:errcheck
 	parentSpanID := model.NewSpanID(binary.BigEndian.Uint64([]byte{0x1F, 0x1E, 0x1D, 0x1C, 0x1B, 0x1A, 0x19, 0x18}))
 	childSpanID := model.NewSpanID(binary.BigEndian.Uint64([]byte{0xAF, 0xAE, 0xAD, 0xAC, 0xAB, 0xAA, 0xA9, 0xA8}))
 
@@ -389,7 +389,7 @@ func TestSampling(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	require.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()))
 	t.Log("Start")
@@ -441,7 +441,7 @@ func TestSamplingFailsOnNotConfigured(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 
 	require.NoError(t, jr.Start(context.Background(), componenttest.NewNopHost()))
 	t.Log("Start")
@@ -470,7 +470,7 @@ func TestSamplingFailsOnBadFile(t *testing.T) {
 
 	params := component.ReceiverCreateParams{Logger: zap.NewNop()}
 	jr := newJaegerReceiver(jaegerReceiver, config, sink, params)
-	defer jr.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, jr.Shutdown(context.Background())) })
 	assert.Error(t, jr.Start(context.Background(), componenttest.NewNopHost()))
 }
 
@@ -531,7 +531,7 @@ func TestSamplingStrategiesMutualTLS(t *testing.T) {
 	require.NoError(t, err)
 	err = exp.Start(context.Background(), newAssertNoErrorHost(t))
 	require.NoError(t, err)
-	defer exp.Shutdown(context.Background())
+	t.Cleanup(func() { require.NoError(t, exp.Shutdown(context.Background())) })
 	<-time.After(200 * time.Millisecond)
 
 	resp, err := http.Get(fmt.Sprintf("http://%s?service=bar", hostEndpoint))
@@ -577,7 +577,10 @@ func sendToCollector(endpoint string, batch *jaegerthrift.Batch) error {
 		return err
 	}
 
-	io.Copy(ioutil.Discard, resp.Body)
+	_, err = io.Copy(ioutil.Discard, resp.Body)
+	if err != nil {
+		return err
+	}
 	resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {