Merge pull request #3856 from BenTheElder/cleanup2

version test cleanup: don't mutate globals, use pre-computed expected values

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

pkg/cmd/kind/version/version_test.go

@@ -64,78 +64,67 @@ func TestTruncate(t *testing.T) {
 func TestVersion(t *testing.T) {
 	tests := []struct {
 		name              string
+		version           string
 		versionPreRelease string
 		gitCommit         string
 		gitCommitCount    string
 		want              string
 	}{
 		{
 			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)
 			}
 		})