Repository: kopia/kopia
Pull request #4074 (2024-08-25 00:29:48)

refactor(general): minor miscellaneous cleanups
Cleanups:

- use non-format variants of `Log`/`Print` with no additional args;
- fold in `Fprintf` call with no args into the following one;
- add missing arg placeholder in format strings;
- use `require.Positive` instead of `Greater(..., 0)`;
- rename function to `fillWithZeros` to avoid collision with builtin `clear`;
- define type for context key to avoid collisions;

See individual commits.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/kopia/kopia/pull/4074?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 77.23%. Comparing base [(`cb455c6`)](https://app.codecov.io/gh/kopia/kopia/commit/cb455c699a04e54cb47e25a676d7d65063ce883e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) to head [(`29074e0`)](https://app.codecov.io/gh/kopia/kopia/commit/29074e0f740466b7d5c7e6f350b6ac45982f4f25?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).
> Report is 235 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4074      +/-   ##
==========================================
+ Coverage   75.86%   77.23%   +1.36%     
==========================================
  Files         470      483      +13     
  Lines       37301    28850    -8451     
==========================================
- Hits        28299    22282    -6017     
+ Misses       7071     4661    -2410     
+ Partials     1931     1907      -24     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/kopia/kopia/pull/4074?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).

julio-lopez: @jkowalski FYI

