Merge pull request #45954 from thaJeztah/cleanup_ioutils

pkg/ioutils: some cleanups in tests

pkg/ioutils/buffer_test.go

@@ -17,7 +17,7 @@ func TestFixedBufferCap(t *testing.T) {
 func TestFixedBufferLen(t *testing.T) {
 	buf := &fixedBuffer{buf: make([]byte, 0, 10)}
 
-	buf.Write([]byte("hello"))
+	_, _ = buf.Write([]byte("hello"))
 	l := buf.Len()
 	if l != 5 {
 		t.Fatalf("expected buffer length to be 5 bytes, got %d", l)
@@ -31,7 +31,7 @@ func TestFixedBufferLen(t *testing.T) {
 
 	// read 5 bytes
 	b := make([]byte, 5)
-	buf.Read(b)
+	_, _ = buf.Read(b)
 
 	l = buf.Len()
 	if l != 5 {
@@ -61,8 +61,8 @@ func TestFixedBufferLen(t *testing.T) {
 func TestFixedBufferString(t *testing.T) {
 	buf := &fixedBuffer{buf: make([]byte, 0, 10)}
 
-	buf.Write([]byte("hello"))
-	buf.Write([]byte("world"))
+	_, _ = buf.Write([]byte("hello"))
+	_, _ = buf.Write([]byte("world"))
 
 	out := buf.String()
 	if out != "helloworld" {
@@ -71,7 +71,7 @@ func TestFixedBufferString(t *testing.T) {
 
 	// read 5 bytes
 	b := make([]byte, 5)
-	buf.Read(b)
+	_, _ = buf.Read(b)
 
 	// test that fixedBuffer.String() only returns the part that hasn't been read
 	out = buf.String()

pkg/ioutils/bytespipe_test.go

@@ -10,11 +10,11 @@ import (
 
 func TestBytesPipeRead(t *testing.T) {
 	buf := NewBytesPipe()
-	buf.Write([]byte("12"))
-	buf.Write([]byte("34"))
-	buf.Write([]byte("56"))
-	buf.Write([]byte("78"))
-	buf.Write([]byte("90"))
+	_, _ = buf.Write([]byte("12"))
+	_, _ = buf.Write([]byte("34"))
+	_, _ = buf.Write([]byte("56"))
+	_, _ = buf.Write([]byte("78"))
+	_, _ = buf.Write([]byte("90"))
 	rd := make([]byte, 4)
 	n, err := buf.Read(rd)
 	if err != nil {
@@ -50,11 +50,11 @@ func TestBytesPipeRead(t *testing.T) {
 
 func TestBytesPipeWrite(t *testing.T) {
 	buf := NewBytesPipe()
-	buf.Write([]byte("12"))
-	buf.Write([]byte("34"))
-	buf.Write([]byte("56"))
-	buf.Write([]byte("78"))
-	buf.Write([]byte("90"))
+	_, _ = buf.Write([]byte("12"))
+	_, _ = buf.Write([]byte("34"))
+	_, _ = buf.Write([]byte("56"))
+	_, _ = buf.Write([]byte("78"))
+	_, _ = buf.Write([]byte("90"))
 	if buf.buf[0].String() != "1234567890" {
 		t.Fatalf("Buffer %q, must be %q", buf.buf[0].String(), "1234567890")
 	}
@@ -121,12 +121,12 @@ func TestBytesPipeDeadlock(t *testing.T) {
 
 // Write and read in different speeds/chunk sizes and check valid data is read.
 func TestBytesPipeWriteRandomChunks(t *testing.T) {
-	cases := []struct{ iterations, writesPerLoop, readsPerLoop int }{
-		{100, 10, 1},
-		{1000, 10, 5},
-		{1000, 100, 0},
-		{1000, 5, 6},
-		{10000, 50, 25},
+	tests := []struct{ iterations, writesPerLoop, readsPerLoop int }{
+		{iterations: 100, writesPerLoop: 10, readsPerLoop: 1},
+		{iterations: 1000, writesPerLoop: 10, readsPerLoop: 5},
+		{iterations: 1000, writesPerLoop: 100},
+		{iterations: 1000, writesPerLoop: 5, readsPerLoop: 6},
+		{iterations: 10000, writesPerLoop: 50, readsPerLoop: 25},
 	}
 
 	testMessage := []byte("this is a random string for testing")
@@ -134,10 +134,10 @@ func TestBytesPipeWriteRandomChunks(t *testing.T) {
 	writeChunks := []int{25, 35, 15, 20}
 	readChunks := []int{5, 45, 20, 25}
 
-	for _, c := range cases {
+	for _, tc := range tests {
 		// first pass: write directly to hash
 		hash := sha256.New()
-		for i := 0; i < c.iterations*c.writesPerLoop; i++ {
+		for i := 0; i < tc.iterations*tc.writesPerLoop; i++ {
 			if _, err := hash.Write(testMessage[:writeChunks[i%len(writeChunks)]]); err != nil {
 				t.Fatal(err)
 			}
@@ -154,7 +154,7 @@ func TestBytesPipeWriteRandomChunks(t *testing.T) {
 			// random delay before read starts
 			<-time.After(time.Duration(rand.Intn(10)) * time.Millisecond)
 			for i := 0; ; i++ {
-				p := make([]byte, readChunks[(c.iterations*c.readsPerLoop+i)%len(readChunks)])
+				p := make([]byte, readChunks[(tc.iterations*tc.readsPerLoop+i)%len(readChunks)])
 				n, _ := buf.Read(p)
 				if n == 0 {
 					break
@@ -165,12 +165,12 @@ func TestBytesPipeWriteRandomChunks(t *testing.T) {
 			close(done)
 		}()
 
-		for i := 0; i < c.iterations; i++ {
-			for w := 0; w < c.writesPerLoop; w++ {
-				buf.Write(testMessage[:writeChunks[(i*c.writesPerLoop+w)%len(writeChunks)]])
+		for i := 0; i < tc.iterations; i++ {
+			for w := 0; w < tc.writesPerLoop; w++ {
+				buf.Write(testMessage[:writeChunks[(i*tc.writesPerLoop+w)%len(writeChunks)]])
 			}
 		}
-		buf.Close()
+		_ = buf.Close()
 		<-done
 
 		actual := hex.EncodeToString(hash.Sum(nil))
@@ -182,6 +182,7 @@ func TestBytesPipeWriteRandomChunks(t *testing.T) {
 }
 
 func BenchmarkBytesPipeWrite(b *testing.B) {
+	b.ReportAllocs()
 	testData := []byte("pretty short line, because why not?")
 	for i := 0; i < b.N; i++ {
 		readBuf := make([]byte, 1024)
@@ -193,19 +194,20 @@ func BenchmarkBytesPipeWrite(b *testing.B) {
 			}
 		}()
 		for j := 0; j < 1000; j++ {
-			buf.Write(testData)
+			_, _ = buf.Write(testData)
 		}
-		buf.Close()
+		_ = buf.Close()
 	}
 }
 
 func BenchmarkBytesPipeRead(b *testing.B) {
+	b.ReportAllocs()
 	rd := make([]byte, 512)
 	for i := 0; i < b.N; i++ {
 		b.StopTimer()
 		buf := NewBytesPipe()
 		for j := 0; j < 500; j++ {
-			buf.Write(make([]byte, 1024))
+			_, _ = buf.Write(make([]byte, 1024))
 		}
 		b.StartTimer()
 		for j := 0; j < 1000; j++ {

pkg/ioutils/fswriters_test.go

@@ -18,11 +18,7 @@ func init() {
 }
 
 func TestAtomicWriteToFile(t *testing.T) {
-	tmpDir, err := os.MkdirTemp("", "atomic-writers-test")
-	if err != nil {
-		t.Fatalf("Error when creating temporary directory: %s", err)
-	}
-	defer os.RemoveAll(tmpDir)
+	tmpDir := t.TempDir()
 
 	expected := []byte("barbaz")
 	if err := AtomicWriteFile(filepath.Join(tmpDir, "foo"), expected, testMode); err != nil {
@@ -48,11 +44,7 @@ func TestAtomicWriteToFile(t *testing.T) {
 }
 
 func TestAtomicWriteSetCommit(t *testing.T) {
-	tmpDir, err := os.MkdirTemp("", "atomic-writerset-test")
-	if err != nil {
-		t.Fatalf("Error when creating temporary directory: %s", err)
-	}
-	defer os.RemoveAll(tmpDir)
+	tmpDir := t.TempDir()
 
 	if err := os.Mkdir(filepath.Join(tmpDir, "tmp"), 0o700); err != nil {
 		t.Fatalf("Error creating tmp directory: %s", err)
@@ -96,11 +88,7 @@ func TestAtomicWriteSetCommit(t *testing.T) {
 }
 
 func TestAtomicWriteSetCancel(t *testing.T) {
-	tmpDir, err := os.MkdirTemp("", "atomic-writerset-test")
-	if err != nil {
-		t.Fatalf("Error when creating temporary directory: %s", err)
-	}
-	defer os.RemoveAll(tmpDir)
+	tmpDir := t.TempDir()
 
 	if err := os.Mkdir(filepath.Join(tmpDir, "tmp"), 0o700); err != nil {
 		t.Fatalf("Error creating tmp directory: %s", err)

pkg/ioutils/readers_test.go

@@ -2,59 +2,61 @@ package ioutils // import "github.com/docker/docker/pkg/ioutils"
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"io"
 	"strings"
 	"testing"
+	"testing/iotest"
 	"time"
-
-	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 )
 
-// Implement io.Reader
-type errorReader struct{}
-
-func (r *errorReader) Read(p []byte) (int, error) {
-	return 0, fmt.Errorf("error reader always fail")
-}
-
 func TestReadCloserWrapperClose(t *testing.T) {
-	reader := strings.NewReader("A string reader")
-	wrapper := NewReadCloserWrapper(reader, func() error {
-		return fmt.Errorf("This will be called when closing")
+	const text = "hello world"
+	testErr := errors.New("this will be called when closing")
+	wrapper := NewReadCloserWrapper(strings.NewReader(text), func() error {
+		return testErr
 	})
-	err := wrapper.Close()
-	if err == nil || !strings.Contains(err.Error(), "This will be called when closing") {
-		t.Fatalf("readCloserWrapper should have call the anonymous func and thus, fail.")
+
+	buf, err := io.ReadAll(wrapper)
+	if err != nil {
+		t.Errorf("io.ReadAll(wrapper) err = %v", err)
+	}
+	if string(buf) != text {
+		t.Errorf("expected %v, got: %v", text, string(buf))
+	}
+	err = wrapper.Close()
+	if !errors.Is(err, testErr) {
+		// readCloserWrapper should have called the anonymous func and thus, fail
+		t.Errorf("expected %v, got: %v", testErr, err)
 	}
 }
 
 func TestReaderErrWrapperReadOnError(t *testing.T) {
 	called := false
-	reader := &errorReader{}
-	wrapper := NewReaderErrWrapper(reader, func() {
+	expectedErr := errors.New("error reader always fail")
+	wrapper := NewReaderErrWrapper(iotest.ErrReader(expectedErr), func() {
 		called = true
 	})
 	_, err := wrapper.Read([]byte{})
-	assert.Check(t, is.Error(err, "error reader always fail"))
+	if !errors.Is(err, expectedErr) {
+		t.Errorf("expected %v, got: %v", expectedErr, err)
+	}
 	if !called {
-		t.Fatalf("readErrWrapper should have call the anonymous function on failure")
+		t.Fatalf("readErrWrapper should have called the anonymous function on failure")
 	}
 }
 
 func TestReaderErrWrapperRead(t *testing.T) {
-	reader := strings.NewReader("a string reader.")
-	wrapper := NewReaderErrWrapper(reader, func() {
+	const text = "hello world"
+	wrapper := NewReaderErrWrapper(strings.NewReader(text), func() {
 		t.Fatalf("readErrWrapper should not have called the anonymous function")
 	})
-	// Read 20 byte (should be ok with the string above)
-	num, err := wrapper.Read(make([]byte, 20))
+	num, err := wrapper.Read(make([]byte, len(text)+10))
 	if err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
-	if num != 16 {
-		t.Fatalf("readerErrWrapper should have read 16 byte, but read %d", num)
+	if expected := len(text); num != expected {
+		t.Errorf("readerErrWrapper should have read %d byte, but read %d", expected, num)
 	}
 }
 
@@ -70,10 +72,10 @@ func (p *perpetualReader) Read(buf []byte) (n int, err error) {
 func TestCancelReadCloser(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
 	defer cancel()
-	cancelReadCloser := NewCancelReadCloser(ctx, io.NopCloser(&perpetualReader{}))
+	crc := NewCancelReadCloser(ctx, io.NopCloser(&perpetualReader{}))
 	for {
 		var buf [128]byte
-		_, err := cancelReadCloser.Read(buf[:])
+		_, err := crc.Read(buf[:])
 		if err == context.DeadlineExceeded {
 			break
 		} else if err != nil {

pkg/ioutils/writers_test.go

@@ -51,8 +51,8 @@ func TestWriteCounter(t *testing.T) {
 	var buffer bytes.Buffer
 	wc := NewWriteCounter(&buffer)
 
-	reader1.WriteTo(wc)
-	reader2.WriteTo(wc)
+	_, _ = reader1.WriteTo(wc)
+	_, _ = reader2.WriteTo(wc)
 
 	if wc.Count != totalLength {
 		t.Errorf("Wrong count: %d vs. %d", wc.Count, totalLength)