Repository: knative/pkg
Pull request #1375 (2020-06-03 09:28:03)

Assorted linting fixes.
- Removing unneeded conversions.
- Preallocate slices.
- Simplify comparisons.

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1375#" title="Author self-approved">markusthoemmes</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **chizhg**, **vagababov**
You can assign the PR to them by writing `/assign @chizhg @vagababov` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[reconciler/OWNERS](https://github.com/knative/pkg/blob/master/reconciler/OWNERS)**
- **[test/OWNERS](https://github.com/knative/pkg/blob/master/test/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["chizhg","vagababov"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[reconciler/reconcile_common.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1375/pull-knative-pkg-go-coverage/1268112335180926977/artifacts/line-cov.html#file0) | 84.6% | 85.7% | 1.1
[test/gcs/mock/mock.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1375/pull-knative-pkg-go-coverage/1268112335180926977/artifacts/line-cov.html#file2) | 96.0% | 94.9% | -1.1


Patch:
diff --git a/configmap/example.go b/configmap/example.go
index ee17666b7..2ffe448e5 100644
--- a/configmap/example.go
+++ b/configmap/example.go
@@ -17,6 +17,7 @@ limitations under the License.
 package configmap
 
 import (
+	"fmt"
 	"hash/crc32"
 )
 
@@ -29,6 +30,6 @@ const (
 )
 
 // Checksum generates a checksum for the example value to be compared against a respective label.
-func Checksum(value string) uint32 {
-	return crc32.ChecksumIEEE([]byte(value))
+func Checksum(value string) string {
+	return fmt.Sprintf("%08x", crc32.ChecksumIEEE([]byte(value)))
 }
diff --git a/configmap/hash-gen/main.go b/configmap/hash-gen/main.go
index ff105b7d7..c6e5110e3 100644
--- a/configmap/hash-gen/main.go
+++ b/configmap/hash-gen/main.go
@@ -74,7 +74,7 @@ func process(data []byte) ([]byte, error) {
 		return nil, errors.New("'metadata.labels' not found")
 	}
 
-	checksum := fmt.Sprint(configmap.Checksum(example.Value))
+	checksum := configmap.Checksum(example.Value)
 	existingLabel := value(labels, configmap.ExampleChecksumLabel)
 	if existingLabel != nil {
 		existingLabel.Value = checksum
diff --git a/webhook/configmaps/configmaps.go b/webhook/configmaps/configmaps.go
index 8cc150b0a..f6ffb81b7 100644
--- a/webhook/configmaps/configmaps.go
+++ b/webhook/configmaps/configmaps.go
@@ -187,7 +187,7 @@ func (ac *reconciler) validate(ctx context.Context, req *admissionv1beta1.Admiss
 	exampleData, hasExampleData := newObj.Data[configmap.ExampleKey]
 	exampleChecksum, hasExampleChecksumLabel := newObj.Labels[configmap.ExampleChecksumLabel]
 	if hasExampleData && hasExampleChecksumLabel &&
-		exampleChecksum != fmt.Sprint(configmap.Checksum(exampleData)) {
+		exampleChecksum != configmap.Checksum(exampleData) {
 		return fmt.Errorf(
 			"%q modified, you likely wanted to create an unindented configuration",
 			configmap.ExampleKey)


Test patch:
diff --git a/configmap/example_test.go b/configmap/example_test.go
index df6734433..aafc951f0 100644
--- a/configmap/example_test.go
+++ b/configmap/example_test.go
@@ -21,21 +21,21 @@ import "testing"
 func TestChecksum(t *testing.T) {
 	tests := []struct {
 		in   string
-		want uint32
+		want string
 	}{{
 		in:   "",
-		want: 0,
+		want: "00000000",
 	}, {
 		in:   "1",
-		want: 2212294583,
+		want: "83dcefb7",
 	}, {
 		in:   "a somewhat longer test",
-		want: 4278087538,
+		want: "fefe6f72",
 	}}
 
 	for _, test := range tests {
 		if got := Checksum(test.in); got != test.want {
-			t.Errorf("Checksum(%q) = %d, want %d", test.in, got, test.want)
+			t.Errorf("Checksum(%q) = %s, want %s", test.in, got, test.want)
 		}
 	}
 }
diff --git a/configmap/hash-gen/testdata/add_want.yaml b/configmap/hash-gen/testdata/add_want.yaml
index 2cccc9b4d..22de0a376 100644
--- a/configmap/hash-gen/testdata/add_want.yaml
+++ b/configmap/hash-gen/testdata/add_want.yaml
@@ -19,7 +19,7 @@ metadata:
   namespace: knative-serving
   labels:
     serving.knative.dev/release: devel
-    knative.dev/example-checksum: "2958957236"
+    knative.dev/example-checksum: b05e1ab4
 data:
   leave-me: "alone"
   _example: |
diff --git a/configmap/hash-gen/testdata/update_want.yaml b/configmap/hash-gen/testdata/update_want.yaml
index 2cccc9b4d..22de0a376 100644
--- a/configmap/hash-gen/testdata/update_want.yaml
+++ b/configmap/hash-gen/testdata/update_want.yaml
@@ -19,7 +19,7 @@ metadata:
   namespace: knative-serving
   labels:
     serving.knative.dev/release: devel
-    knative.dev/example-checksum: "2958957236"
+    knative.dev/example-checksum: b05e1ab4
 data:
   leave-me: "alone"
   _example: |
diff --git a/configmap/testing/configmap.go b/configmap/testing/configmap.go
index bd61285bf..5c4a2d8c1 100644
--- a/configmap/testing/configmap.go
+++ b/configmap/testing/configmap.go
@@ -86,7 +86,7 @@ func ConfigMapsFromTestFile(t *testing.T, name string, allowed ...string) (*core
 	// Check that the hashed exampleBody matches the assigned label, if present.
 	gotChecksum, hasExampleChecksumLabel := orig.Labels[configmap.ExampleChecksumLabel]
 	if hasExampleBody && hasExampleChecksumLabel {
-		wantChecksum := fmt.Sprint(configmap.Checksum(exampleBody))
+		wantChecksum := configmap.Checksum(exampleBody)
 		if gotChecksum != wantChecksum {
 			t.Errorf("example checksum label = %s, want %s", gotChecksum, wantChecksum)
 		}
diff --git a/reconciler/reconcile_common_test.go b/reconciler/reconcile_common_test.go
index 0d75af590..a1858fb2f 100644
--- a/reconciler/reconcile_common_test.go
+++ b/reconciler/reconcile_common_test.go
@@ -49,7 +49,7 @@ func makeResource() *TestResource {
 		Message: "Something something foo",
 	}
 	readyCond := apis.Condition{
-		Type:    apis.ConditionType(apis.ConditionReady),
+		Type:    apis.ConditionReady,
 		Status:  corev1.ConditionTrue,
 		Message: "Something something bar",
 	}
diff --git a/test/gcs/gcs.go b/test/gcs/gcs.go
index fc3cc3e21..b2798ac6a 100644
--- a/test/gcs/gcs.go
+++ b/test/gcs/gcs.go
@@ -120,7 +120,7 @@ func (g *GCSClient) list(ctx context.Context, bucketName, storagePath, exclusion
 	if err != nil {
 		return nil, err
 	}
-	var filePaths []string
+	filePaths := make([]string, 0, len(objsAttrs))
 	for _, attrs := range objsAttrs {
 		filePaths = append(filePaths, path.Join(attrs.Prefix, attrs.Name))
 	}
diff --git a/test/gcs/mock/mock.go b/test/gcs/mock/mock.go
index efb3e791b..711cc04bd 100644
--- a/test/gcs/mock/mock.go
+++ b/test/gcs/mock/mock.go
@@ -91,7 +91,7 @@ func (c *clientMocker) ClearError() {
 // getError is a helper that returns the error if it is set for this function
 func (c *clientMocker) getError(funcName method) (bool, error) {
 	if val, ok := c.err[funcName]; ok {
-		if val.NumCall <= 0 {
+		if val.NumCall == 0 {
 			delete(c.err, funcName)
 			return true, val.Err
 		}
diff --git a/test/gcs/mock/mock_test.go b/test/gcs/mock/mock_test.go
index 40f8d7eb4..6c4f5d796 100644
--- a/test/gcs/mock/mock_test.go
+++ b/test/gcs/mock/mock_test.go
@@ -796,7 +796,7 @@ func TestCopyObject(t *testing.T) {
 				t.Fatalf("cannot read %v from bucket %v, got error %v", tt.dstObjPath, tt.dstBkt, err)
 			}
 
-			if bytes.Compare(objContent, content) != 0 {
+			if !bytes.Equal(objContent, content) {
 				t.Fatalf("expected copied content %v, got content %v", content, objContent)
 			}
 		})
@@ -853,7 +853,7 @@ func TestReadObject(t *testing.T) {
 				return
 			}
 
-			if bytes.Compare(content, objContent) != 0 {
+			if !bytes.Equal(content, objContent) {
 				t.Fatalf("expected content %v, got content %v", content, objContent)
 			}
 		})
@@ -915,7 +915,7 @@ func TestWriteObject(t *testing.T) {
 
 			if content, err := mockClient.ReadObject(ctx, tt.bkt, tt.objpath); err != nil {
 				t.Fatalf("read object returned error %v", err)
-			} else if bytes.Compare(content, tt.content) != 0 {
+			} else if !bytes.Equal(content, tt.content) {
 				t.Fatalf("expected content %v, got content %v", tt.content, content)
 			}
 		})
@@ -1027,7 +1027,7 @@ func TestDownload(t *testing.T) {
 			if err != nil {
 				t.Fatalf("cannot read content %v, error %v", file, err)
 			}
-			if bytes.Compare(fileContent, content) != 0 {
+			if !bytes.Equal(fileContent, content) {
 				t.Fatalf("expected copied content %v, got content %v", content, fileContent)
 			}
 		})
@@ -1090,7 +1090,7 @@ func TestUpload(t *testing.T) {
 			if err != nil {
 				t.Fatalf("cannot read content %v in bucket %v, error %v", tt.objPath, tt.bkt, err)
 			}
-			if bytes.Compare(objContent, content) != 0 {
+			if !bytes.Equal(objContent, content) {
 				t.Fatalf("expected copied content %v, got content %v", content, objContent)
 			}
 		})
diff --git a/test/junit/junit_test.go b/test/junit/junit_test.go
index 21cc64dc7..7241b8bea 100644
--- a/test/junit/junit_test.go
+++ b/test/junit/junit_test.go
@@ -160,8 +160,8 @@ func TestCreateXMLErrorMsg(t *testing.T) {
 	}
 	defer os.RemoveAll(testDir) // clean up
 	dest := path.Join(testDir, "TestCreateXMLErrorTestFile")
-	CreateXMLErrorMsg("dummySuite", "dummyTest", "dummyError has occured", dest)
-	expected := `<testsuites><testsuite name="dummySuite" time="0" failures="1" tests="1"><testcase name="dummyTest" time="0" classname=""><failure>dummyError has occured</failure><properties></properties></testcase><properties></properties></testsuite></testsuites>`
+	CreateXMLErrorMsg("dummySuite", "dummyTest", "dummyError has occurred", dest)
+	expected := `<testsuites><testsuite name="dummySuite" time="0" failures="1" tests="1"><testcase name="dummyTest" time="0" classname=""><failure>dummyError has occurred</failure><properties></properties></testcase><properties></properties></testsuite></testsuites>`
 
 	got, err := ioutil.ReadFile(dest)
 	if err != nil {