Decrease cyclomatic complexity for test

Mostly by using the "extract method" pattern.

Here was the report (from gocyclo), before:

17 tunnel TestTunnelManagerCleanup pkg/minikube/tunnel/tunnel_manager_test.go:174:1
44 tunnel TestTunnel pkg/minikube/tunnel/tunnel_test.go:35:1
34 kubeconfig configEquals pkg/util/kubeconfig/config_test.go:420:1
25 integration testMounting test/integration/mount_test.go:36:1
18 integration testTunnel test/integration/tunnel_test.go:37:1

pkg/minikube/tunnel/tunnel_manager_test.go

@@ -171,10 +171,7 @@ func TestTunnelManagerDelayAndContext(t *testing.T) {
 	}
 }
 
-func TestTunnelManagerCleanup(t *testing.T) {
-	reg, cleanup := createTestRegistry(t)
-	defer cleanup()
-
+func registerRunningTunnels(reg *persistentRegistry) (*ID, *ID, error) {
 	runningTunnel1 := &ID{
 		Route:       unsafeParseRoute("1.2.3.4", "5.6.7.8/9"),
 		Pid:         os.Getpid(),
@@ -187,6 +184,19 @@ func TestTunnelManagerCleanup(t *testing.T) {
 		MachineName: "minikube",
 	}
 
+	err := reg.Register(runningTunnel1)
+	if err != nil {
+		return runningTunnel1, runningTunnel2, err
+	}
+	err = reg.Register(runningTunnel2)
+	if err != nil {
+		return runningTunnel1, runningTunnel2, err
+	}
+
+	return runningTunnel1, runningTunnel2, nil
+}
+
+func registerNotRunningTunnels(reg *persistentRegistry) (*ID, *ID, error) {
 	notRunningTunnel1 := &ID{
 		Route:       unsafeParseRoute("200.2.3.4", "10.6.7.8/9"),
 		Pid:         12341234,
@@ -198,19 +208,28 @@ func TestTunnelManagerCleanup(t *testing.T) {
 		Pid:         12341234,
 		MachineName: "minikube",
 	}
-	err := reg.Register(runningTunnel1)
+
+	err := reg.Register(notRunningTunnel1)
 	if err != nil {
-		t.Errorf("expected no error got: %v", err)
+		return notRunningTunnel1, notRunningTunnel2, err
 	}
-	err = reg.Register(runningTunnel2)
+	err = reg.Register(notRunningTunnel2)
 	if err != nil {
-		t.Errorf("expected no error got: %v", err)
+		return notRunningTunnel1, notRunningTunnel2, err
 	}
-	err = reg.Register(notRunningTunnel1)
+
+	return notRunningTunnel1, notRunningTunnel2, nil
+}
+func TestTunnelManagerCleanup(t *testing.T) {
+	reg, cleanup := createTestRegistry(t)
+	defer cleanup()
+
+	runningTunnel1, runningTunnel2, err := registerRunningTunnels(reg)
 	if err != nil {
 		t.Errorf("expected no error got: %v", err)
 	}
-	err = reg.Register(notRunningTunnel2)
+
+	notRunningTunnel1, notRunningTunnel2, err := registerNotRunningTunnels(reg)
 	if err != nil {
 		t.Errorf("expected no error got: %v", err)
 	}

pkg/util/kubeconfig/config_test.go

@@ -434,6 +434,24 @@ func configEquals(a, b *api.Config) bool {
 	}
 
 	// clusters
+	if !clustersEquals(a, b) {
+		return false
+	}
+
+	// users
+	if !authInfosEquals(a, b) {
+		return false
+	}
+
+	// contexts
+	if !contextsEquals(a, b) {
+		return false
+	}
+
+	return true
+}
+
+func clustersEquals(a, b *api.Config) bool {
 	if len(a.Clusters) != len(b.Clusters) {
 		return false
 	}
@@ -443,17 +461,26 @@ func configEquals(a, b *api.Config) bool {
 			return false
 		}
 
-		if aCluster.LocationOfOrigin != bCluster.LocationOfOrigin ||
-			aCluster.Server != bCluster.Server ||
-			aCluster.InsecureSkipTLSVerify != bCluster.InsecureSkipTLSVerify ||
-			aCluster.CertificateAuthority != bCluster.CertificateAuthority ||
-			len(aCluster.CertificateAuthorityData) != len(bCluster.CertificateAuthorityData) ||
-			len(aCluster.Extensions) != len(bCluster.Extensions) {
+		if !clusterEquals(aCluster, bCluster) {
 			return false
 		}
 	}
+	return true
+}
 
-	// users
+func clusterEquals(aCluster, bCluster *api.Cluster) bool {
+	if aCluster.LocationOfOrigin != bCluster.LocationOfOrigin ||
+		aCluster.Server != bCluster.Server ||
+		aCluster.InsecureSkipTLSVerify != bCluster.InsecureSkipTLSVerify ||
+		aCluster.CertificateAuthority != bCluster.CertificateAuthority ||
+		len(aCluster.CertificateAuthorityData) != len(bCluster.CertificateAuthorityData) ||
+		len(aCluster.Extensions) != len(bCluster.Extensions) {
+		return false
+	}
+	return true
+}
+
+func authInfosEquals(a, b *api.Config) bool {
 	if len(a.AuthInfos) != len(b.AuthInfos) {
 		return false
 	}
@@ -462,21 +489,29 @@ func configEquals(a, b *api.Config) bool {
 		if !exists {
 			return false
 		}
-		if aAuth.LocationOfOrigin != bAuth.LocationOfOrigin ||
-			aAuth.ClientCertificate != bAuth.ClientCertificate ||
-			len(aAuth.ClientCertificateData) != len(bAuth.ClientCertificateData) ||
-			aAuth.ClientKey != bAuth.ClientKey ||
-			len(aAuth.ClientKeyData) != len(bAuth.ClientKeyData) ||
-			aAuth.Token != bAuth.Token ||
-			aAuth.Username != bAuth.Username ||
-			aAuth.Password != bAuth.Password ||
-			len(aAuth.Extensions) != len(bAuth.Extensions) {
+		if !authInfoEquals(aAuth, bAuth) {
 			return false
 		}
+	}
+	return true
+}
 
+func authInfoEquals(aAuth, bAuth *api.AuthInfo) bool {
+	if aAuth.LocationOfOrigin != bAuth.LocationOfOrigin ||
+		aAuth.ClientCertificate != bAuth.ClientCertificate ||
+		len(aAuth.ClientCertificateData) != len(bAuth.ClientCertificateData) ||
+		aAuth.ClientKey != bAuth.ClientKey ||
+		len(aAuth.ClientKeyData) != len(bAuth.ClientKeyData) ||
+		aAuth.Token != bAuth.Token ||
+		aAuth.Username != bAuth.Username ||
+		aAuth.Password != bAuth.Password ||
+		len(aAuth.Extensions) != len(bAuth.Extensions) {
+		return false
 	}
+	return true
+}
 
-	// contexts
+func contextsEquals(a, b *api.Config) bool {
 	if len(a.Contexts) != len(b.Contexts) {
 		return false
 	}
@@ -485,14 +520,20 @@ func configEquals(a, b *api.Config) bool {
 		if !exists {
 			return false
 		}
-		if aContext.LocationOfOrigin != bContext.LocationOfOrigin ||
-			aContext.Cluster != bContext.Cluster ||
-			aContext.AuthInfo != bContext.AuthInfo ||
-			aContext.Namespace != bContext.Namespace ||
-			len(aContext.Extensions) != len(bContext.Extensions) {
+		if !contextEquals(aContext, bContext) {
 			return false
 		}
+	}
+	return true
+}
 
+func contextEquals(aContext, bContext *api.Context) bool {
+	if aContext.LocationOfOrigin != bContext.LocationOfOrigin ||
+		aContext.Cluster != bContext.Cluster ||
+		aContext.AuthInfo != bContext.AuthInfo ||
+		aContext.Namespace != bContext.Namespace ||
+		len(aContext.Extensions) != len(bContext.Extensions) {
+		return false
 	}
 	return true
 }

test/integration/mount_test.go

@@ -50,12 +50,7 @@ func testMounting(t *testing.T) {
 	}
 	defer os.RemoveAll(tempDir)
 
-	var mountCmd string
-	if len(minikubeRunner.MountArgs) > 0 {
-		mountCmd = fmt.Sprintf("mount %s %s:/mount-9p", minikubeRunner.MountArgs, tempDir)
-	} else {
-		mountCmd = fmt.Sprintf("mount %s:/mount-9p", tempDir)
-	}
+	mountCmd := getMountCmd(minikubeRunner, tempDir)
 	cmd, _, _ := minikubeRunner.RunDaemon2(mountCmd)
 	defer func() {
 		err := cmd.Process.Kill()
@@ -70,13 +65,8 @@ func testMounting(t *testing.T) {
 
 	// Write file in mounted dir from host
 	expected := "test\n"
-	files := []string{"fromhost", "fromhostremove"}
-	for _, file := range files {
-		path := filepath.Join(tempDir, file)
-		err = ioutil.WriteFile(path, []byte(expected), 0644)
-		if err != nil {
-			t.Fatalf("Unexpected error while writing file %s: %v", path, err)
-		}
+	if err := writeFilesFromHost(tempDir, []string{"fromhost", "fromhostremove"}, expected); err != nil {
+		t.Fatalf(err.Error())
 	}
 
 	// Create the pods we need outside the main test loop.
@@ -96,70 +86,106 @@ func testMounting(t *testing.T) {
 		t.Fatal("mountTest failed with error:", err)
 	}
 
-	client, err := pkgutil.GetClient()
-	if err != nil {
-		t.Fatalf("getting kubernetes client: %v", err)
-	}
-	selector := labels.SelectorFromSet(labels.Set(map[string]string{"integration-test": "busybox-mount"}))
-	if err := pkgutil.WaitForPodsWithLabelRunning(client, "default", selector); err != nil {
+	if err := waitForPods(map[string]string{"integration-test": "busybox-mount"}); err != nil {
 		t.Fatalf("Error waiting for busybox mount pod to be up: %v", err)
 	}
 
 	mountTest := func() error {
-		path := filepath.Join(tempDir, "frompod")
-		out, err := ioutil.ReadFile(path)
-		if err != nil {
-			return err
-		}
-		// test that file written from pod can be read from host echo test > /mount-9p/frompod; in pod
-		if string(out) != expected {
-			t.Fatalf("Expected file %s to contain text %s, was %s.", path, expected, out)
+		if err := verifyFiles(minikubeRunner, kubectlRunner, tempDir, podName, expected); err != nil {
+			t.Fatalf(err.Error())
 		}
 
-		// test that file written from host was read in by the pod via cat /mount-9p/fromhost;
-		if out, err = kubectlRunner.RunCommand([]string{"logs", podName}); err != nil {
-			return err
-		}
-		if string(out) != expected {
-			t.Fatalf("Expected file %s to contain text %s, was %s.", path, expected, out)
+		return nil
+	}
+	if err := util.Retry(t, mountTest, 5*time.Second, 40); err != nil {
+		t.Fatalf("mountTest failed with error: %v", err)
+	}
+
+}
+
+func getMountCmd(minikubeRunner util.MinikubeRunner, mountDir string) string {
+	var mountCmd string
+	if len(minikubeRunner.MountArgs) > 0 {
+		mountCmd = fmt.Sprintf("mount %s %s:/mount-9p", minikubeRunner.MountArgs, mountDir)
+	} else {
+		mountCmd = fmt.Sprintf("mount %s:/mount-9p", mountDir)
+	}
+	return mountCmd
+}
+
+func writeFilesFromHost(mountedDir string, files []string, content string) error {
+	for _, file := range files {
+		path := filepath.Join(mountedDir, file)
+		err := ioutil.WriteFile(path, []byte(content), 0644)
+		if err != nil {
+			return fmt.Errorf("Unexpected error while writing file %s: %v", path, err)
 		}
+	}
+	return nil
+}
 
-		// test file timestamps are correct
-		files := []string{"fromhost", "frompod"}
-		for _, file := range files {
-			statCmd := fmt.Sprintf("stat /mount-9p/%s", file)
-			statOutput, err := minikubeRunner.SSH(statCmd)
-			if err != nil {
-				t.Fatalf("Unable to stat %s via SSH. error %v, %s", file, err, statOutput)
-			}
+func waitForPods(s map[string]string) error {
+	client, err := pkgutil.GetClient()
+	if err != nil {
+		return fmt.Errorf("getting kubernetes client: %v", err)
+	}
+	selector := labels.SelectorFromSet(labels.Set(s))
+	if err := pkgutil.WaitForPodsWithLabelRunning(client, "default", selector); err != nil {
+		return err
+	}
+	return nil
+}
 
-			if runtime.GOOS == "windows" {
-				if strings.Contains(statOutput, "Access: 1970-01-01") {
-					t.Fatalf("Invalid access time\n%s", statOutput)
-				}
-			}
+func verifyFiles(minikubeRunner util.MinikubeRunner, kubectlRunner *util.KubectlRunner, tempDir string, podName string, expected string) error {
+	path := filepath.Join(tempDir, "frompod")
+	out, err := ioutil.ReadFile(path)
+	if err != nil {
+		return err
+	}
+	// test that file written from pod can be read from host echo test > /mount-9p/frompod; in pod
+	if string(out) != expected {
+		return fmt.Errorf("Expected file %s to contain text %s, was %s.", path, expected, out)
+	}
 
-			if strings.Contains(statOutput, "Modify: 1970-01-01") {
-				t.Fatalf("Invalid modify time\n%s", statOutput)
-			}
+	// test that file written from host was read in by the pod via cat /mount-9p/fromhost;
+	if out, err = kubectlRunner.RunCommand([]string{"logs", podName}); err != nil {
+		return err
+	}
+	if string(out) != expected {
+		return fmt.Errorf("Expected file %s to contain text %s, was %s.", path, expected, out)
+	}
+
+	// test file timestamps are correct
+	files := []string{"fromhost", "frompod"}
+	for _, file := range files {
+		statCmd := fmt.Sprintf("stat /mount-9p/%s", file)
+		statOutput, err := minikubeRunner.SSH(statCmd)
+		if err != nil {
+			return fmt.Errorf("Unable to stat %s via SSH. error %v, %s", file, err, statOutput)
 		}
 
-		// test that fromhostremove was deleted by the pod from the mount via rm /mount-9p/fromhostremove
-		path = filepath.Join(tempDir, "fromhostremove")
-		if _, err := os.Stat(path); err == nil {
-			t.Fatalf("Expected file %s to be removed", path)
+		if runtime.GOOS == "windows" {
+			if strings.Contains(statOutput, "Access: 1970-01-01") {
+				return fmt.Errorf("Invalid access time\n%s", statOutput)
+			}
 		}
 
-		// test that frompodremove can be deleted on the host
-		path = filepath.Join(tempDir, "frompodremove")
-		if err := os.Remove(path); err != nil {
-			t.Fatalf("Unexpected error removing file %s: %v", path, err)
+		if strings.Contains(statOutput, "Modify: 1970-01-01") {
+			return fmt.Errorf("Invalid modify time\n%s", statOutput)
 		}
+	}
 
-		return nil
+	// test that fromhostremove was deleted by the pod from the mount via rm /mount-9p/fromhostremove
+	path = filepath.Join(tempDir, "fromhostremove")
+	if _, err := os.Stat(path); err == nil {
+		return fmt.Errorf("Expected file %s to be removed", path)
 	}
-	if err := util.Retry(t, mountTest, 5*time.Second, 40); err != nil {
-		t.Fatalf("mountTest failed with error: %v", err)
+
+	// test that frompodremove can be deleted on the host
+	path = filepath.Join(tempDir, "frompodremove")
+	if err := os.Remove(path); err != nil {
+		return fmt.Errorf("Unexpected error removing file %s: %v", path, err)
 	}
 
+	return nil
 }

test/integration/tunnel_test.go

@@ -98,34 +98,41 @@ func testTunnel(t *testing.T) {
 		t.Fatal("svc should have ingress after tunnel is created, but it was empty!")
 	}
 
+	responseBody, err := getResponseBody(nginxIP)
+	if err != nil {
+		t.Fatalf("error reading from nginx at address(%s): %s", nginxIP, err)
+	}
+	if !strings.Contains(responseBody, "Welcome to nginx!") {
+		t.Fatalf("response body doesn't seem like an nginx response:\n%s", responseBody)
+	}
+}
+
+func getResponseBody(address string) (string, error) {
 	httpClient := http.DefaultClient
 	httpClient.Timeout = 5 * time.Second
 
 	var resp *http.Response
+	var err error
 
 	request := func() error {
-		resp, err = httpClient.Get(fmt.Sprintf("http://%s", nginxIP))
+		resp, err = httpClient.Get(fmt.Sprintf("http://%s", address))
 		if err != nil {
 			retriable := &commonutil.RetriableError{Err: err}
-			t.Log(retriable)
 			return retriable
 		}
 		return nil
 	}
 
 	if err = commonutil.RetryAfter(5, request, 1*time.Second); err != nil {
-		t.Fatalf("error reading from nginx at address(%s): %s", nginxIP, err)
+		return "", err
 	}
 
 	defer resp.Body.Close()
 
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil || len(body) == 0 {
-		t.Fatalf("error reading body from nginx at address(%s): error: %s, len bytes read: %d", nginxIP, err, len(body))
+		return "", errors.Wrapf(err, "error reading body, len bytes read: %d", len(body))
 	}
 
-	responseBody := string(body)
-	if !strings.Contains(responseBody, "Welcome to nginx!") {
-		t.Fatalf("response body doesn't seem like an nginx response:\n%s", responseBody)
-	}
+	return string(body), nil
 }