Repository: kubernetes-sigs/krew
Pull request #435 (2019-12-13 01:31:29)

Improve linting and fix found issues
Enable all rules from gocritic except some troublesome rules (see `.golangci.yml` for details).

Here is a part of what new rules bring:
* don't shadow imports or builtins such as
`os` and `close` respectively.
* use wrapper ReplaceAll
* combine types in func calls if same type
* find duplicate imports
* find suspicious regex
* ensure proper comment format

To fix some of the issues, I created a type for os and arch, then exported it.

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/435#" title="Author self-approved">ferhatelmas</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **corneliusweig**
You can assign the PR to them by writing `/assign @corneliusweig` 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=kubernetes-sigs%2Fkrew).

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

- **[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/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":["corneliusweig"]} -->
codecov-io: # [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=h1) Report
> Merging [#435](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=desc) into [master](https://codecov.io/gh/kubernetes-sigs/krew/commit/541b48bf85579ffde7e70236d4f8e58b3359a975?src=pr&el=desc) will **decrease** coverage by `0.06%`.
> The diff coverage is `75%`.

[![Impacted file tree graph](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/graphs/tree.svg?width=650&token=J3nFtARI2m&height=150&src=pr)](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
- Coverage   56.35%   56.29%   -0.07%     
==========================================
  Files          19       19              
  Lines         928      929       +1     
==========================================
  Hits          523      523              
- Misses        350      351       +1     
  Partials       55       55
```


| [Impacted Files](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/installation/upgrade.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/diff?src=pr&el=tree#diff-aW50ZXJuYWwvaW5zdGFsbGF0aW9uL3VwZ3JhZGUuZ28=) | `0% <0%> (ø)` | :arrow_up: |
| [internal/download/verifier.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZG93bmxvYWQvdmVyaWZpZXIuZ28=) | `100% <100%> (ø)` | :arrow_up: |
| [internal/receiptsmigration/migration.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcmVjZWlwdHNtaWdyYXRpb24vbWlncmF0aW9uLmdv) | `46.15% <100%> (ø)` | :arrow_up: |
| [internal/installation/install.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/diff?src=pr&el=tree#diff-aW50ZXJuYWwvaW5zdGFsbGF0aW9uL2luc3RhbGwuZ28=) | `42.42% <100%> (ø)` | :arrow_up: |
| [internal/download/downloader.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZG93bmxvYWQvZG93bmxvYWRlci5nbw==) | `68.14% <60%> (-0.28%)` | :arrow_down: |
| [internal/installation/platform.go](https://codecov.io/gh/kubernetes-sigs/krew/pull/435/diff?src=pr&el=tree#diff-aW50ZXJuYWwvaW5zdGFsbGF0aW9uL3BsYXRmb3JtLmdv) | `76.92% <80%> (-2.25%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=footer). Last update [541b48b...1273c64](https://codecov.io/gh/kubernetes-sigs/krew/pull/435?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ferhatelmas: @ahmetb It was mostly from my reading but checked linters and added more rules 
 and it found more issues so I added more refactoring.
corneliusweig: /lgtm

Patch:
diff --git a/.golangci.yml b/.golangci.yml
index 0fc5c40..be0ee52 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -1,5 +1,16 @@
 # all available settings of specific linters
 linters-settings:
+  gocritic:
+    enabled-tags:
+      - performance
+      - diagnostic
+      - style
+      - experimental
+      - opinionated
+    disabled-checks:
+      - hugeParam
+      - rangeValCopy
+      - unnamedResult
   gofmt:
     simplify: true
   goimports:
diff --git a/cmd/generate-plugin-overview/generate-plugin-overview.go b/cmd/generate-plugin-overview/generate-plugin-overview.go
index 4aebed5..214d813 100644
--- a/cmd/generate-plugin-overview/generate-plugin-overview.go
+++ b/cmd/generate-plugin-overview/generate-plugin-overview.go
@@ -55,7 +55,7 @@ tool._
 )
 
 var (
-	githubRepoPattern = regexp.MustCompile(`.*github.com/([^/]+/[^/#]+)`)
+	githubRepoPattern = regexp.MustCompile(`.*github\.com/([^/]+/[^/#]+)`)
 )
 
 func main() {
diff --git a/cmd/validate-krew-manifest/main.go b/cmd/validate-krew-manifest/main.go
index 0f86a49..513cca1 100644
--- a/cmd/validate-krew-manifest/main.go
+++ b/cmd/validate-krew-manifest/main.go
@@ -32,6 +32,7 @@ import (
 
 	"sigs.k8s.io/krew/internal/index/indexscanner"
 	"sigs.k8s.io/krew/internal/index/validation"
+	"sigs.k8s.io/krew/internal/installation"
 	"sigs.k8s.io/krew/pkg/constants"
 	"sigs.k8s.io/krew/pkg/index"
 )
@@ -83,7 +84,7 @@ func validateManifestFile(path string) error {
 
 	// make sure each platform matches a supported platform
 	for i, p := range p.Spec.Platforms {
-		if os, arch := findAnyMatchingPlatform(p.Selector); os == "" || arch == "" {
+		if env := findAnyMatchingPlatform(p.Selector); env.OS == "" || env.Arch == "" {
 			return errors.Errorf("spec.platform[%d]'s selector (%v) doesn't match any supported platforms", i, p.Selector)
 		}
 	}
@@ -110,18 +111,16 @@ func validateManifestFile(path string) error {
 // isOverlappingPlatformSelectors validates if multiple platforms have selectors
 // that match to a supported <os,arch> pair.
 func isOverlappingPlatformSelectors(platforms []index.Platform) error {
-	for _, v := range allPlatforms() {
-		os, arch := v[0], v[1]
-
+	for _, env := range allPlatforms() {
 		var matchIndex []int
 		for i, p := range platforms {
-			if selectorMatchesOSArch(p.Selector, os, arch) {
+			if selectorMatchesOSArch(p.Selector, env) {
 				matchIndex = append(matchIndex, i)
 			}
 		}
 
 		if len(matchIndex) > 1 {
-			return errors.Errorf("multiple spec.platforms (at indexes %v) have overlapping selectors that select os=%s/arch=%s", matchIndex, os, arch)
+			return errors.Errorf("multiple spec.platforms (at indexes %v) have overlapping selectors that select %s", matchIndex, env)
 		}
 	}
 	return nil
@@ -130,8 +129,8 @@ func isOverlappingPlatformSelectors(platforms []index.Platform) error {
 // installPlatformSpec installs the p to a temporary location on disk to verify
 // by shelling out to external command.
 func installPlatformSpec(manifestFile string, p index.Platform) error {
-	goos, goarch := findAnyMatchingPlatform(p.Selector)
-	if goos == "" || goarch == "" {
+	env := findAnyMatchingPlatform(p.Selector)
+	if env.OS == "" || env.Arch == "" {
 		return errors.Errorf("no supported platform matched platform selector: %+v", p.Selector)
 	}
 
@@ -149,33 +148,33 @@ func installPlatformSpec(manifestFile string, p index.Platform) error {
 	cmd.Stdin = nil
 	cmd.Env = []string{
 		"KREW_ROOT=" + tmpDir,
-		"KREW_OS=" + goos,
-		"KREW_ARCH=" + goarch,
+		"KREW_OS=" + env.OS,
+		"KREW_ARCH=" + env.Arch,
 	}
 	klog.V(2).Infof("installing plugin with: %+v", cmd.Env)
 	cmd.Env = append(cmd.Env, "PATH="+os.Getenv("PATH"))
 
 	b, err := cmd.CombinedOutput()
 	if err != nil {
-		output := strings.Replace(string(b), "\n", "\n\t", -1)
+		output := strings.ReplaceAll(string(b), "\n", "\n\t")
 		return errors.Wrapf(err, "plugin install command failed: %s", output)
 	}
 	return nil
 }
 
 // findAnyMatchingPlatform finds an <os,arch> pair matches to given selector
-func findAnyMatchingPlatform(selector *metav1.LabelSelector) (string, string) {
+func findAnyMatchingPlatform(selector *metav1.LabelSelector) installation.OSArchPair {
 	for _, p := range allPlatforms() {
-		if selectorMatchesOSArch(selector, p[0], p[1]) {
-			klog.V(4).Infof("%s MATCHED <%s,%s>", selector, p[0], p[1])
-			return p[0], p[1]
+		if selectorMatchesOSArch(selector, p) {
+			klog.V(4).Infof("%s MATCHED <%s>", selector, p)
+			return p
 		}
-		klog.V(4).Infof("%s didn't match <%s,%s>", selector, p[0], p[1])
+		klog.V(4).Infof("%s didn't match <%s>", selector, p)
 	}
-	return "", ""
+	return installation.OSArchPair{}
 }
 
-func selectorMatchesOSArch(selector *metav1.LabelSelector, os, arch string) bool {
+func selectorMatchesOSArch(selector *metav1.LabelSelector, env installation.OSArchPair) bool {
 	sel, err := metav1.LabelSelectorAsSelector(selector)
 	if err != nil {
 		// this should've been caught by validation.ValidatePlatform() earlier
@@ -183,21 +182,21 @@ func selectorMatchesOSArch(selector *metav1.LabelSelector, os, arch string) bool
 		return false
 	}
 	return sel.Matches(labels.Set{
-		"os":   os,
-		"arch": arch,
+		"os":   env.OS,
+		"arch": env.Arch,
 	})
 }
 
 // allPlatforms returns all <os,arch> pairs krew is supported on.
-func allPlatforms() [][2]string {
+func allPlatforms() []installation.OSArchPair {
 	// TODO(ahmetb) find a more authoritative source for this list
-	return [][2]string{
-		{"windows", "386"},
-		{"windows", "amd64"},
-		{"linux", "386"},
-		{"linux", "amd64"},
-		{"linux", "arm"},
-		{"darwin", "386"},
-		{"darwin", "amd64"},
+	return []installation.OSArchPair{
+		{OS: "windows", Arch: "386"},
+		{OS: "windows", Arch: "amd64"},
+		{OS: "linux", Arch: "386"},
+		{OS: "linux", Arch: "amd64"},
+		{OS: "linux", Arch: "arm"},
+		{OS: "darwin", Arch: "386"},
+		{OS: "darwin", Arch: "amd64"},
 	}
 }
diff --git a/docs/DEVELOPER_GUIDE.md b/docs/DEVELOPER_GUIDE.md
index 5ea291f..6312f00 100644
--- a/docs/DEVELOPER_GUIDE.md
+++ b/docs/DEVELOPER_GUIDE.md
@@ -69,11 +69,12 @@ To package a plugin via krew, you need to:
 2. Write a [plugin manifest file](#writing-a-plugin-manifest).
 3. Test installation locally with the archive and the manifest file.
 
-To make a plugin available to everyone via krew, you need to:
+To publish a plugin in `krew-index` to make it available via krew, you need to:
 
+1. Make the plugin source code publicly available.
 1. Make the archive file (`.zip` or `.tar.gz`) **publicly downloadable** on a
    URL (you can host it yourself or use GitHub releases feature).
-2. Submit the plugin manifest file to the [krew index][index] repository.
+1. Submit the plugin manifest file to the [krew index][index] repository.
 
 Plugin packages need to be available to download from the public Internet.
 A service like
diff --git a/internal/download/downloader.go b/internal/download/downloader.go
index 3ea5c26..2890b3a 100644
--- a/internal/download/downloader.go
+++ b/internal/download/downloader.go
@@ -80,16 +80,16 @@ func extractZIP(targetDir string, read io.ReaderAt, size int64) error {
 			src.Close()
 			return errors.Wrap(err, "can't create file in zip destination dir")
 		}
-		close := func() {
+		closeAll := func() {
 			src.Close()
 			dst.Close()
 		}
 
 		if _, err := io.Copy(dst, src); err != nil {
-			close()
+			closeAll()
 			return errors.Wrap(err, "can't copy content to zip destination file")
 		}
-		close()
+		closeAll()
 	}
 
 	return nil
@@ -142,12 +142,12 @@ func extractTARGZ(targetDir string, at io.ReaderAt, size int64) error {
 			if err != nil {
 				return errors.Wrapf(err, "failed to create file %q", path)
 			}
-			close := func() { f.Close() }
+
 			if _, err := io.Copy(f, tr); err != nil {
-				close()
+				f.Close()
 				return errors.Wrapf(err, "failed to copy %q from tar into file", hdr.Name)
 			}
-			close()
+			f.Close()
 		default:
 			return errors.Errorf("unable to handle file type %d for %q in tar", hdr.Typeflag, hdr.Name)
 		}
diff --git a/internal/download/verifier.go b/internal/download/verifier.go
index 8db95a3..f4adb25 100644
--- a/internal/download/verifier.go
+++ b/internal/download/verifier.go
@@ -39,8 +39,8 @@ type sha256Verifier struct {
 }
 
 // NewSha256Verifier creates a Verifier that tests against the given hash.
-func NewSha256Verifier(hash string) Verifier {
-	raw, _ := hex.DecodeString(hash)
+func NewSha256Verifier(hashed string) Verifier {
+	raw, _ := hex.DecodeString(hashed)
 	return sha256Verifier{
 		Hash:       sha256.New(),
 		wantedHash: raw,
diff --git a/internal/installation/install.go b/internal/installation/install.go
index 762f13f..c283778 100644
--- a/internal/installation/install.go
+++ b/internal/installation/install.go
@@ -186,7 +186,7 @@ func Uninstall(p environment.Paths, name string) error {
 	return errors.Wrapf(err, "could not remove plugin receipt %q", pluginReceiptPath)
 }
 
-func createOrUpdateLink(binDir string, binary string, plugin string) error {
+func createOrUpdateLink(binDir, binary, plugin string) error {
 	dst := filepath.Join(binDir, pluginNameToBin(plugin, IsWindows()))
 
 	if err := removeLink(dst); err != nil {
@@ -238,7 +238,7 @@ func IsWindows() bool {
 // pluginNameToBin creates the name of the symlink file for the plugin name.
 // It converts dashes to underscores.
 func pluginNameToBin(name string, isWindows bool) string {
-	name = strings.Replace(name, "-", "_", -1)
+	name = strings.ReplaceAll(name, "-", "_")
 	name = "kubectl-" + name
 	if isWindows {
 		name += ".exe"
@@ -247,7 +247,7 @@ func pluginNameToBin(name string, isWindows bool) string {
 }
 
 // CleanupStaleKrewInstallations removes the versions that aren't the current version.
-func CleanupStaleKrewInstallations(dir string, currentVersion string) error {
+func CleanupStaleKrewInstallations(dir, currentVersion string) error {
 	ls, err := ioutil.ReadDir(dir)
 	if err != nil {
 		return errors.Wrap(err, "failed to read krew store directory")
diff --git a/internal/installation/platform.go b/internal/installation/platform.go
index 6875445..3d1f930 100644
--- a/internal/installation/platform.go
+++ b/internal/installation/platform.go
@@ -15,6 +15,7 @@
 package installation
 
 import (
+	"fmt"
 	"os"
 	"runtime"
 
@@ -27,18 +28,17 @@ import (
 )
 
 // GetMatchingPlatform finds the platform spec in the specified plugin that
-// matches the OS/arch of the current machine (can be overridden via KREW_OS
+// matches the os/arch of the current machine (can be overridden via KREW_OS
 // and/or KREW_ARCH).
 func GetMatchingPlatform(platforms []index.Platform) (index.Platform, bool, error) {
-	os, arch := osArch()
-	return matchPlatform(platforms, os, arch)
+	return matchPlatform(platforms, OSArch())
 }
 
 // matchPlatform returns the first matching platform to given os/arch.
-func matchPlatform(platforms []index.Platform, os, arch string) (index.Platform, bool, error) {
+func matchPlatform(platforms []index.Platform, env OSArchPair) (index.Platform, bool, error) {
 	envLabels := labels.Set{
-		"os":   os,
-		"arch": arch,
+		"os":   env.OS,
+		"arch": env.Arch,
 	}
 	klog.V(2).Infof("Matching platform for labels(%v)", envLabels)
 
@@ -55,16 +55,29 @@ func matchPlatform(platforms []index.Platform, os, arch string) (index.Platform,
 	return index.Platform{}, false, nil
 }
 
-// osArch returns the OS/arch combination to be used on the current system. It
+// OSArchPair is wrapper around operating system and architecture
+type OSArchPair struct {
+	OS, Arch string
+}
+
+// String converts environment into a string
+func (p OSArchPair) String() string {
+	return fmt.Sprintf("%s/%s", p.OS, p.Arch)
+}
+
+// OSArch returns the OS/arch combination to be used on the current system. It
 // can be overridden by setting KREW_OS and/or KREW_ARCH environment variables.
-func osArch() (string, string) {
-	goos, goarch := runtime.GOOS, runtime.GOARCH
-	envOS, envArch := os.Getenv("KREW_OS"), os.Getenv("KREW_ARCH")
-	if envOS != "" {
-		goos = envOS
+func OSArch() OSArchPair {
+	return OSArchPair{
+		OS:   getEnvOrDefault("KREW_OS", runtime.GOOS),
+		Arch: getEnvOrDefault("KREW_ARCH", runtime.GOARCH),
 	}
-	if envArch != "" {
-		goarch = envArch
+}
+
+func getEnvOrDefault(env, absent string) string {
+	v := os.Getenv(env)
+	if v != "" {
+		return v
 	}
-	return goos, goarch
+	return absent
 }
diff --git a/internal/installation/upgrade.go b/internal/installation/upgrade.go
index cfa853b..b329a98 100644
--- a/internal/installation/upgrade.go
+++ b/internal/installation/upgrade.go
@@ -48,8 +48,8 @@ func Upgrade(p environment.Paths, plugin index.Plugin) error {
 		return errors.Wrap(err, "failed trying to find a matching platform in plugin spec")
 	}
 	if !ok {
-		os, arch := osArch()
-		return errors.Errorf("plugin %q does not offer installation for this platform (%s/%s)", plugin.Name, os, arch)
+		return errors.Errorf("plugin %q does not offer installation for this platform (%s)",
+			plugin.Name, OSArch())
 	}
 
 	newVersion := plugin.Spec.Version
diff --git a/internal/receiptsmigration/migration.go b/internal/receiptsmigration/migration.go
index 37f6c49..e0b49a8 100644
--- a/internal/receiptsmigration/migration.go
+++ b/internal/receiptsmigration/migration.go
@@ -193,7 +193,7 @@ func removeLink(path string) error {
 	return nil
 }
 
-/// same as pkg/installation/install.go:186
+// same as pkg/installation/install.go:186
 func isWindows() bool {
 	goos := runtime.GOOS
 	if env := os.Getenv("KREW_OS"); env != "" {
@@ -206,7 +206,7 @@ func isWindows() bool {
 // It converts dashes to underscores.
 // same as pkg/installation/install.go:196
 func pluginNameToBin(name string, isWindows bool) string {
-	name = strings.Replace(name, "-", "_", -1)
+	name = strings.ReplaceAll(name, "-", "_")
 	name = "kubectl-" + name
 	if isWindows {
 		name += ".exe"


Test patch:
diff --git a/cmd/validate-krew-manifest/main_test.go b/cmd/validate-krew-manifest/main_test.go
index d3619eb..cffef1e 100644
--- a/cmd/validate-krew-manifest/main_test.go
+++ b/cmd/validate-krew-manifest/main_test.go
@@ -20,9 +20,9 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
 
+	"sigs.k8s.io/krew/internal/installation"
 	"sigs.k8s.io/krew/internal/testutil"
 	"sigs.k8s.io/krew/pkg/index"
 )
@@ -112,30 +112,57 @@ func TestValidateManifestFile(t *testing.T) {
 func Test_selectorMatchesOSArch(t *testing.T) {
 	type args struct {
 		selector *metav1.LabelSelector
-		os       string
-		arch     string
+		env      installation.OSArchPair
 	}
 	tests := []struct {
 		name string
 		args args
 		want bool
 	}{
-		{"label - no match", args{&metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}, "windows", "amd64"}, false},
-		{"label - match", args{&metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}, "darwin", "amd64"}, true},
-		{"expression - no match", args{&metav1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
-			Key:      "os",
-			Operator: v1.LabelSelectorOpIn,
-			Values:   []string{"darwin", "linux"},
-		}}}, "windows", "amd64"}, false},
-		{"expression - match", args{&metav1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
-			Key:      "os",
-			Operator: v1.LabelSelectorOpIn,
-			Values:   []string{"darwin", "linux"},
-		}}}, "darwin", "amd64"}, true},
+		{
+			name: "label - no match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}},
+				env:      installation.OSArchPair{OS: "windows", Arch: "amd64"},
+			},
+			want: false,
+		},
+		{
+			name: "label - match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}},
+				env:      installation.OSArchPair{OS: "darwin", Arch: "amd64"},
+			},
+			want: true,
+		},
+		{
+			name: "expression - no match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
+					Key:      "os",
+					Operator: metav1.LabelSelectorOpIn,
+					Values:   []string{"darwin", "linux"},
+				}}},
+				env: installation.OSArchPair{OS: "windows", Arch: "amd64"},
+			},
+			want: false,
+		},
+		{
+			name: "expression - match",
+			args: args{
+				selector: &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
+					Key:      "os",
+					Operator: metav1.LabelSelectorOpIn,
+					Values:   []string{"darwin", "linux"},
+				}}},
+				env: installation.OSArchPair{OS: "darwin", Arch: "amd64"},
+			},
+			want: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := selectorMatchesOSArch(tt.args.selector, tt.args.os, tt.args.arch); got != tt.want {
+			if got := selectorMatchesOSArch(tt.args.selector, tt.args.env); got != tt.want {
 				t.Errorf("selectorMatchesOSArch() = %v, want %v", got, tt.want)
 			}
 		})
@@ -143,24 +170,24 @@ func Test_selectorMatchesOSArch(t *testing.T) {
 }
 
 func Test_findAnyMatchingPlatform(t *testing.T) {
-	s1 := &v1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}
-	o1, a1 := findAnyMatchingPlatform(s1)
-	if o1 == "" || a1 == "" {
+	s1 := &metav1.LabelSelector{MatchLabels: map[string]string{"os": "darwin"}}
+	env := findAnyMatchingPlatform(s1)
+	if env.OS == "" || env.Arch == "" {
 		t.Fatalf("with selector %v, expected os/arch", s1)
 	}
 
-	s2 := &v1.LabelSelector{MatchLabels: map[string]string{"os": "non-existing"}}
-	o2, a2 := findAnyMatchingPlatform(s1)
-	if o2 == "" && a2 == "" {
+	s2 := &metav1.LabelSelector{MatchLabels: map[string]string{"os": "non-existing"}}
+	env2 := findAnyMatchingPlatform(s1)
+	if env2.OS == "" && env2.Arch == "" {
 		t.Fatalf("with selector %v, expected os/arch", s2)
 	}
 
-	s3 := &v1.LabelSelector{MatchExpressions: []v1.LabelSelectorRequirement{{
+	s3 := &metav1.LabelSelector{MatchExpressions: []metav1.LabelSelectorRequirement{{
 		Key:      "os",
-		Operator: v1.LabelSelectorOpIn,
+		Operator: metav1.LabelSelectorOpIn,
 		Values:   []string{"darwin", "linux"}}}}
-	o3, a3 := findAnyMatchingPlatform(s3)
-	if o3 == "" || a3 == "" {
+	env3 := findAnyMatchingPlatform(s3)
+	if env3.OS == "" || env3.Arch == "" {
 		t.Fatalf("with selector %v, expected os/arch", s2)
 	}
 }
diff --git a/internal/installation/install_test.go b/internal/installation/install_test.go
index 3d2aee8..8a10014 100644
--- a/internal/installation/install_test.go
+++ b/internal/installation/install_test.go
@@ -299,8 +299,8 @@ func Test_applyDefaults(t *testing.T) {
 }
 
 func TestCleanupStaleKrewInstallations(t *testing.T) {
-	dir, close := testutil.NewTempDir(t)
-	defer close()
+	dir, cleanup := testutil.NewTempDir(t)
+	defer cleanup()
 
 	testFiles := []string{
 		"dir1/f1.txt",
diff --git a/internal/installation/platform_test.go b/internal/installation/platform_test.go
index 874334b..e5778f7 100644
--- a/internal/installation/platform_test.go
+++ b/internal/installation/platform_test.go
@@ -26,41 +26,34 @@ import (
 )
 
 func Test_osArch(t *testing.T) {
-	inOS, inArch := runtime.GOOS, runtime.GOARCH
-	outOS, outArch := osArch()
-	if inOS != outOS {
-		t.Errorf("returned OS=%q; expected=%q", outOS, inOS)
-	}
-	if inArch != outArch {
-		t.Errorf("returned Arch=%q; expected=%q", outArch, inArch)
+	in := OSArchPair{OS: runtime.GOOS, Arch: runtime.GOARCH}
+
+	if diff := cmp.Diff(in, OSArch()); diff != "" {
+		t.Errorf("os/arch got a different result:\n%s", diff)
 	}
 }
 
 func Test_osArch_override(t *testing.T) {
-	customOS, customArch := "dragons", "metav1"
-	os.Setenv("KREW_OS", customOS)
-	os.Setenv("KREW_ARCH", customArch)
+	customGoOSArch := OSArchPair{OS: "dragons", Arch: "metav1"}
+	os.Setenv("KREW_OS", customGoOSArch.OS)
+	os.Setenv("KREW_ARCH", customGoOSArch.Arch)
 	defer func() {
 		os.Unsetenv("KREW_ARCH")
 		os.Unsetenv("KREW_OS")
 	}()
 
-	outOS, outArch := osArch()
-	if customOS != outOS {
-		t.Errorf("returned OS=%q; expected=%q", outOS, customOS)
-	}
-	if customArch != outArch {
-		t.Errorf("returned Arch=%q; expected=%q", outArch, customArch)
+	if diff := cmp.Diff(customGoOSArch, OSArch()); diff != "" {
+		t.Errorf("os/arch override got a different result:\n%s", diff)
 	}
 }
 
 func Test_matchPlatform(t *testing.T) {
-	const targetOS, targetArch = "foo", "amd64"
-	matchingPlatform := testutil.NewPlatform().WithOSArch(targetOS, targetArch).V()
-	differentOS := testutil.NewPlatform().WithOSArch("other", targetArch).V()
-	differentArch := testutil.NewPlatform().WithOSArch(targetOS, "other").V()
+	target := OSArchPair{OS: "foo", Arch: "amd64"}
+	matchingPlatform := testutil.NewPlatform().WithOSArch(target.OS, target.Arch).V()
+	differentOS := testutil.NewPlatform().WithOSArch("other", target.Arch).V()
+	differentArch := testutil.NewPlatform().WithOSArch(target.OS, "other").V()
 
-	p, ok, err := matchPlatform([]index.Platform{differentOS, differentArch, matchingPlatform}, targetOS, targetArch)
+	p, ok, err := matchPlatform([]index.Platform{differentOS, differentArch, matchingPlatform}, target)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -71,7 +64,7 @@ func Test_matchPlatform(t *testing.T) {
 		t.Fatalf("got a different object from the matching platform:\n%s", diff)
 	}
 
-	_, ok, err = matchPlatform([]index.Platform{differentOS, differentArch}, targetOS, targetArch)
+	_, ok, err = matchPlatform([]index.Platform{differentOS, differentArch}, target)
 	if err != nil {
 		t.Fatal(err)
 	}