From 9e4b00b128dc65dcb080f4125311ec74e9ed1de7 Mon Sep 17 00:00:00 2001
From: Benjamin Elder <bentheelder@google.com>
Date: Fri, 31 Jan 2025 13:33:25 -0800
Subject: [PATCH] version test cleanup: don't mutate globals, use pre-computed
 expected values

---
 pkg/cmd/kind/version/version.go      | 20 ++++++++-----
 pkg/cmd/kind/version/version_test.go | 45 +++++++++++-----------------
 2 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/pkg/cmd/kind/version/version.go b/pkg/cmd/kind/version/version.go
index 743f6c828c..a4bbc06126 100644
--- a/pkg/cmd/kind/version/version.go
+++ b/pkg/cmd/kind/version/version.go
@@ -29,19 +29,23 @@ import (
 
 // Version returns the kind CLI Semantic Version
 func Version() string {
-	v := versionCore
+	return version(versionCore, versionPreRelease, gitCommit, gitCommitCount)
+}
+
+func version(core, preRelease, commit, commitCount string) string {
+	v := core
 	// add pre-release version info if we have it
-	if versionPreRelease != "" {
-		v += "-" + versionPreRelease
-		// If gitCommitCount was set, add to the pre-release version
-		if gitCommitCount != "" {
-			v += "." + gitCommitCount
+	if preRelease != "" {
+		v += "-" + preRelease
+		// If commitCount was set, add to the pre-release version
+		if commitCount != "" {
+			v += "." + commitCount
 		}
 		// if commit was set, add the + <build>
 		// we only do this for pre-release versions
-		if gitCommit != "" {
+		if commit != "" {
 			// NOTE: use 14 character short hash, like Kubernetes
-			v += "+" + truncate(gitCommit, 14)
+			v += "+" + truncate(commit, 14)
 		}
 	}
 	return v
diff --git a/pkg/cmd/kind/version/version_test.go b/pkg/cmd/kind/version/version_test.go
index 06ff9373d3..08a236a797 100644
--- a/pkg/cmd/kind/version/version_test.go
+++ b/pkg/cmd/kind/version/version_test.go
@@ -64,6 +64,7 @@ func TestTruncate(t *testing.T) {
 func TestVersion(t *testing.T) {
 	tests := []struct {
 		name              string
+		version           string
 		versionPreRelease string
 		gitCommit         string
 		gitCommitCount    string
@@ -71,71 +72,59 @@ func TestVersion(t *testing.T) {
 	}{
 		{
 			name:              "With git commit count and with commit hash",
+			version:           "v0.27.0",
 			versionPreRelease: "alpha",
 			gitCommit:         "mocked-hash",
 			gitCommitCount:    "mocked-count",
-			want:              versionCore + "-" + "alpha" + "." + "mocked-count" + "+" + "mocked-hash",
+			want:              "v0.27.0-alpha.mocked-count+mocked-hash",
 		},
 		{
 			name:              "Without git commit count and and with hash",
+			version:           "v0.27.0",
 			versionPreRelease: "beta",
 			gitCommit:         "mocked-hash",
 			gitCommitCount:    "",
-			want:              versionCore + "-" + "beta" + "+" + "mocked-hash",
+			want:              "v0.27.0-beta+mocked-hash",
 		},
 		{
 			name:              "Without git commit hash and with commit count",
+			version:           "v0.30.0",
 			versionPreRelease: "alpha",
 			gitCommit:         "",
 			gitCommitCount:    "mocked-count",
-			want:              versionCore + "-" + "alpha" + "." + "mocked-count",
+			want:              "v0.30.0-alpha.mocked-count",
 		},
 		{
 			name:              "Without git commit hash and without commit count",
+			version:           "v0.27.0",
 			versionPreRelease: "alpha",
 			gitCommit:         "",
 			gitCommitCount:    "",
-			want:              versionCore + "-" + "alpha",
+			want:              "v0.27.0-alpha",
 		},
 		{
 			name:              "Without pre release version",
+			version:           "v0.27.0",
 			versionPreRelease: "",
 			gitCommit:         "",
 			gitCommitCount:    "",
-			want:              versionCore,
+			want:              "v0.27.0",
 		},
 		{
 			name:              "Without pre release version and with git commit hash and count",
+			version:           "v0.27.0",
 			versionPreRelease: "",
 			gitCommit:         "mocked-commit",
 			gitCommitCount:    "mocked-count",
-			want:              versionCore,
+			want:              "v0.27.0",
 		},
 	}
 	for _, tt := range tests {
+		// TODO: this won't be necessary when we require go 1.22+
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			if tt.gitCommit != "" {
-				gitCommitBackup := gitCommit
-				gitCommit = tt.gitCommit
-				defer func() {
-					gitCommit = gitCommitBackup
-				}()
-			}
-
-			if tt.gitCommitCount != "" {
-				gitCommitCountBackup := gitCommitCount
-				gitCommitCount = tt.gitCommitCount
-				defer func() {
-					gitCommitCount = gitCommitCountBackup
-				}()
-			}
-
-			versionPreReleaseBackup := versionPreRelease
-			versionPreRelease = tt.versionPreRelease
-			defer func() {
-				versionPreRelease = versionPreReleaseBackup
-			}()
-			if got := Version(); got != tt.want {
+			t.Parallel()
+			if got := version(tt.version, tt.versionPreRelease, tt.gitCommit, tt.gitCommitCount); got != tt.want {
 				t.Errorf("Version() = %v, want %v", got, tt.want)
 			}
 		})