Always use the same technic to cleanup global variables in tests.

cmd/skaffold/app/cmd/build_test.go

@@ -37,10 +37,6 @@ func TestQuietFlag(t *testing.T) {
 		}}, nil
 	}
 
-	defer func(f func(context.Context, io.Writer) ([]build.Artifact, error)) {
-		createRunnerAndBuildFunc = f
-	}(createRunnerAndBuildFunc)
-
 	var tests = []struct {
 		description    string
 		template       string
@@ -72,13 +68,17 @@ func TestQuietFlag(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
+			defer func(flag bool) { quietFlag = flag }(quietFlag)
 			quietFlag = true
-			defer func() { quietFlag = false }()
+
+			defer func(tf *flags.TemplateFlag) { buildFormatFlag = tf }(buildFormatFlag)
 			if test.template != "" {
 				buildFormatFlag = flags.NewTemplateFlag(test.template, flags.BuildOutput{})
 			}
-			defer func() { buildFormatFlag = nil }()
+
+			defer func(f func(context.Context, io.Writer) ([]build.Artifact, error)) { createRunnerAndBuildFunc = f }(createRunnerAndBuildFunc)
 			createRunnerAndBuildFunc = test.mock
+
 			var output bytes.Buffer
 			err := runBuild(&output)
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, string(test.expectedOutput), output.String())
@@ -96,9 +96,6 @@ func TestRunBuild(t *testing.T) {
 			Tag:       "test",
 		}}, nil
 	}
-	defer func(f func(context.Context, io.Writer) ([]build.Artifact, error)) {
-		createRunnerAndBuildFunc = f
-	}(createRunnerAndBuildFunc)
 
 	var tests = []struct {
 		description string
@@ -118,7 +115,9 @@ func TestRunBuild(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
+			defer func(f func(context.Context, io.Writer) ([]build.Artifact, error)) { createRunnerAndBuildFunc = f }(createRunnerAndBuildFunc)
 			createRunnerAndBuildFunc = test.mock
+
 			err := runBuild(ioutil.Discard)
 			testutil.CheckError(t, test.shouldErr, err)
 		})

pkg/skaffold/build/cache/cache_test.go

@@ -147,13 +147,10 @@ func Test_NewCache(t *testing.T) {
 			}
 			test.opts.CacheFile = cacheFile
 
-			originalDockerClient := newDockerClient
+			defer func(c func(bool, map[string]bool) (docker.LocalDaemon, error)) { newDockerClient = c }(newDockerClient)
 			newDockerClient = func(forceRemove bool, insecureRegistries map[string]bool) (docker.LocalDaemon, error) {
 				return docker.NewLocalDaemon(test.api, nil, forceRemove, test.insecureRegistries), nil
 			}
