Add boolean to disable opening browser during test

The WaitAndMaybeOpenService(..) method allow user to open the service
url in a browser when found, this create issue during testing as it's
opening browser and consuming resources.

The fix is to introduce a boolean flag allowing the caller to specify
whether to just print out the url or open a browser window

cmd/minikube/cmd/config/open.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/machine"
 	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/service"
+	"k8s.io/minikube/pkg/util"
 )
 
 var (
@@ -95,9 +96,16 @@ You can add one by annotating a service with the label {{.labelName}}:{{.addonNa
 		}
 		for i := range serviceList.Items {
 			svc := serviceList.Items[i].ObjectMeta.Name
-			if err := service.WaitAndMaybeOpenService(api, namespace, svc, addonsURLTemplate, addonsURLMode, https, wait, interval); err != nil {
+			var urlString string
+
+			if urlString, err = service.WaitForService(api, namespace, svc, addonsURLTemplate, addonsURLMode, https, wait, interval); err != nil {
 				exit.WithCodeT(exit.Unavailable, "Wait failed: {{.error}}", out.V{"error": err})
 			}
+
+			if len(urlString) != 0 {
+				out.T(out.Celebrate, "Opening kubernetes service  {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": svc})
+				util.OpenBrowser(urlString)
+			}
 		}
 	},
 }

cmd/minikube/cmd/service.go

@@ -20,11 +20,14 @@ import (
 	"os"
 	"text/template"
 
+	"k8s.io/minikube/pkg/minikube/out"
+
 	"github.com/spf13/cobra"
 	"k8s.io/minikube/pkg/minikube/cluster"
 	"k8s.io/minikube/pkg/minikube/exit"
 	"k8s.io/minikube/pkg/minikube/machine"
 	"k8s.io/minikube/pkg/minikube/service"
+	"k8s.io/minikube/pkg/util"
 )
 
 const defaultServiceFormatTemplate = "http://{{.IP}}:{{.Port}}"
@@ -68,11 +71,16 @@ var serviceCmd = &cobra.Command{
 		if !cluster.IsMinikubeRunning(api) {
 			os.Exit(1)
 		}
-		err = service.WaitAndMaybeOpenService(api, namespace, svc,
+		urlString, err := service.WaitForService(api, namespace, svc,
 			serviceURLTemplate, serviceURLMode, https, wait, interval)
 		if err != nil {
 			exit.WithError("Error opening service", err)
 		}
+
+		if len(urlString) != 0 {
+			out.T(out.Celebrate, "Opening kubernetes service  {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": svc})
+			util.OpenBrowser(urlString)
+		}
 	},
 }
 

pkg/minikube/registry/drvs/hyperkit/driver.go

