Refactor spinner library & hide sub steps after spinning (#21215)

* remove omitnew line

* add new option to hide after spin

* refactor to use new return hideAfterSpin

* add new line by default only when not spinining and delegate spinner newline to spinner code

* add new func for ouptputing with spinner and pass fdwriter directly to the spininer func

* fix unit test

* fix lint for krunkit

* add comment and context

* use different spinning progress bar for sub steps

* make func private

* making more spinning icons

* integration test dont expect sub steps to be visible

* fix unit test and comment why

* change sub step spining icon not to be a progressbar

* pass the filewriter to the spinner library for the spinning steps

Author: medyagh

pkg/minikube/out/out.go

@@ -66,7 +66,7 @@ var (
 	// JSON is whether or not we should output stdout in JSON format. Set using SetJSON()
 	JSON = false
 	// spin is spinner showed at starting minikube
-	spin = spinner.New(spinner.CharSets[style.SpinnerCharacter], 100*time.Millisecond)
+	spin = spinner.New(spinner.CharSets[style.SpinnerCharacter], 100*time.Millisecond, spinner.WithWriter(outFile))
 	// defaultBoxCfg is the default style config for cli box output
 	defaultBoxCfg = box.Config{Py: 1, Px: 4, Type: "Round", Color: "Red"}
 )
@@ -91,7 +91,7 @@ func Step(st style.Enum, format string, a ...V) {
 		Infof(format, a...)
 		return
 	}
-	outStyled, _ := stylized(st, useColor, format, a...)
+	outStyled, _, _ := stylized(st, useColor, format, a...)
 	if JSON {
 		register.PrintStep(outStyled)
 		klog.Info(outStyled)
@@ -107,9 +107,9 @@ func Styled(st style.Enum, format string, a ...V) {
 		Infof(format, a...)
 		return
 	}
-	outStyled, useSpinner := stylized(st, useColor, format, a...)
+	outStyled, useSpinner, hideAfterSpin := stylized(st, useColor, format, a...)
 	if useSpinner {
-		spinnerString(outStyled)
+		spinnerString(outStyled, hideAfterSpin)
 	} else {
 		String(outStyled)
 	}
@@ -146,13 +146,13 @@ func BoxedWithConfig(cfg box.Config, st style.Enum, title string, text string, a
 
 // Sprintf is used for returning the string (doesn't write anything)
 func Sprintf(st style.Enum, format string, a ...V) string {
-	outStyled, _ := stylized(st, useColor, format, a...)
+	outStyled, _, _ := stylized(st, useColor, format, a...)
 	return outStyled
 }
 
 // Infof is used for informational logs (options, env variables, etc)
 func Infof(format string, a ...V) {
-	outStyled, _ := stylized(style.Option, useColor, format, a...)
+	outStyled, _, _ := stylized(style.Option, useColor, format, a...)
 	if JSON {
 		register.PrintInfo(outStyled)
 	}
@@ -214,6 +214,21 @@ func Output(file fdWriter, s string) {
 	}
 }
 
+// outputSpining writes a basic string with spinining
+func outputSpining(file fdWriter, s string, hideAfterSpin bool) {
+	spin.Writer = file
+	spin.Prefix = s
+	if hideAfterSpin {
+		spin.UpdateCharSet(spinner.CharSets[style.SpinnerSubStepCharacter])
+		spin.FinalMSG = ""
+	} else {
+		spin.UpdateCharSet(spinner.CharSets[style.SpinnerCharacter])
+		spin.FinalMSG = s + "\n"
+	}
+	spin.Start()
+
+}
+
 // Outputf writes a basic formatted string
 func Outputf(file fdWriter, format string, a ...interface{}) {
 	_, err := fmt.Fprintf(file, format, a...)
@@ -223,7 +238,7 @@ func Outputf(file fdWriter, format string, a ...interface{}) {
 }
 
 // spinnerString writes a basic formatted string to stdout with spinner character
-func spinnerString(s string) {
+func spinnerString(s string, hideAfterSpin bool) {
 	// Flush log buffer so that output order makes sense
 	klog.Flush()
 
@@ -237,9 +252,7 @@ func spinnerString(s string) {
 	if spin.Active() {
 		spin.Stop()
 	}
-	Output(outFile, s)
-	// Start spinning at the end of the printed line
-	spin.Start()
+	outputSpining(outFile, s, hideAfterSpin)
 }
 
 // Ln writes a basic formatted string with a newline to stdout
@@ -249,7 +262,7 @@ func Ln(format string, a ...interface{}) {
 
 // ErrT writes a stylized and templated error message to stderr
 func ErrT(st style.Enum, format string, a ...V) {
-	errStyled, _ := stylized(st, useColor, format, a...)
+	errStyled, _, _ := stylized(st, useColor, format, a...)
 	Err(errStyled)
 }
 
@@ -316,7 +329,7 @@ func WarningT(format string, a ...V) {
 		if spin.Active() {
 			spin.Stop()
 		}
-		st, _ := stylized(style.Warning, useColor, format, a...)
+		st, _, _ := stylized(style.Warning, useColor, format, a...)
 		register.PrintWarning(st)
 		klog.Warning(st)
 		return
@@ -343,6 +356,7 @@ func SetSilent(q bool) {
 // SetOutFile configures which writer standard output goes to.
 func SetOutFile(w fdWriter) {
 	klog.Infof("Setting OutFile to fd %d ...", w.Fd())
+	spin.Writer = w
 	outFile = w
 	useColor = wantsColor(w)
 }

pkg/minikube/out/out_reason.go

@@ -106,6 +106,6 @@ func determineOutput(st style.Enum, format string, a ...V) {
 		ErrT(st, format, a...)
 		return
 	}
-	errStyled, _ := stylized(st, useColor, format, a...)
+	errStyled, _, _ := stylized(st, useColor, format, a...)
 	klog.Warning(errStyled)
 }

pkg/minikube/out/out_style.go

@@ -31,31 +31,33 @@ func applyPrefix(prefix, format string) string {
 }
 
 // applyStyle translates the given string if necessary then adds any appropriate style prefix.
-func applyStyle(st style.Enum, useColor bool, format string) (string, bool) {
+func applyStyle(st style.Enum, useColor bool, format string) (string, bool, bool) {
 	format = translate.T(format)
 
 	s, ok := style.Config[st]
-	if !s.OmitNewline {
+	// becaue of https://github.com/kubernetes/minikube/issues/21148
+	// will handle making new lines with spinner library itself
+	if !s.ShouldSpin {
 		format += "\n"
 	}
 
 	// Similar to CSS styles, if no style matches, output an unformatted string.
 	if !ok || JSON {
-		return format, s.Spinner
+		return format, s.ShouldSpin, s.HideAfterSpin
 	}
 
 	if !useColor {
-		return applyPrefix(style.LowPrefix(s), format), s.Spinner
+		return applyPrefix(style.LowPrefix(s), format), s.ShouldSpin, s.HideAfterSpin
 	}
-	return applyPrefix(s.Prefix, format), s.Spinner
+	return applyPrefix(s.Prefix, format), s.ShouldSpin, s.HideAfterSpin
 }
 
 // stylized applies formatting to the provided template
-func stylized(st style.Enum, useColor bool, format string, a ...V) (string, bool) {
-	var spinner bool
+func stylized(st style.Enum, useColor bool, format string, a ...V) (string, bool, bool) {
+	var shouldSpin, hideAfterSpin bool
 	if a == nil {
 		a = []V{}
 	}
-	format, spinner = applyStyle(st, useColor, format)
-	return Fmt(format, a...), spinner
+	format, shouldSpin, hideAfterSpin = applyStyle(st, useColor, format)
+	return Fmt(format, a...), shouldSpin, hideAfterSpin
 }

pkg/minikube/out/out_style_test.go

@@ -83,7 +83,7 @@ func TestApplyStyle(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			rawGot, _ := applyStyle(test.styleEnum, test.useColor, test.format)
+			rawGot, _, _ := applyStyle(test.styleEnum, test.useColor, test.format)
 			got := strings.TrimSpace(rawGot)
 			if got != test.expected {
 				t.Errorf("Expected '%v' but got '%v'", test.expected, got)
@@ -139,7 +139,7 @@ func TestApplyTemplateFormating(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			rawGot, _ := stylized(test.styleEnum, test.useColor, test.format, test.a...)
+			rawGot, _, _ := stylized(test.styleEnum, test.useColor, test.format, test.a...)
 			got := strings.TrimSpace(rawGot)
 			if got != test.expected {
 				t.Errorf("Expected '%v' but got '%v'", test.expected, got)

pkg/minikube/out/out_test.go

@@ -54,7 +54,8 @@ func TestStep(t *testing.T) {
 		{style.Fatal, "Fatal: {{.error}}", V{"error": "ugh"}, "💣  Fatal: ugh\n", "X Fatal: ugh\n"},
 		{style.Issue, "http://i/{{.number}}", V{"number": 10000}, "    ▪ http://i/10000\n", "  - http://i/10000\n"},
 		{style.Usage, "raw: {{.one}} {{.two}}", V{"one": "'%'", "two": "%d"}, "💡  raw: '%' %d\n", "* raw: '%' %d\n"},
-		{style.Running, "Installing Kubernetes version {{.version}} ...", V{"version": "v1.13"}, "🏃  ... v1.13 تثبيت Kubernetes الإصدار\n", "* ... v1.13 تثبيت Kubernetes الإصدار\n"},
+		// spining steps do not support being unit tested with fake file writer, since passing the fake writer to the spininer library is not testable.
+		{style.Provisioning, "Installing Kubernetes version {{.version}} ...", V{"version": "v1.13"}, "🌱  ... v1.13 تثبيت Kubernetes الإصدار\n", "* ... v1.13 تثبيت Kubernetes الإصدار\n"},
 	}
 	for _, tc := range testCases {
 		for _, override := range []bool{true, false} {

pkg/minikube/registry/drvs/krunkit/krunkit.go

@@ -87,7 +87,7 @@ func configure(cfg config.ClusterConfig, n config.Node) (interface{}, error) {
 
 func status() registry.State {
 	if runtime.GOOS != "darwin" && runtime.GOARCH != "arm64" {
-		err := errors.New("The krunkit driver is only supported on macOS arm64 machines")
+		err := errors.New("the krunkit driver is only supported on macOS arm64 machines")
 		return registry.State{Error: err, Fix: "Use another driver", Doc: docURL}
 	}
 	if _, err := exec.LookPath("krunkit"); err != nil {

pkg/minikube/style/style.go

@@ -41,15 +41,17 @@ type Options struct {
 	Prefix string
 	// LowPrefix is the 7-bit compatible prefix we fallback to for less-awesome terminals
 	LowPrefix string
-	// OmitNewline omits a newline at the end of a message.
-	OmitNewline bool
-	// Spinner is a character to place at ending of message
-	Spinner bool
+	// ShouldSpin is a character to place at ending of message
+	ShouldSpin    bool
+	HideAfterSpin bool // Hide the prefix after spinning
 }
 
 // SpinnerCharacter is which of the spinner.CharSets to use
 const SpinnerCharacter = 9
 
+// SpinnerSubStepCharacter is Character to use for sub-steps in a spinner (it looks like a progress bar)
+const SpinnerSubStepCharacter = 40
+
 // Config is a map of style name to style struct
 // For consistency, ensure that emojis added render with the same width across platforms.
 var Config = map[Enum]Options{
@@ -72,8 +74,8 @@ var Config = map[Enum]Options{
 	Pause:              {Prefix: "⏸️  "},
 	Provisioning:       {Prefix: "🌱  "},
 	Ready:              {Prefix: "🏄  "},
-	Restarting:         {Prefix: "🔄  "},
-	Running:            {Prefix: "🏃  "},
+	Restarting:         {Prefix: "🔄  ", ShouldSpin: true},
+	Running:            {Prefix: "🏃  ", ShouldSpin: true}, // this is used when minikube start for a second time (already started)
 	Sparkle:            {Prefix: "✨  "},
 	Stopped:            {Prefix: "🛑  "},
 	Stopping:           {Prefix: "✋  "},
@@ -84,7 +86,7 @@ var Config = map[Enum]Options{
 	URL:                {Prefix: "👉  ", LowPrefix: LowIndent},
 	Usage:              {Prefix: "💡  "},
 	Waiting:            {Prefix: "⌛  "},
-	WaitingWithSpinner: {Prefix: "⌛  ", OmitNewline: true, Spinner: true},
+	WaitingWithSpinner: {Prefix: "⌛  ", ShouldSpin: true},
 	Unsupported:        {Prefix: "🚡  "},
 	Workaround:         {Prefix: "👉  ", LowPrefix: LowIndent},
 
@@ -113,11 +115,11 @@ var Config = map[Enum]Options{
 	Copying:          {Prefix: "✨  "},
 	CRIO:             {Prefix: "🎁  "}, // This should be a snow-flake, but the emoji has a strange width on macOS
 	DeletingHost:     {Prefix: "🔥  "},
-	Docker:           {Prefix: "🐳  ", OmitNewline: true, Spinner: true},
+	Docker:           {Prefix: "🐳  ", ShouldSpin: true},
 	DryRun:           {Prefix: "🌵  "},
 	Enabling:         {Prefix: "🔌  "},
 	FileDownload:     {Prefix: "💾  "},
-	Fileserver:       {Prefix: "🚀  ", OmitNewline: true},
+	Fileserver:       {Prefix: "🚀  "},
 	HealthCheck:      {Prefix: "🔎  "},
 	Internet:         {Prefix: "🌐  "},
 	ISODownload:      {Prefix: "💿  "},
@@ -132,11 +134,11 @@ var Config = map[Enum]Options{
 	Shutdown:         {Prefix: "🛑  "},
 	StartingNone:     {Prefix: "🤹  "},
 	StartingSSH:      {Prefix: "🔗  "},
-	StartingVM:       {Prefix: "🔥  ", OmitNewline: true, Spinner: true},
-	SubStep:          {Prefix: "    ▪ ", LowPrefix: LowIndentBullet, OmitNewline: true, Spinner: true},
+	StartingVM:       {Prefix: "🔥  ", ShouldSpin: true},
+	SubStep:          {Prefix: "    ▪ ", LowPrefix: LowIndentBullet, ShouldSpin: true, HideAfterSpin: true},
 	Tip:              {Prefix: "💡  "},
 	Unmount:          {Prefix: "🔥  "},
-	VerifyingNoLine:  {Prefix: "🤔  ", OmitNewline: true},
+	VerifyingNoLine:  {Prefix: "🤔  "},
 	Verifying:        {Prefix: "🤔  "},
 	CNI:              {Prefix: "🔗  "},
 	Toolkit:          {Prefix: "🛠️   "},

test/integration/error_spam_test.go

@@ -111,16 +111,6 @@ func TestErrorSpam(t *testing.T) {
 			t.Logf("minikube stderr:\n%s", stderr)
 		}
 
-		steps := []string{
-			"Generating certificates and keys ...",
-			"Booting up control plane ...",
-			"Configuring RBAC rules ...",
-		}
-		for _, step := range steps {
-			if !strings.Contains(stdout, step) {
-				t.Errorf("missing kubeadm init sub-step %q", step)
-			}
-		}
 	})
 
 	logTests := []struct {