-			defer func() {
-				newDockerClient = originalDockerClient
-			}()
 
 			if test.updateClient {
 				test.expectedCache.client = docker.NewLocalDaemon(test.api, nil, false, test.insecureRegistries)

pkg/skaffold/build/cache/hash_test.go

@@ -67,11 +67,8 @@ func TestGetHashForArtifact(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			originalHasher := cacheHasher
+			defer func(h func(string) (string, error)) { hashFunction = h }(hashFunction)
 			hashFunction = mockCacheHasher
-			defer func() {
-				hashFunction = originalHasher
-			}()
 
 			for _, d := range test.dependencies {
 				builder := &mockBuilder{dependencies: d}

pkg/skaffold/build/cache/retrieve_test.go

@@ -124,11 +124,8 @@ func Test_RetrieveCachedArtifacts(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			originalHash := hashForArtifact
+			defer func(h func(context.Context, build.Builder, *latest.Artifact) (string, error)) { hashForArtifact = h }(hashForArtifact)
 			hashForArtifact = mockHashForArtifact(test.hashes)
-			defer func() {
-				hashForArtifact = originalHash
-			}()
 
 			test.cache.client = docker.NewLocalDaemon(&test.api, nil, false, map[string]bool{})
 
@@ -307,27 +304,18 @@ func TestRetrieveCachedArtifactDetails(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			originalHash := hashForArtifact
+			defer func(h func(context.Context, build.Builder, *latest.Artifact) (string, error)) { hashForArtifact = h }(hashForArtifact)
 			hashForArtifact = mockHashForArtifact(test.hashes)
-			defer func() {
-				hashForArtifact = originalHash
-			}()
 
-			originalRemoteDigest := remoteDigest
+			defer func(d func(string, map[string]bool) (string, error)) { remoteDigest = d }(remoteDigest)
 			remoteDigest = func(string, map[string]bool) (string, error) {
 				return test.digest, nil
 			}
-			defer func() {
-				remoteDigest = originalRemoteDigest
-			}()
 
-			originalImgExistsRemotely := imgExistsRemotely
-			imgExistsRemotely = func(_, _ string, _ map[string]bool) bool {
+			defer func(i func(string, string, map[string]bool) bool) { imgExistsRemotely = i }(imgExistsRemotely)
+			imgExistsRemotely = func(string, string, map[string]bool) bool {
 				return test.targetImageExistsRemotely
 			}
-			defer func() {
-				imgExistsRemotely = originalImgExistsRemotely
-			}()
 
 			if test.api != nil {
 				test.cache.client = docker.NewLocalDaemon(test.api, nil, false, map[string]bool{})

pkg/skaffold/build/cluster/logs_test.go

@@ -20,13 +20,10 @@ import (
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/testutil"
-
 	"github.com/sirupsen/logrus"
 )
 
 func TestLogLevel(t *testing.T) {
-	defer func(l logrus.Level) { logrus.SetLevel(l) }(logrus.GetLevel())
-
 	tests := []struct {
 		logrusLevel logrus.Level
 		expected    logrus.Level
@@ -40,6 +37,7 @@ func TestLogLevel(t *testing.T) {
 	}
 
 	for _, test := range tests {
+		defer func(l logrus.Level) { logrus.SetLevel(l) }(logrus.GetLevel())
 		logrus.SetLevel(test.logrusLevel)
 
 		kanikoLevel := logLevel()

pkg/skaffold/build/custom/custom_test.go

@@ -65,19 +65,13 @@ func TestRetrieveEnv(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			initialEnviron := environ
-			defer func() {
-				environ = initialEnviron
-			}()
+			defer func(e func() []string) { environ = e }(environ)
 			environ = func() []string {
 				return test.environ
 			}
 
-			initialBuildContext := buildContext
-			defer func() {
-				buildContext = initialBuildContext
-			}()
-			buildContext = func(_ string) (string, error) {
+			defer func(bc func(string) (string, error)) { buildContext = bc }(buildContext)
+			buildContext = func(string) (string, error) {
 				return test.buildContext, nil
 			}
 
@@ -125,11 +119,11 @@ func TestRetrieveCmd(t *testing.T) {
 		t.Run(test.description, func(t *testing.T) {
 			builder := NewArtifactBuilder(false, nil)
 
-			defer func(initialEnviron func() []string) { environ = initialEnviron }(environ)
+			defer func(e func() []string) { environ = e }(environ)
 			environ = func() []string { return nil }
 
-			defer func(initialBuildContext func(string) (string, error)) { buildContext = initialBuildContext }(buildContext)
-			buildContext = func(_ string) (string, error) {
+			defer func(bc func(string) (string, error)) { buildContext = bc }(buildContext)
+			buildContext = func(string) (string, error) {
 				return test.artifact.Workspace, nil
 			}
 

pkg/skaffold/docker/context_test.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
 func TestDockerContext(t *testing.T) {
@@ -32,9 +33,9 @@ func TestDockerContext(t *testing.T) {
 			tmpDir, cleanup := testutil.NewTempDir(t)
 			defer cleanup()
 
+			defer func(f func(string, map[string]bool) (*v1.ConfigFile, error)) { RetrieveImage = f }(RetrieveImage)
 			imageFetcher := fakeImageFetcher{}
 			RetrieveImage = imageFetcher.fetch
-			defer func() { RetrieveImage = retrieveImage }()
 
 			artifact := &latest.DockerArtifact{
 				DockerfilePath: "Dockerfile",

pkg/skaffold/docker/parse_test.go

@@ -506,9 +506,9 @@ func TestGetDependencies(t *testing.T) {
 			tmpDir, cleanup := testutil.NewTempDir(t)
 			defer cleanup()
 
+			defer func(f func(string, map[string]bool) (*v1.ConfigFile, error)) { RetrieveImage = f }(RetrieveImage)
 			imageFetcher := fakeImageFetcher{}
 			RetrieveImage = imageFetcher.fetch
-			defer func() { RetrieveImage = retrieveImage }()
 
 			for _, file := range []string{"docker/nginx.conf", "docker/bar", "server.go", "test.conf", "worker.go", "bar", "file", ".dot"} {
 				tmpDir.Write(file, "")

pkg/skaffold/kubernetes/port_forward_test.go

@@ -25,7 +25,6 @@ import (
 	"sync"
 	"testing"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -436,13 +435,10 @@ func TestPortForwardPod(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-
 			taken := map[int]struct{}{}
-			originalGetAvailablePort := util.GetAvailablePort
+
+			defer func(r func(int, *sync.Map) int) { retrieveAvailablePort = r }(retrieveAvailablePort)
 			retrieveAvailablePort = mockRetrieveAvailablePort(taken, test.availablePorts)
-			defer func() {
-				retrieveAvailablePort = originalGetAvailablePort
-			}()
 
 			p := NewPortForwarder(ioutil.Discard, NewImageList(), []string{""})
 			if test.forwarder == nil {

pkg/skaffold/runner/dev_test.go

@@ -330,13 +330,11 @@ func TestDevSync(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			originalWorkingDir := sync.WorkingDir
-			sync.WorkingDir = func(_ string, _ map[string]bool) (string, error) {
+			defer func(s func(string, map[string]bool) (string, error)) { sync.WorkingDir = s }(sync.WorkingDir)
+			sync.WorkingDir = func(string, map[string]bool) (string, error) {
 				return "/", nil
 			}
-			defer func() {
-				sync.WorkingDir = originalWorkingDir
-			}()
+
 			runner := createRunner(t, test.testBench)
 			runner.Watcher = &TestWatcher{
 				events:    test.watchEvents,

pkg/skaffold/schema/validation/validation_test.go

@@ -349,9 +349,8 @@ func TestValidateNetworkMode(t *testing.T) {
 	}
 
 	// disable yamltags validation
-	origValidateYamlTags := validateYamltags
-	validateYamltags = func(_ interface{}) error { return nil }
-	defer func() { validateYamltags = origValidateYamlTags }()
+	defer func(v func(interface{}) error) { validateYamltags = v }(validateYamltags)
+	validateYamltags = func(interface{}) error { return nil }
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
@@ -460,9 +459,8 @@ func TestValidateSyncRules(t *testing.T) {
 	}
 
 	// disable yamltags validation
-	origValidateYamlTags := validateYamltags
-	validateYamltags = func(_ interface{}) error { return nil }
-	defer func() { validateYamltags = origValidateYamlTags }()
+	defer func(v func(interface{}) error) { validateYamltags = v }(validateYamltags)
+	validateYamltags = func(interface{}) error { return nil }
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

pkg/skaffold/sync/sync_test.go

@@ -387,18 +387,15 @@ func TestNewSyncItem(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			originalWorkingDir := WorkingDir
-			WorkingDir = func(_ string, _ map[string]bool) (string, error) {
+			defer func(w func(string, map[string]bool) (string, error)) { WorkingDir = w }(WorkingDir)
+			WorkingDir = func(string, map[string]bool) (string, error) {
 				return test.workingDir, nil
 			}
-			defer func() {
-				WorkingDir = originalWorkingDir
-			}()
+
 			actual, err := NewItem(test.artifact, test.evt, test.builds, map[string]bool{})
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, actual)
 		})
 	}
-
 }
 
 func TestIntersect(t *testing.T) {