Various cleanups (#1446)

Thanks @markusthoemmes for the tool :)

apis/condition_set.go

@@ -184,25 +184,25 @@ func (r conditionsImpl) GetCondition(t ConditionType) *Condition {
 
 // SetCondition sets or updates the Condition on Conditions for Condition.Type.
 // If there is an update, Conditions are stored back sorted.
-func (r conditionsImpl) SetCondition(new Condition) {
+func (r conditionsImpl) SetCondition(cond Condition) {
 	if r.accessor == nil {
 		return
 	}
-	t := new.Type
+	t := cond.Type
 	var conditions Conditions
 	for _, c := range r.accessor.GetConditions() {
 		if c.Type != t {
 			conditions = append(conditions, c)
 		} else {
 			// If we'd only update the LastTransitionTime, then return.
-			new.LastTransitionTime = c.LastTransitionTime
-			if reflect.DeepEqual(&new, &c) {
+			cond.LastTransitionTime = c.LastTransitionTime
+			if reflect.DeepEqual(cond, c) {
 				return
 			}
 		}
 	}
-	new.LastTransitionTime = VolatileTime{Inner: metav1.NewTime(time.Now())}
-	conditions = append(conditions, new)
+	cond.LastTransitionTime = VolatileTime{Inner: metav1.NewTime(time.Now())}
+	conditions = append(conditions, cond)
 	// Sorted for convenience of the consumer, i.e. kubectl.
 	sort.Slice(conditions, func(i, j int) bool { return conditions[i].Type < conditions[j].Type })
 	r.accessor.SetConditions(conditions)

apis/deprecated.go

@@ -36,7 +36,7 @@ func CheckDeprecated(ctx context.Context, obj interface{}) *FieldError {
 // CheckDeprecated checks whether the provided named deprecated fields
 // are set in a context where deprecation is disallowed.
 // This is a json shallow check. We will recursively check inlined structs.
-func CheckDeprecatedUpdate(ctx context.Context, obj interface{}, original interface{}) *FieldError {
+func CheckDeprecatedUpdate(ctx context.Context, obj, original interface{}) *FieldError {
 	if IsDeprecatedAllowed(ctx) {
 		return nil
 	}

apis/field_error.go

@@ -204,11 +204,12 @@ func flatten(path []string) string {
 	var newPath []string
 	for _, part := range path {
 		for _, p := range strings.Split(part, ".") {
-			if p == CurrentField {
+			switch {
+			case p == CurrentField:
 				continue
-			} else if len(newPath) > 0 && isIndex(p) {
+			case len(newPath) > 0 && isIndex(p):
 				newPath[len(newPath)-1] += p
-			} else {
+			default:
 				newPath = append(newPath, p)
 			}
 		}

metrics/record.go

@@ -40,7 +40,7 @@ func RecordBatch(ctx context.Context, mss ...stats.Measurement) {
 // be used to create a view.Distribution.
 func Buckets125(low, high float64) []float64 {
 	buckets := []float64{low}
-	for last := low; last < high; last = last * 10 {
+	for last := low; last < high; last *= 10 {
 		buckets = append(buckets, 2*last, 5*last, 10*last)
 	}
 	return buckets

reconciler/testing/hooks_test.go

@@ -134,7 +134,7 @@ func TestMultiUpdate(t *testing.T) {
 
 	updates := 0
 	h.OnUpdate(&f.Fake, "pods", func(obj runtime.Object) HookResult {
-		updates = updates + 1
+		updates++
 		switch updates {
 		case 1:
 		case 2:

test/gcs/mock/mock.go

@@ -95,7 +95,7 @@ func (c *clientMocker) getError(funcName Method) (bool, error) {
 			delete(c.err, funcName)
 			return true, val.Err
 		}
-		val.NumCall = val.NumCall - 1
+		val.NumCall--
 	}
 	return false, nil
 }

test/interactive/docker_test.go

@@ -40,7 +40,7 @@ func TestEnv(t *testing.T) {
 		if !exists {
 			break
 		}
-		badName = badName + "z"
+		badName += "z"
 	}
 	err = e.PromoteFromEnv(badName)
 	if err == nil {

tracing/config/tracing.go

@@ -86,16 +86,14 @@ func NewTracingConfigFromMap(cfgMap map[string]string) (*Config, error) {
 		default:
 			return nil, fmt.Errorf("unsupported tracing backend value %q", backend)
 		}
-	} else {
+	} else if enable, ok := cfgMap[enableKey]; ok {
 		// For backwards compatibility, parse the enabled flag as Zipkin.
-		if enable, ok := cfgMap[enableKey]; ok {
-			enableBool, err := strconv.ParseBool(enable)
-			if err != nil {
-				return nil, fmt.Errorf("failed parsing tracing config %q: %w", enableKey, err)
-			}
-			if enableBool {
-				tc.Backend = Zipkin
-			}
+		enableBool, err := strconv.ParseBool(enable)
+		if err != nil {
+			return nil, fmt.Errorf("failed parsing tracing config %q: %w", enableKey, err)
+		}
+		if enableBool {
+			tc.Backend = Zipkin
 		}
 	}
 

tracing/http_test.go

@@ -21,30 +21,27 @@ import (
 	"net/url"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	. "knative.dev/pkg/tracing"
 	"knative.dev/pkg/tracing/config"
 	. "knative.dev/pkg/tracing/testing"
 )
 
 type fakeWriter struct {
-	lastWrite *[]byte
+	lastWrite string
 }
 
-func (fw fakeWriter) Header() http.Header {
+func (fw *fakeWriter) Header() http.Header {
 	return http.Header{}
 }
 
-func (fw fakeWriter) Write(data []byte) (int, error) {
-	*fw.lastWrite = data
+func (fw *fakeWriter) Write(data []byte) (int, error) {
+	fw.lastWrite = string(data)
 	return len(data), nil
 }
 
-func (fw fakeWriter) WriteHeader(statusCode int) {
-}
+func (fw *fakeWriter) WriteHeader(statusCode int) {}
 
-type testHandler struct {
-}
+type testHandler struct{}
 
 func (th *testHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte("fake"))
@@ -58,9 +55,11 @@ func TestHTTPSpanMiddleware(t *testing.T) {
 
 	// Create tracer with reporter recorder
 	reporter, co := FakeZipkinExporter()
-	defer reporter.Close()
 	oct := NewOpenCensusTracer(co)
-	defer oct.Finish()
+	t.Cleanup(func() {
+		reporter.Close()
+		oct.Finish()
+	})
 
 	if err := oct.ApplyConfig(&cfg); err != nil {
 		t.Errorf("Failed to apply tracer config: %v", err)
@@ -69,22 +68,21 @@ func TestHTTPSpanMiddleware(t *testing.T) {
 	next := testHandler{}
 	middleware := HTTPSpanMiddleware(&next)
 
-	var lastWrite []byte
-	fw := fakeWriter{lastWrite: &lastWrite}
+	fw := &fakeWriter{}
 
 	req, err := http.NewRequest("GET", "http://test.example.com", nil)
 	if err != nil {
-		t.Errorf("Failed to make fake request: %v", err)
+		t.Fatal("Failed to make fake request:", err)
 	}
-	traceID := "821e0d50d931235a5ba3fa42eddddd8f"
+	const traceID = "821e0d50d931235a5ba3fa42eddddd8f"
 	req.Header["X-B3-Traceid"] = []string{traceID}
 	req.Header["X-B3-Spanid"] = []string{"b3bd5e1c4318c78a"}
 
 	middleware.ServeHTTP(fw, req)
 
 	// Assert our next handler was called
-	if diff := cmp.Diff([]byte("fake"), lastWrite); diff != "" {
-		t.Errorf("Got http response (-want, +got) = %v", diff)
+	if got, want := fw.lastWrite, "fake"; got != want {
+		t.Errorf("HTTP Response: %q, want: %q", got, want)
 	}
 
 	spans := reporter.Flush()
@@ -104,53 +102,54 @@ func TestHTTPSpanIgnoringPaths(t *testing.T) {
 
 	// Create tracer with reporter recorder
 	reporter, co := FakeZipkinExporter()
-	defer reporter.Close()
 	oct := NewOpenCensusTracer(co)
-	defer oct.Finish()
+	t.Cleanup(func() {
+		reporter.Close()
+		oct.Finish()
+	})
 
 	if err := oct.ApplyConfig(&cfg); err != nil {
-		t.Errorf("Failed to apply tracer config: %v", err)
+		t.Fatal("Failed to apply tracer config:", err)
 	}
 
 	paths := []string{"/readyz"}
 	middleware := HTTPSpanIgnoringPaths(paths...)(&testHandler{})
 
-	testCases := map[string]struct {
+	testCases := []struct {
+		name   string
 		path   string
 		traced bool
-	}{
-		"traced": {
-			path:   "/",
-			traced: true,
-		},
-		"ignored": {
-			path:   paths[0],
-			traced: false,
-		},
-	}
-	for n, tc := range testCases {
-		t.Run(n, func(t *testing.T) {
-			var lastWrite []byte
-			fw := fakeWriter{lastWrite: &lastWrite}
+	}{{
+		name:   "traced",
+		path:   "/",
+		traced: true,
+	}, {
+		name:   "ignored",
+		path:   paths[0],
+		traced: false,
+	}}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			fw := &fakeWriter{}
 
 			u := &url.URL{
 				Scheme: "http",
 				Host:   "test.example.com",
 				Path:   tc.path,
 			}
-			req, err := http.NewRequest("GET", u.String(), nil)
+			req, err := http.NewRequest(http.MethodGet, u.String(), nil)
 			if err != nil {
-				t.Errorf("Failed to make fake request: %v", err)
+				t.Fatal("Failed to make fake request:", err)
 			}
-			traceID := "821e0d50d931235a5ba3fa42eddddd8f"
+			const traceID = "821e0d50d931235a5ba3fa42eddddd8f"
 			req.Header.Set("X-B3-Traceid", traceID)
 			req.Header.Set("X-B3-Spanid", "b3bd5e1c4318c78a")
 
 			middleware.ServeHTTP(fw, req)
 
 			// Assert our next handler was called
-			if diff := cmp.Diff([]byte("fake"), lastWrite); diff != "" {
-				t.Errorf("Got http response (-want, +got) = %v", diff)
+			if got, want := string(fw.lastWrite), "fake"; got != want {
+				t.Errorf("HTTP response: %q, want: %q", got, want)
 			}
 
 			spans := reporter.Flush()
@@ -161,10 +160,8 @@ func TestHTTPSpanIgnoringPaths(t *testing.T) {
 				if got := spans[0].TraceID.String(); got != traceID {
 					t.Errorf("spans[0].TraceID = %s, want %s", got, traceID)
 				}
-			} else {
-				if len(spans) != 0 {
-					t.Errorf("Got %d spans, expected 0: spans = %v", len(spans), spans)
-				}
+			} else if len(spans) != 0 {
+				t.Errorf("Got %d spans, expected 0: spans = %v", len(spans), spans)
 			}
 		})
 	}

webhook/resourcesemantics/defaulting/user_info_test.go

@@ -64,12 +64,9 @@ func TestSetUserInfoAnnotationsWhenWithinCreate(t *testing.T) {
 			"pkg.knative.dev/lastModifier": user1,
 		},
 	}, {
-		name: "test create (should not touch annotations when no user info available)",
-		configureContext: func(ctx context.Context) context.Context {
-			return apis.WithinCreate(ctx)
-		},
-		setup: func(ctx context.Context, r *Resource) {
-		},
+		name:                "test create (should not touch annotations when no user info available)",
+		configureContext:    apis.WithinCreate,
+		setup:               func(ctx context.Context, r *Resource) {},
 		expectedAnnotations: nil,
 	}}