From ccd99e331e2af15d68c4284709daa882c9439ad1 Mon Sep 17 00:00:00 2001
From: Chris Kim <chriskim06@gmail.com>
Date: Thu, 27 Feb 2020 07:16:01 -0800
Subject: [PATCH] Code review changes

---
 internal/index/indexscanner/scanner.go      | 14 ++----
 internal/index/indexscanner/scanner_test.go | 53 ---------------------
 internal/installation/util_test.go          |  2 +-
 internal/testutil/tempdir.go                |  2 +-
 4 files changed, 6 insertions(+), 65 deletions(-)

diff --git a/internal/index/indexscanner/scanner.go b/internal/index/indexscanner/scanner.go
index af11f865..58684e46 100644
--- a/internal/index/indexscanner/scanner.go
+++ b/internal/index/indexscanner/scanner.go
@@ -95,7 +95,6 @@ func ReadPluginFromFile(path string) (index.Plugin, error) {
 }
 
 func ReadPlugin(f io.ReadCloser) (index.Plugin, error) {
-	defer f.Close()
 	var plugin index.Plugin
 	err := decodeFile(f, &plugin)
 	if err != nil {
@@ -115,21 +114,16 @@ func ReadReceiptFromFile(path string) (index.Receipt, error) {
 // readFromFile loads a file from the FS into the provided object.
 func readFromFile(path string, as interface{}) error {
 	f, err := os.Open(path)
-	if os.IsNotExist(err) {
+	if err != nil {
 		return err
-	} else if err != nil {
-		return errors.Wrapf(err, "failed to open file %s", path)
 	}
-	defer f.Close()
 	err = decodeFile(f, &as)
-	if err != nil {
-		return errors.Wrap(err, "failed to decode plugin manifest")
-	}
-	return nil
+	return errors.Wrap(err, "failed to parse yaml file")
 }
 
 // decodeFile tries to decode a plugin/receipt
-func decodeFile(r io.Reader, as interface{}) error {
+func decodeFile(r io.ReadCloser, as interface{}) error {
+	defer r.Close()
 	b, err := ioutil.ReadAll(r)
 	if err != nil {
 		return err
diff --git a/internal/index/indexscanner/scanner_test.go b/internal/index/indexscanner/scanner_test.go
index c7804f8c..de6da1a2 100644
--- a/internal/index/indexscanner/scanner_test.go
+++ b/internal/index/indexscanner/scanner_test.go
@@ -19,11 +19,8 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-
-	"sigs.k8s.io/krew/internal/testutil"
 )
 
 func TestReadPluginFile(t *testing.T) {
@@ -93,56 +90,6 @@ func TestReadPluginFile(t *testing.T) {
 	}
 }
 
-func TestReadReceiptFile(t *testing.T) {
-	tests := []struct {
-		name            string
-		receiptFileName string
-		wantErr         bool
-		matchFirst      labels.Set
-	}{
-		{
-			name:            "read receipt file",
-			receiptFileName: "foo.yaml",
-			wantErr:         false,
-			matchFirst: labels.Set{
-				"arch": "amd64",
-				"os":   "linux",
-			},
-		},
-	}
-	neverMatch := labels.Set{}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			tmpDir, cleanup := testutil.NewTempDir(t)
-			defer cleanup()
-
-			receipt := testutil.NewPlugin().WithName("foo").V()
-			receiptFile := filepath.Join(tmpDir.Root(), "receipts", tt.receiptFileName)
-			tmpDir.WriteYaml(receiptFile, receipt)
-
-			got, err := ReadReceiptFromFile(receiptFile)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("ReadReceiptFromFile() error = %v, wantErr %v", err, tt.wantErr)
-			}
-			if err != nil {
-				t.Error(err)
-			}
-			if cmp.Diff(got.Plugin, receipt) != "" {
-				t.Errorf("ReadReceiptFromFile() has not parsed the receipt properly: %+v", got)
-			}
-
-			sel, err := metav1.LabelSelectorAsSelector(got.Spec.Platforms[0].Selector)
-			if err != nil {
-				t.Errorf("ReadReceiptFromFile() error parsing label err: %v", err)
-			}
-			if !sel.Matches(tt.matchFirst) || sel.Matches(neverMatch) {
-				t.Errorf("ReadReceiptFromFile() didn't parse label selector properly: %##v", sel)
-			}
-		})
-	}
-}
-
 func TestReadPluginFile_preservesNotFoundErr(t *testing.T) {
 	_, err := ReadPluginFromFile(filepath.Join(testdataPath(t), "does-not-exist.yaml"))
 	if err == nil {
diff --git a/internal/installation/util_test.go b/internal/installation/util_test.go
index c37c23fa..7d6bd4d4 100644
--- a/internal/installation/util_test.go
+++ b/internal/installation/util_test.go
@@ -65,7 +65,7 @@ func TestListInstalledPlugins(t *testing.T) {
 			defer cleanup()
 
 			for _, plugin := range test.plugins {
-				tempDir.WriteYaml(plugin.Name+constants.ManifestExtension, plugin)
+				tempDir.WriteYAML(plugin.Name+constants.ManifestExtension, plugin)
 			}
 
 			actual, err := ListInstalledPlugins(tempDir.Root())
diff --git a/internal/testutil/tempdir.go b/internal/testutil/tempdir.go
index 6878fdcf..9b28feea 100644
--- a/internal/testutil/tempdir.go
+++ b/internal/testutil/tempdir.go
@@ -75,7 +75,7 @@ func (td *TempDir) Write(file string, content []byte) *TempDir {
 	return td
 }
 
-func (td *TempDir) WriteYaml(file string, obj interface{}) *TempDir {
+func (td *TempDir) WriteYAML(file string, obj interface{}) *TempDir {
 	td.t.Helper()
 	content, err := yaml.Marshal(obj)
 	if err != nil {