@@ -25,9 +25,9 @@ import (
 	"github.com/pborman/uuid"
 	"k8s.io/minikube/pkg/drivers/hyperkit"
 	cfg "k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/localpath"
 	"k8s.io/minikube/pkg/minikube/registry"
-	"k8s.io/minikube/pkg/minikube/driver"
 )
 
 func init() {

pkg/minikube/registry/drvs/vmwarefusion/driver.go

@@ -24,10 +24,9 @@ import (
 	"github.com/docker/machine/drivers/vmwarefusion"
 	"github.com/docker/machine/libmachine/drivers"
 	cfg "k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/localpath"
 	"k8s.io/minikube/pkg/minikube/registry"
-	"k8s.io/minikube/pkg/minikube/driver"
-
 )
 
 func init() {

pkg/minikube/service/service.go

@@ -29,7 +29,6 @@ import (
 	"github.com/docker/machine/libmachine"
 	"github.com/golang/glog"
 	"github.com/olekukonko/tablewriter"
-	"github.com/pkg/browser"
 	"github.com/pkg/errors"
 	"github.com/spf13/viper"
 	core "k8s.io/api/core/v1"
@@ -265,22 +264,24 @@ func PrintServiceList(writer io.Writer, data [][]string) {
 	table.Render()
 }
 
-// WaitAndMaybeOpenService waits for a service, and opens it when running
-func WaitAndMaybeOpenService(api libmachine.API, namespace string, service string, urlTemplate *template.Template, urlMode bool, https bool,
-	wait int, interval int) error {
+// WaitForService waits for a service, and opens it when running
+func WaitForService(api libmachine.API, namespace string, service string, urlTemplate *template.Template, urlMode bool, https bool,
+	wait int, interval int) (string, error) {
+
+	var urlString string
 	// Convert "Amount of time to wait" and "interval of each check" to attempts
 	if interval == 0 {
 		interval = 1
 	}
 	chkSVC := func() error { return CheckService(namespace, service) }
 
 	if err := retry.Expo(chkSVC, time.Duration(interval)*time.Second, time.Duration(wait)*time.Second); err != nil {
-		return errors.Wrapf(err, "Service %s was not found in %q namespace. You may select another namespace by using 'minikube service %s -n <namespace>", service, namespace, service)
+		return urlString, errors.Wrapf(err, "Service %s was not found in %q namespace. You may select another namespace by using 'minikube service %s -n <namespace>", service, namespace, service)
 	}
 
 	serviceURL, err := GetServiceURLsForService(api, namespace, service, urlTemplate)
 	if err != nil {
-		return errors.Wrap(err, "Check that minikube is running and that you have specified the correct namespace")
+		return urlString, errors.Wrap(err, "Check that minikube is running and that you have specified the correct namespace")
 	}
 
 	if !urlMode {
@@ -295,22 +296,17 @@ func WaitAndMaybeOpenService(api libmachine.API, namespace string, service strin
 
 	if len(serviceURL.URLs) == 0 {
 		out.T(out.Sad, "service {{.namespace_name}}/{{.service_name}} has no node port", out.V{"namespace_name": namespace, "service_name": service})
-		return nil
+		return urlString, nil
 	}
 
 	for _, bareURLString := range serviceURL.URLs {
 		urlString, isHTTPSchemedURL := OptionallyHTTPSFormattedURLString(bareURLString, https)
 
 		if urlMode || !isHTTPSchemedURL {
 			out.T(out.Empty, urlString)
-		} else {
-			out.T(out.Celebrate, "Opening kubernetes service  {{.namespace_name}}/{{.service_name}} in default browser...", out.V{"namespace_name": namespace, "service_name": service})
-			if err := browser.OpenURL(urlString); err != nil {
-				out.ErrT(out.Empty, "browser failed to open url: {{.error}}", out.V{"error": err})
-			}
 		}
 	}
-	return nil
+	return urlString, nil
 }
 
 // GetServiceListByLabel returns a ServiceList by label

pkg/minikube/service/service_test.go

@@ -903,12 +903,12 @@ func TestWaitAndMaybeOpenService(t *testing.T) {
 				servicesMap:  serviceNamespaces,
 				endpointsMap: endpointNamespaces,
 			}
-			err := WaitAndMaybeOpenService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
+			_, err := WaitForService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
 			if test.err && err == nil {
-				t.Fatalf("WaitAndMaybeOpenService expected to fail for test: %v", test)
+				t.Fatalf("WaitForService expected to fail for test: %v", test)
 			}
 			if !test.err && err != nil {
-				t.Fatalf("WaitAndMaybeOpenService not expected to fail but got err: %v", err)
+				t.Fatalf("WaitForService not expected to fail but got err: %v", err)
 			}
 
 		})
@@ -954,12 +954,12 @@ func TestWaitAndMaybeOpenServiceForNotDefaultNamspace(t *testing.T) {
 				servicesMap:  serviceNamespaceOther,
 				endpointsMap: endpointNamespaces,
 			}
-			err := WaitAndMaybeOpenService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
+			_, err := WaitForService(test.api, test.namespace, test.service, defaultTemplate, test.urlMode, test.https, 1, 0)
 			if test.err && err == nil {
-				t.Fatalf("WaitAndMaybeOpenService expected to fail for test: %v", test)
+				t.Fatalf("WaitForService expected to fail for test: %v", test)
 			}
 			if !test.err && err != nil {
-				t.Fatalf("WaitAndMaybeOpenService not expected to fail but got err: %v", err)
+				t.Fatalf("WaitForService not expected to fail but got err: %v", err)
 			}
 
 		})

pkg/util/utils.go

@@ -28,6 +28,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/pkg/browser"
+
 	units "github.com/docker/go-units"
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/exit"
@@ -200,3 +202,9 @@ func ConcatStrings(src []string, prefix string, postfix string) []string {
 	}
 	return ret
 }
+
+func OpenBrowser(urlString string) {
+	if err := browser.OpenURL(urlString); err != nil {
+		out.ErrT(out.Empty, "browser failed to open url: {{.error}}", out.V{"error": err})
+	}
+}