From 34668a5945616f27739967c8e22525d6de521a21 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Wed, 1 Jun 2022 10:57:04 +0200
Subject: [PATCH] pkg/archive: fixe some unclosed file-handles in tests

Also fixing a "defer in loop" warning, instead changing to use
sub-tests, and simplifying some code, using os.WriteFile() instead.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 pkg/archive/archive_test.go       | 69 ++++++++++++++++++++-----------
 pkg/archive/changes_posix_test.go |  6 +--
 2 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go
index 006a57651d..e8edd50ec5 100644
--- a/pkg/archive/archive_test.go
+++ b/pkg/archive/archive_test.go
@@ -291,8 +291,15 @@ func TestUntarPathWithInvalidDest(t *testing.T) {
 	// Create a src file
 	srcFile := filepath.Join(tempFolder, "src")
 	tarFile := filepath.Join(tempFolder, "src.tar")
-	os.Create(srcFile)
-	os.Create(invalidDestFolder) // being a file (not dir) should cause an error
+	f, err := os.Create(srcFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
+
+	d, err := os.Create(invalidDestFolder) // being a file (not dir) should cause an error
+	if assert.Check(t, err) {
+		_ = d.Close()
+	}
 
 	// Translate back to Unix semantics as next exec.Command is run under sh
 	srcFileU := srcFile
@@ -331,7 +338,10 @@ func TestUntarPath(t *testing.T) {
 	defer os.RemoveAll(tmpFolder)
 	srcFile := filepath.Join(tmpFolder, "src")
 	tarFile := filepath.Join(tmpFolder, "src.tar")
-	os.Create(filepath.Join(tmpFolder, "src"))
+	f, err := os.Create(filepath.Join(tmpFolder, "src"))
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
 
 	destFolder := filepath.Join(tmpFolder, "dest")
 	err = os.MkdirAll(destFolder, 0o740)
@@ -370,7 +380,10 @@ func TestUntarPathWithDestinationFile(t *testing.T) {
 	defer os.RemoveAll(tmpFolder)
 	srcFile := filepath.Join(tmpFolder, "src")
 	tarFile := filepath.Join(tmpFolder, "src.tar")
-	os.Create(filepath.Join(tmpFolder, "src"))
+	f, err := os.Create(filepath.Join(tmpFolder, "src"))
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
 
 	// Translate back to Unix semantics as next exec.Command is run under sh
 	srcFileU := srcFile
@@ -385,9 +398,9 @@ func TestUntarPathWithDestinationFile(t *testing.T) {
 		t.Fatal(err)
 	}
 	destFile := filepath.Join(tmpFolder, "dest")
-	_, err = os.Create(destFile)
-	if err != nil {
-		t.Fatalf("Fail to create the destination file")
+	f, err = os.Create(destFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
 	}
 	err = defaultUntarPath(tarFile, destFile)
 	if err == nil {
@@ -406,7 +419,10 @@ func TestUntarPathWithDestinationSrcFileAsFolder(t *testing.T) {
 	defer os.RemoveAll(tmpFolder)
 	srcFile := filepath.Join(tmpFolder, "src")
 	tarFile := filepath.Join(tmpFolder, "src.tar")
-	os.Create(srcFile)
+	f, err := os.Create(srcFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
+	}
 
 	// Translate back to Unix semantics as next exec.Command is run under sh
 	srcFileU := srcFile
@@ -562,9 +578,9 @@ func TestCopyFileWithTarInexistentDestWillCreateIt(t *testing.T) {
 	defer os.RemoveAll(tempFolder)
 	srcFile := filepath.Join(tempFolder, "src")
 	inexistentDestFolder := filepath.Join(tempFolder, "doesnotexists")
-	_, err = os.Create(srcFile)
-	if err != nil {
-		t.Fatal(err)
+	f, err := os.Create(srcFile)
+	if assert.Check(t, err) {
+		_ = f.Close()
 	}
 	err = defaultCopyFileWithTar(srcFile, inexistentDestFolder)
 	if err != nil {
@@ -800,21 +816,24 @@ func TestTarWithOptionsChownOptsAlwaysOverridesIdPair(t *testing.T) {
 		{&TarOptions{ChownOpts: &idtools.Identity{UID: 1, GID: 1}, NoLchown: true}, 1, 1},
 		{&TarOptions{ChownOpts: &idtools.Identity{UID: 1000, GID: 1000}, NoLchown: true}, 1000, 1000},
 	}
-	for _, testCase := range cases {
-		reader, err := TarWithOptions(filePath, testCase.opts)
-		assert.NilError(t, err)
-		tr := tar.NewReader(reader)
-		defer reader.Close()
-		for {
-			hdr, err := tr.Next()
-			if err == io.EOF {
-				// end of tar archive
-				break
-			}
+	for _, tc := range cases {
+		tc := tc
+		t.Run("", func(t *testing.T) {
+			reader, err := TarWithOptions(filePath, tc.opts)
 			assert.NilError(t, err)
-			assert.Check(t, is.Equal(hdr.Uid, testCase.expectedUID), "Uid equals expected value")
-			assert.Check(t, is.Equal(hdr.Gid, testCase.expectedGID), "Gid equals expected value")
-		}
+			tr := tar.NewReader(reader)
+			defer reader.Close()
+			for {
+				hdr, err := tr.Next()
+				if err == io.EOF {
+					// end of tar archive
+					break
+				}
+				assert.NilError(t, err)
+				assert.Check(t, is.Equal(hdr.Uid, tc.expectedUID), "Uid equals expected value")
+				assert.Check(t, is.Equal(hdr.Gid, tc.expectedGID), "Gid equals expected value")
+			}
+		})
 	}
 }
 
diff --git a/pkg/archive/changes_posix_test.go b/pkg/archive/changes_posix_test.go
index 84864a6bcf..a28f26023c 100644
--- a/pkg/archive/changes_posix_test.go
+++ b/pkg/archive/changes_posix_test.go
@@ -24,14 +24,10 @@ func TestHardLinkOrder(t *testing.T) {
 	defer os.RemoveAll(src)
 	for _, name := range names {
 		func() {
-			fh, err := os.Create(path.Join(src, name))
+			err := os.WriteFile(path.Join(src, name), msg, 0666)
 			if err != nil {
 				t.Fatal(err)
 			}
-			defer fh.Close()
-			if _, err = fh.Write(msg); err != nil {
-				t.Fatal(err)
-			}
 		}()
 	}
 	// Create dest, with changes that includes hardlinks