Patch:
diff --git a/cli/json_output.go b/cli/json_output.go
index 2a46f555..6435eb9a 100644
--- a/cli/json_output.go
+++ b/cli/json_output.go
@@ -96,7 +96,7 @@ func (l *jsonList) begin(o *jsonOutput) {
 	l.o = o
 
 	if o.jsonOutput {
-		fmt.Fprintf(l.o.out, "[") //nolint:errcheck
+		fmt.Fprint(l.o.out, "[") //nolint:errcheck
 
 		if !o.jsonIndent {
 			l.separator = "\n "
@@ -107,16 +107,15 @@ func (l *jsonList) begin(o *jsonOutput) {
 func (l *jsonList) end() {
 	if l.o.jsonOutput {
 		if !l.o.jsonIndent {
-			fmt.Fprintf(l.o.out, "\n") //nolint:errcheck
+			fmt.Fprint(l.o.out, "\n") //nolint:errcheck
 		}
 
-		fmt.Fprintf(l.o.out, "]") //nolint:errcheck
+		fmt.Fprint(l.o.out, "]") //nolint:errcheck
 	}
 }
 
 func (l *jsonList) emit(v interface{}) {
-	fmt.Fprintf(l.o.out, l.separator)            //nolint:errcheck
-	fmt.Fprintf(l.o.out, "%s", l.o.jsonBytes(v)) //nolint:errcheck
+	fmt.Fprintf(l.o.out, "%s%s", l.separator, l.o.jsonBytes(v)) //nolint:errcheck
 
 	if l.o.jsonIndent {
 		l.separator = ","
diff --git a/repo/content/content_manager.go b/repo/content/content_manager.go
index 11307871..ff37cf45 100644
--- a/repo/content/content_manager.go
+++ b/repo/content/content_manager.go
@@ -833,7 +833,7 @@ func (bm *WriteManager) WriteContent(ctx context.Context, data gather.Bytes, pre
 		logbuf.AppendInt64(previousWriteTime)
 	}
 
-	bm.log.Debugf(logbuf.String())
+	bm.log.Debug(logbuf.String())
 
 	return contentID, bm.addToPackUnlocked(ctx, contentID, data, false, comp, previousWriteTime, mp)
 }
diff --git a/repo/content/content_manager_lock_free.go b/repo/content/content_manager_lock_free.go
index 089e2bc4..d43978e0 100644
--- a/repo/content/content_manager_lock_free.go
+++ b/repo/content/content_manager_lock_free.go
@@ -144,7 +144,7 @@ func (sm *SharedManager) preparePackDataContent(mp format.MutableParameters, pp
 		sb.AppendUint32(info.PackedLength)
 		sb.AppendString(" d:")
 		sb.AppendBoolean(info.Deleted)
-		sm.log.Debugf(sb.String())
+		sm.log.Debug(sb.String())
 
 		packFileIndex.Add(info)
 	}
diff --git a/repo/ecc/ecc_rs_crc.go b/repo/ecc/ecc_rs_crc.go
index 80e46117..34110751 100644
--- a/repo/ecc/ecc_rs_crc.go
+++ b/repo/ecc/ecc_rs_crc.go
@@ -179,7 +179,7 @@ func (r *ReedSolomonCrcECC) Encrypt(input gather.Bytes, _ []byte, output *gather
 
 	// WriteBuffer does not clear the data, so we must clear the padding
 	if lengthSize+len(copied) < len(inputBytes) {
-		clear(inputBytes[lengthSize+len(copied):])
+		fillWithZeros(inputBytes[lengthSize+len(copied):])
 	}
 
 	// Compute and store ECC + checksum
@@ -261,7 +261,7 @@ func (r *ReedSolomonCrcECC) Decrypt(input gather.Bytes, _ []byte, output *gather
 
 	// WriteBuffer does not clear the data, so we must clear the padding
 	if len(copied) < len(inputBytes) {
-		clear(inputBytes[len(copied):])
+		fillWithZeros(inputBytes[len(copied):])
 	}
 
 	eccBytes := inputBytes[:parityPlusCrcSizeInBlock*sizes.Blocks]
diff --git a/repo/ecc/ecc_utils.go b/repo/ecc/ecc_utils.go
index ba29319e..b64a2f37 100644
--- a/repo/ecc/ecc_utils.go
+++ b/repo/ecc/ecc_utils.go
@@ -34,9 +34,9 @@ func applyPercent(val int, percent float32) int {
 	return int(math.Floor(float64(val) * float64(percent)))
 }
 
-func clear(bytes []byte) {
-	for i := range bytes {
-		bytes[i] = 0
+func fillWithZeros(b []byte) {
+	for i := range b {
+		b[i] = 0
 	}
 }
 


Test patch:
diff --git a/internal/gather/gather_bytes_test.go b/internal/gather/gather_bytes_test.go
index 7e2cda21..6e1d195e 100644
--- a/internal/gather/gather_bytes_test.go
+++ b/internal/gather/gather_bytes_test.go
@@ -314,7 +314,7 @@ type testCase struct {
 
 			// write the generated data
 			n, err := preWrt.Write(buf)
-			require.NoErrorf(t, err, "Write() faiiled, inputBufferSize: %8", tc.inputBufferSize)
+			require.NoErrorf(t, err, "Write() faiiled, inputBufferSize: %v", tc.inputBufferSize)
 			require.Equalf(t, defaultAllocator.chunkSize, preWrt.alloc.chunkSize,
 				"this test expects that the default-allocator will be used, but we are using: %#v", preWrt.alloc)
 
diff --git a/internal/repodiag/log_manager_test.go b/internal/repodiag/log_manager_test.go
index a1b5eb50..47530fbd 100644
--- a/internal/repodiag/log_manager_test.go
+++ b/internal/repodiag/log_manager_test.go
@@ -53,7 +53,7 @@ func TestLogManager_AutoFlush(t *testing.T) {
 		var b [1024]byte
 
 		rand.Read(b[:])
-		l.Infof(hex.EncodeToString(b[:]))
+		l.Info(hex.EncodeToString(b[:]))
 	}
 
 	w.Wait(ctx)
diff --git a/repo/blob/s3/s3_versioned_test.go b/repo/blob/s3/s3_versioned_test.go
index bc933855..4dfd017a 100644
--- a/repo/blob/s3/s3_versioned_test.go
+++ b/repo/blob/s3/s3_versioned_test.go
@@ -748,7 +748,7 @@ func compareMetadata(tb testing.TB, a, b versionMetadata) {
 	// deletion-marker metadata is not returned by the delete blob operation,
 	// and can only be retrieved later by listing versions.
 	if !a.IsDeleteMarker {
-		require.Equalf(tb, a.Version, b.Version, "blob versions do not match a:%v b:v", a, b)
+		require.Equalf(tb, a.Version, b.Version, "blob versions do not match a:%v b:%v", a, b)
 	}
 }
 
diff --git a/snapshot/snapshotfs/upload_test.go b/snapshot/snapshotfs/upload_test.go
index 934c0a9b..279ed8f2 100644
--- a/snapshot/snapshotfs/upload_test.go
+++ b/snapshot/snapshotfs/upload_test.go
@@ -740,7 +740,7 @@ func TestParallelUploadUploadsBlobsInParallel(t *testing.T) {
 
 	require.NoError(t, th.repo.Flush(ctx))
 
-	require.Greater(t, maxParallelCalls.Load(), int32(0))
+	require.Positive(t, maxParallelCalls.Load())
 }
 
 func randomBytes(n int64) []byte {
diff --git a/tests/recovery/recovery_test/recovery_test.go b/tests/recovery/recovery_test/recovery_test.go
index 6c77672c..747c6d82 100644
--- a/tests/recovery/recovery_test/recovery_test.go
+++ b/tests/recovery/recovery_test/recovery_test.go
@@ -218,7 +218,7 @@ func TestConsistencyWhenKill9AfterModify(t *testing.T) {
 
 	o, err := cmd.CombinedOutput()
 	require.NoError(t, err)
-	t.Logf(string(o))
+	t.Log(string(o))
 
 	// create snapshot with StderrPipe
 	cmd = exec.Command(kopiaExe, "snap", "create", newDir, "--json", "--parallel=1")
@@ -240,9 +240,8 @@ func TestConsistencyWhenKill9AfterModify(t *testing.T) {
 	stdout, err := bm.RestoreGivenOrRandomSnapshot("", restoreDir)
 	require.NoError(t, err)
 
-	t.Logf(stdout)
-
-	t.Logf("Compare restored data and original data:")
+	t.Log(stdout)
+	t.Log("Compare restored data and original data:")
 	CompareDirs(t, restoreDir, cmpDir)
 }
 
@@ -267,7 +266,7 @@ func killOnCondition(t *testing.T, cmd *exec.Cmd) {
 
 		for scanner.Scan() {
 			output := scanner.Text()
-			t.Logf(output)
+			t.Log(output)
 
 			// Check if the output contains the "hashing" etc.
 			if strings.Contains(output, "hashing") && strings.Contains(output, "hashed") && strings.Contains(output, "uploaded") {
diff --git a/tests/robustness/multiclient_test/framework/client.go b/tests/robustness/multiclient_test/framework/client.go
index 5e040fd0..7704bcdb 100644
--- a/tests/robustness/multiclient_test/framework/client.go
+++ b/tests/robustness/multiclient_test/framework/client.go
@@ -12,7 +12,9 @@
 
 const nameLen int = 2
 
-var clientKey = struct{}{}
+type clientKeyT struct{}
+
+var clientKey clientKeyT
 
 // Client is a unique client for use in multiclient robustness tests.
 type Client struct {