Faster tests

 + Optimize all tests that take over .6s
 + Use t.Override in placed where it should be used
 + Remove TestRemoteDigest that was not really testing anything
 + Run git tests in parallel

Signed-off-by: David Gageot <[email protected]>

cmd/skaffold/app/cmd/runner_test.go

@@ -22,7 +22,9 @@ import (
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/update"
 	"github.com/GoogleContainerTools/skaffold/testutil"
+	"github.com/blang/semver"
 )
 
 func TestCreateNewRunner(t *testing.T) {
@@ -81,6 +83,9 @@ func TestCreateNewRunner(t *testing.T) {
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.Override(&update.GetLatestAndCurrentVersion, func() (semver.Version, semver.Version, error) {
+				return semver.Version{}, semver.Version{}, nil
+			})
 			t.NewTempDir().
 				Write("skaffold.yaml", fmt.Sprintf("apiVersion: %s\nkind: Config\n%s", latest.Version, test.config)).
 				Chdir()

pkg/skaffold/build/cache/retrieve_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
+	"github.com/docker/docker/api/types"
 )
 
 type depLister struct {
@@ -83,6 +84,11 @@ func (b *mockBuilder) BuildAndTest(ctx context.Context, out io.Writer, tags tag.
 	return built, nil
 }
 
+type stubAuth struct{}
+
+func (t stubAuth) GetAuthConfig(string) (types.AuthConfig, error)          { return types.AuthConfig{}, nil }
+func (t stubAuth) GetAllAuthConfigs() (map[string]types.AuthConfig, error) { return nil, nil }
+
 func TestCacheBuildLocal(t *testing.T) {
 	testutil.Run(t, "", func(t *testutil.T) {
 		tmpDir := t.NewTempDir().
@@ -183,7 +189,7 @@ func TestCacheBuildRemote(t *testing.T) {
 		t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
 			return dockerDaemon, nil
 		})
-
+		t.Override(&docker.DefaultAuthHelper, stubAuth{})
 		t.Override(&docker.RemoteDigest, func(ref string, _ map[string]bool) (string, error) {
 			switch ref {
 			case "artifact1:tag1":

pkg/skaffold/build/tag/git_commit_test.go

@@ -331,6 +331,8 @@ func TestGitCommit_GenerateFullyQualifiedImageName(t *testing.T) {
 
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.Parallel()
+
 			tmpDir := t.NewTempDir()
 			test.createGitRepo(tmpDir.Root())
 			workspace := tmpDir.Path(test.subDir)

pkg/skaffold/deploy/status_check_test.go

@@ -210,7 +210,7 @@ func TestPollDeploymentRolloutStatus(t *testing.T) {
 				responses: []string{"dep successfully rolled out"},
 			},
 			exactCalls: 1,
-			duration:   500,
+			duration:   50,
 		},
 		{
 			description: "rollout returns error in the first attempt",
@@ -219,7 +219,7 @@ func TestPollDeploymentRolloutStatus(t *testing.T) {
 			},
 			shouldErr:  true,
 			exactCalls: 1,
-			duration:   500,
+			duration:   50,
 		},
 		{
 			description: "rollout returns success before time out",
@@ -229,7 +229,7 @@ func TestPollDeploymentRolloutStatus(t *testing.T) {
 					"Waiting for rollout to finish: 0 of 1 updated replicas are available...",
 					"deployment.apps/dep successfully rolled out"},
 			},
-			duration:   800,
+			duration:   80,
 			exactCalls: 3,
 		},
 		{
@@ -240,22 +240,15 @@ func TestPollDeploymentRolloutStatus(t *testing.T) {
 					"Waiting for rollout to finish: 1 of 3 updated replicas are available...",
 					"Waiting for rollout to finish: 2 of 3 updated replicas are available..."},
 			},
-			duration:  1000,
+			duration:  100,
 			shouldErr: true,
 			timedOut:  true,
 		},
 	}
-	originalPollingPeriod := defaultPollPeriodInMilliseconds
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			mock := test.mock
-			// Figure out why i can't use t.Override.
-			// Using t.Override throws an error "reflect: call of reflect.Value.Elem on func Value"
-			executeRolloutStatus = mock.Executefunc
-			defer func() { executeRolloutStatus = getRollOutStatus }()
-
-			defaultPollPeriodInMilliseconds = 100
-			defer func() { defaultPollPeriodInMilliseconds = originalPollingPeriod }()
+			t.Override(&executeRolloutStatus, test.mock.Executefunc)
+			t.Override(&defaultPollPeriodInMilliseconds, 10)
 
 			actual := &sync.Map{}
 			pollDeploymentRolloutStatus(context.Background(), &kubectl.CLI{}, "dep", time.Duration(test.duration)*time.Millisecond, actual)
@@ -267,7 +260,7 @@ func TestPollDeploymentRolloutStatus(t *testing.T) {
 			t.CheckError(test.shouldErr, err)
 			// Check number of calls only if command did not timeout since there could be n-1 or n or n+1 calls when command timed out
 			if !test.timedOut {
-				t.CheckDeepEqual(test.exactCalls, mock.called)
+				t.CheckDeepEqual(test.exactCalls, test.mock.called)
 			}
 		})
 	}

pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go

@@ -437,7 +437,7 @@ func TestAutomaticPortForwardPod(t *testing.T) {
 			event.InitializeState(latest.BuildConfig{})
 			taken := map[int]struct{}{}
 
-			forwardingTimeoutTime = time.Second
+			t.Override(&forwardingTimeoutTime, 500*time.Millisecond)
 			t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort(taken, test.availablePorts))
 
 			entryManager := EntryManager{
@@ -481,14 +481,17 @@ func TestStartPodForwarder(t *testing.T) {
 			description:   "pod modified event",
 			entryExpected: true,
 			event:         watch.Modified,
-		}, {
+		},
+		{
 			description: "pod error event",
 			event:       watch.Error,
-		}, {
+		},
+		{
 			description: "event isn't for a pod",
 			obj:         &v1.Service{},
 			event:       watch.Modified,
-		}, {
+		},
+		{
 			description: "event is deleted",
 			event:       watch.Deleted,
 		},
@@ -551,8 +554,8 @@ func TestStartPodForwarder(t *testing.T) {
 
 			fakeWatcher.Action(test.event, obj)
 
-			// poll for 2 seconds for the pod resource to be forwarded
-			err := wait.PollImmediate(time.Second, 2*time.Second, func() (bool, error) {
+			// wait for the pod resource to be forwarded
+			err := wait.PollImmediate(10*time.Millisecond, 100*time.Millisecond, func() (bool, error) {
 				_, ok := fakeForwarder.forwardedResources.Load("mycontainer-default-myport-8080")
 				return ok, nil
 			})

pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go

@@ -131,7 +131,7 @@ func TestStart(t *testing.T) {
 				t.Fatalf("error starting resource forwarder: %v", err)
 			}
 			// poll up to 10 seconds for the resources to be forwarded
-			err := wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) {
+			err := wait.PollImmediate(100*time.Millisecond, 10*time.Second, func() (bool, error) {
 				return len(test.expected) == fakeForwarder.forwardedResources.Length(), nil
 			})
 			if err != nil {
@@ -245,7 +245,7 @@ func TestUserDefinedResources(t *testing.T) {
 			t.Fatalf("error starting resource forwarder: %v", err)
 		}
 		// poll up to 10 seconds for the resources to be forwarded
-		err := wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) {
+		err := wait.PollImmediate(100*time.Millisecond, 10*time.Second, func() (bool, error) {
 			return len(expected) == fakeForwarder.forwardedResources.Length(), nil
 		})
 		if err != nil {

pkg/skaffold/kubernetes/wait_test.go

@@ -22,10 +22,8 @@ import (
 	"time"
 
 	"github.com/GoogleContainerTools/skaffold/testutil"
-
-	"k8s.io/apimachinery/pkg/watch"
-
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/watch"
 	fakekubeclientset "k8s.io/client-go/kubernetes/fake"
 )
 
@@ -60,7 +58,7 @@ func TestWaitForPodSucceeded(t *testing.T) {
 
 			errChan := make(chan error)
 			go func() {
-				errChan <- WaitForPodSucceeded(context.TODO(), fakePods, "", 5*time.Second)
+				errChan <- WaitForPodSucceeded(context.TODO(), fakePods, "", 50*time.Millisecond)
 			}()
 
 			for _, phase := range test.phases {
@@ -72,7 +70,7 @@ func TestWaitForPodSucceeded(t *testing.T) {
 						Phase: phase,
 					},
 				})
-				time.Sleep(time.Second)
+				time.Sleep(10 * time.Millisecond)
 			}
 			err := <-errChan
 

pkg/skaffold/runner/deploy_test.go

@@ -60,14 +60,10 @@ func TestDeploy(t *testing.T) {
 	dummyStatusCheck := func(ctx context.Context, l *deploy.DefaultLabeller, runCtx *runcontext.RunContext) error {
 		return nil
 	}
-	originalStatusCheck := deploy.StatusCheck
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"})
-			// Figure out why i can't use t.Override.
-			// Using t.Override throws an error "reflect: call of reflect.Value.Elem on func Value"
-			statusCheck = dummyStatusCheck
-			defer func() { statusCheck = originalStatusCheck }()
+			t.Override(&statusCheck, dummyStatusCheck)
 
 			runner := createRunner(t, test.testBench, nil)
 			runner.runCtx.Opts.StatusCheck = test.statusCheck

pkg/skaffold/runner/runner_test.go

@@ -199,7 +199,16 @@ func (r *SkaffoldRunner) WithMonitor(m filemon.Monitor) *SkaffoldRunner {
 }
 
 func createRunner(t *testutil.T, testBench *TestBench, monitor filemon.Monitor) *SkaffoldRunner {
-	cfg := &latest.SkaffoldConfig{}
+	cfg := &latest.SkaffoldConfig{
+		Pipeline: latest.Pipeline{
+			Build: latest.BuildConfig{
+				TagPolicy: latest.TagPolicy{
+					// Use the fastest tagger
+					ShaTagger: &latest.ShaTagger{},
+				},
+			},
+		},
+	}
 	defaults.Set(cfg)
 
 	runCtx := &runcontext.RunContext{

pkg/skaffold/update/update.go

@@ -28,6 +28,11 @@ import (
 	"github.com/pkg/errors"
 )
 
+// Fot testing
+var (
+	GetLatestAndCurrentVersion = getLatestAndCurrentVersion
+)
+
 const latestVersionURL = "https://storage.googleapis.com/skaffold/releases/latest/VERSION"
 
 // IsUpdateCheckEnabled returns whether or not the update check is enabled
@@ -42,9 +47,9 @@ func IsUpdateCheckEnabled() bool {
 	return v == "" || strings.ToLower(v) == "true"
 }
 
-// GetLatestAndCurrentVersion uses a VERSION file stored on GCS to determine the latest released version
+// getLatestAndCurrentVersion uses a VERSION file stored on GCS to determine the latest released version
 // and returns it with the current version of Skaffold
-func GetLatestAndCurrentVersion() (semver.Version, semver.Version, error) {
+func getLatestAndCurrentVersion() (semver.Version, semver.Version, error) {
 	none := semver.Version{}
 	resp, err := http.Get(latestVersionURL)
 	if err != nil {