From 510278b362a4ca3c5a502b0a34b2de1d06eeea8d Mon Sep 17 00:00:00 2001 From: Chandra P Date: Sun, 21 Jan 2024 10:50:40 -0600 Subject: [PATCH] Fix: Update spinner constructor and options api (#149) --- component/output_spinner.go | 67 ++++++++++++++++++- component/output_spinner_test.go | 107 +++++++++++++++++++++++++++++++ log/log.go | 20 +++--- log/logger.go | 35 ++++------ log/types.go | 27 ++++++-- log/writer.go | 4 +- 6 files changed, 217 insertions(+), 43 deletions(-) create mode 100644 component/output_spinner_test.go diff --git a/component/output_spinner.go b/component/output_spinner.go index 116306d34..17e90ee61 100644 --- a/component/output_spinner.go +++ b/component/output_spinner.go @@ -11,6 +11,8 @@ import ( "github.com/briandowns/spinner" "github.com/mattn/go-isatty" + + "github.com/vmware-tanzu/tanzu-plugin-runtime/log" ) // OutputWriterSpinner is OutputWriter augmented with a spinner. @@ -18,19 +20,40 @@ type OutputWriterSpinner interface { OutputWriter RenderWithSpinner() StopSpinner() + // SetFinalText sets the spinner final text and prefix + // log indicator (log.LogTypeOUTPUT can be used for no prefix) + SetFinalText(finalText string, prefix log.LogType) } // outputwriterspinner is our internal implementation. type outputwriterspinner struct { outputwriter - spinnerText string - spinner *spinner.Spinner + spinnerText string + spinnerFinalText string + spinner *spinner.Spinner +} + +type OutputWriterSpinnerOptions struct { + OutputWriterOptions []OutputWriterOption + SpinnerOptions []OutputWriterSpinnerOption +} + +// OutputWriterSpinnerOption is an option for outputwriterspinner +type OutputWriterSpinnerOption func(*outputwriterspinner) + +// WithSpinnerFinalMsg sets the spinner final text and prefix log indicator +// (log.LogTypeOUTPUT can be used for no prefix) +func WithSpinnerFinalMsg(finalText string, prefix log.LogType) OutputWriterSpinnerOption { + finalText = fmt.Sprintf("%s%s", log.GetLogTypeIndicator(prefix), finalText) + return func(ows *outputwriterspinner) { + ows.spinnerFinalText = finalText + } } // NewOutputWriterWithSpinner returns implementation of OutputWriterSpinner. // // Deprecated: NewOutputWriterWithSpinner is being deprecated in favor of -// NewOutputWriterspinnerWithOptions. +// NewOutputWriterSpinnerWithSpinnerOptions. // Until it is removed, it will retain the existing behavior of converting // incoming row values to their golang string representation for backward // compatibility reasons @@ -40,6 +63,9 @@ func NewOutputWriterWithSpinner(output io.Writer, outputFormat, spinnerText stri } // NewOutputWriterSpinnerWithOptions returns implementation of OutputWriterSpinner. +// +// Deprecated: NewOutputWriterSpinnerWithOptions is being deprecated in favor of +// NewOutputWriterSpinnerWithSpinnerOptions. func NewOutputWriterSpinnerWithOptions(output io.Writer, outputFormat, spinnerText string, startSpinner bool, opts []OutputWriterOption, headers ...string) (OutputWriterSpinner, error) { ows := &outputwriterspinner{} ows.out = output @@ -47,6 +73,22 @@ func NewOutputWriterSpinnerWithOptions(output io.Writer, outputFormat, spinnerTe ows.keys = headers ows.applyOptions(opts) + return setAndInitializeSpinner(ows, spinnerText, startSpinner) +} + +// NewOutputWriterSpinnerWithSpinnerOptions returns implementation of OutputWriterSpinner. +func NewOutputWriterSpinnerWithSpinnerOptions(output io.Writer, outputFormat OutputType, spinnerText string, startSpinner bool, opts OutputWriterSpinnerOptions, headers ...string) (OutputWriterSpinner, error) { + ows := &outputwriterspinner{} + ows.out = output + ows.outputFormat = outputFormat + ows.keys = headers + ows.applyOptions(opts.OutputWriterOptions) + ows.applyOutputWriterSpinnerOptions(opts.SpinnerOptions) + return setAndInitializeSpinner(ows, spinnerText, startSpinner) +} + +// setAndInitializeSpinner sets the spinner text and initializes the spinner +func setAndInitializeSpinner(ows *outputwriterspinner, spinnerText string, startSpinner bool) (OutputWriterSpinner, error) { if ows.outputFormat != JSONOutputType && ows.outputFormat != YAMLOutputType { ows.spinnerText = spinnerText ows.spinner = spinner.New(spinner.CharSets[9], 100*time.Millisecond) @@ -54,6 +96,9 @@ func NewOutputWriterSpinnerWithOptions(output io.Writer, outputFormat, spinnerTe return nil, err } ows.spinner.Suffix = fmt.Sprintf(" %s", spinnerText) + if ows.spinnerFinalText != "" { + spinner.WithFinalMSG(ows.spinnerFinalText)(ows.spinner) + } // Start the spinner only if attached to terminal attachedToTerminal := isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()) @@ -80,3 +125,19 @@ func (ows *outputwriterspinner) StopSpinner() { fmt.Fprintln(ows.out) } } + +// SetFinalText sets the spinner final text and prefix log indicator +// (log.LogTypeOUTPUT can be used for no prefix) +func (ows *outputwriterspinner) SetFinalText(finalText string, prefix log.LogType) { + if ows.spinner != nil { + ows.spinnerFinalText = fmt.Sprintf("%s%s", log.GetLogTypeIndicator(prefix), finalText) + spinner.WithFinalMSG(ows.spinnerFinalText)(ows.spinner) + } +} + +// applyOutputWriterSpinnerOptions applies the options to the outputwriterspinner +func (ows *outputwriterspinner) applyOutputWriterSpinnerOptions(spinnerOpts []OutputWriterSpinnerOption) { + for i := range spinnerOpts { + spinnerOpts[i](ows) + } +} diff --git a/component/output_spinner_test.go b/component/output_spinner_test.go new file mode 100644 index 000000000..f4e708d7c --- /dev/null +++ b/component/output_spinner_test.go @@ -0,0 +1,107 @@ +// Copyright 2024 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package component + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/vmware-tanzu/tanzu-plugin-runtime/log" +) + +const loading = "Loading..." + +func TestNewOutputWriterWithSpinner(t *testing.T) { + output := bytes.Buffer{} + spinnerText := loading + headers := []string{"Name", "Age"} + + // Test creating an OutputWriterSpinner with a spinner + ows, err := NewOutputWriterWithSpinner(&output, "table", spinnerText, true, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Test creating an OutputWriterSpinner without a spinner + ows, err = NewOutputWriterWithSpinner(&output, "table", spinnerText, false, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Test creating an OutputWriterSpinner with unsupported output format + ows, err = NewOutputWriterWithSpinner(&output, "unsupported", spinnerText, true, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) +} + +func TestNewOutputWriterSpinnerWithOptions(t *testing.T) { + output := bytes.Buffer{} + spinnerText := loading + headers := []string{"Name", "Age"} + + // Test creating an OutputWriterSpinner with options and a spinner + opts := []OutputWriterOption{WithAutoStringify()} + ows, err := NewOutputWriterSpinnerWithOptions(&output, "table", spinnerText, true, opts, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Test creating an OutputWriterSpinner with options without a spinner + opts = []OutputWriterOption{WithAutoStringify()} + ows, err = NewOutputWriterSpinnerWithOptions(&output, "table", spinnerText, false, opts, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Test creating an OutputWriterSpinner with unsupported output format + opts = []OutputWriterOption{WithAutoStringify()} + ows, err = NewOutputWriterSpinnerWithOptions(&output, "unsupported", spinnerText, true, opts, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) +} + +func TestNewOutputWriterSpinnerWithSpinnerOptions(t *testing.T) { + output := bytes.Buffer{} + spinnerText := loading + headers := []string{"Name", "Age"} + + // Test creating an OutputWriterSpinner with spinner options and a spinner + opts := OutputWriterSpinnerOptions{ + OutputWriterOptions: []OutputWriterOption{WithAutoStringify()}, + SpinnerOptions: []OutputWriterSpinnerOption{WithSpinnerFinalMsg("Done!", log.LogTypeSUCCESS)}, + } + ows, err := NewOutputWriterSpinnerWithSpinnerOptions(&output, "table", spinnerText, true, opts, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Test creating an OutputWriterSpinner with spinner options without a spinner + opts = OutputWriterSpinnerOptions{ + SpinnerOptions: []OutputWriterSpinnerOption{WithSpinnerFinalMsg("Done!", log.LogTypeSUCCESS)}, + } + ows, err = NewOutputWriterSpinnerWithSpinnerOptions(&output, "table", spinnerText, false, opts, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Test creating an OutputWriterSpinner with unsupported output format + opts = OutputWriterSpinnerOptions{} + ows, err = NewOutputWriterSpinnerWithSpinnerOptions(&output, "unsupported", spinnerText, true, opts, headers...) + assert.NoError(t, err) + assert.NotNil(t, ows) +} + +func TestOutputWriterSpinnerRenderWithSpinner(t *testing.T) { + output := bytes.Buffer{} + spinnerText := loading + headers := []string{"Name", "Age"} + + // Create an OutputWriterSpinner with a spinner + ows, err := NewOutputWriterWithSpinner(&output, "table", spinnerText, true, headers...) + ows.AddRow(map[string]interface{}{"Name": "John", "Age": 30}) + assert.NoError(t, err) + assert.NotNil(t, ows) + + // Render with spinner + ows.RenderWithSpinner() + assert.Contains(t, output.String(), "NAME") + assert.Contains(t, output.String(), "John") + assert.Contains(t, output.String(), "30") +} diff --git a/log/log.go b/log/log.go index 7441c41be..cb194d6b0 100644 --- a/log/log.go +++ b/log/log.go @@ -85,58 +85,58 @@ var l = NewLogger() // Info logs a non-error message with the given key/value pairs as context. func Info(msg string, kvs ...interface{}) { - l.Print(msg, nil, logTypeINFO, kvs...) + l.Print(msg, nil, string(LogTypeINFO), kvs...) } // Infof logs a non-error message with the given key/value pairs as context. func Infof(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeINFO) + l.Print(msg, nil, string(LogTypeINFO)) } // Error logs an error message with the given key/value pairs as context. func Error(err error, msg string, kvs ...interface{}) { - l.Print(msg, err, logTypeERROR, kvs...) + l.Print(msg, err, string(LogTypeERROR), kvs...) } // Errorf logs a error message with the given key/value pairs as context. func Errorf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeERROR) + l.Print(msg, nil, string(LogTypeERROR)) } // Warning logs a warning messages with the given key/value pairs as context. func Warning(msg string, kvs ...interface{}) { - l.Print(msg, nil, logTypeWARN, kvs...) + l.Print(msg, nil, string(LogTypeWARN), kvs...) } // Warningf logs a warning messages with the given message format with format specifier and arguments. func Warningf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeWARN) + l.Print(msg, nil, string(LogTypeWARN)) } // Success logs a success messages with the given key/value pairs as context. func Success(msg string, kvs ...interface{}) { - l.Print(msg, nil, logTypeSUCCESS, kvs...) + l.Print(msg, nil, string(LogTypeSUCCESS), kvs...) } // Successf logs a success messages with the given message format with format specifier and arguments. func Successf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeSUCCESS) + l.Print(msg, nil, string(LogTypeSUCCESS)) } // Fatal logs a fatal message with the given key/value pairs as context and returns with os.Exit(1) func Fatal(err error, msg string, kvs ...interface{}) { - l.Print(msg, err, logTypeERROR, kvs...) + l.Print(msg, err, string(LogTypeERROR), kvs...) os.Exit(1) } // Outputf writes a message to stdout func Outputf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeOUTPUT) + l.Print(msg, nil, string(LogTypeOUTPUT)) } // V returns an InfoLogger value for a specific verbosity level. diff --git a/log/logger.go b/log/logger.go index 3de3355f4..8408ed609 100644 --- a/log/logger.go +++ b/log/logger.go @@ -88,58 +88,58 @@ func (l *logger) Enabled() bool { // Info logs a non-error message with the given key/value pairs as context. func (l *logger) Info(msg string, kvs ...interface{}) { - l.Print(msg, nil, logTypeINFO, kvs...) + l.Print(msg, nil, string(LogTypeINFO), kvs...) } // Infof logs a non-error messages with the given message format with format specifier and arguments. func (l *logger) Infof(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeINFO) + l.Print(msg, nil, string(LogTypeINFO)) } // Error logs an error message with the given key/value pairs as context. func (l *logger) Error(err error, msg string, kvs ...interface{}) { - l.Print(msg, err, logTypeERROR, kvs...) + l.Print(msg, err, string(LogTypeERROR), kvs...) } // Errorf logs a error message with the given key/value pairs as context. func (l *logger) Errorf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeERROR) + l.Print(msg, nil, string(LogTypeERROR)) } // Warning logs a warning messages with the given key/value pairs as context. func (l *logger) Warning(msg string, kvs ...interface{}) { - l.Print(msg, nil, logTypeWARN, kvs...) + l.Print(msg, nil, string(LogTypeWARN), kvs...) } // Warningf logs a warning messages with the given message format with format specifier and arguments. func (l *logger) Warningf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeWARN) + l.Print(msg, nil, string(LogTypeWARN)) } // Success logs a success messages with the given key/value pairs as context. func (l *logger) Success(msg string, kvs ...interface{}) { - l.Print(msg, nil, logTypeSUCCESS, kvs...) + l.Print(msg, nil, string(LogTypeSUCCESS), kvs...) } // Successf logs a success messages with the given message format with format specifier and arguments. func (l *logger) Successf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeSUCCESS) + l.Print(msg, nil, string(LogTypeSUCCESS)) } // Fatal logs a fatal message with the given key/value pairs as context and returns with os.exit(1) func (l *logger) Fatal(err error, msg string, kvs ...interface{}) { - l.Print(msg, err, logTypeERROR, kvs...) + l.Print(msg, err, string(LogTypeERROR), kvs...) os.Exit(1) } // Outputf writes a message to stdout func (l *logger) Outputf(format string, args ...interface{}) { msg := fmt.Sprintf(format, args...) - l.Print(msg, nil, logTypeOUTPUT) + l.Print(msg, nil, string(LogTypeOUTPUT)) } // V returns an InfoLogger value for a specific verbosity level. @@ -212,25 +212,14 @@ func (l *logger) getLogString(values []interface{}) string { } f, err := flatten(entry) if err != nil { - _, _ = logWriter.Write([]byte{}, []byte(err.Error()), l.Enabled(), 0, logTypeWARN) + _, _ = logWriter.Write([]byte{}, []byte(err.Error()), l.Enabled(), 0, string(LogTypeWARN)) return "" } return f } func (l *logger) getLogTypeIndicator(logType string) string { - switch logType { - case logTypeINFO: - return "[i] " - case logTypeWARN: - return "[!] " - case logTypeERROR: - return "[x] " - case logTypeSUCCESS: - return "[ok] " - case logTypeOUTPUT: - } - return "" + return GetLogTypeIndicator(LogType(logType)) } func copySlice(in []interface{}) []interface{} { diff --git a/log/types.go b/log/types.go index 7ec25b784..9cb9814f5 100644 --- a/log/types.go +++ b/log/types.go @@ -3,10 +3,27 @@ package log +type LogType string + const ( - logTypeINFO = "INFO" - logTypeWARN = "WARN" - logTypeERROR = "ERROR" - logTypeSUCCESS = "SUCCESS" - logTypeOUTPUT = "OUTPUT" + LogTypeINFO LogType = "INFO" + LogTypeWARN LogType = "WARN" + LogTypeERROR LogType = "ERROR" + LogTypeSUCCESS LogType = "SUCCESS" + LogTypeOUTPUT LogType = "OUTPUT" ) + +func GetLogTypeIndicator(logType LogType) string { + switch logType { + case LogTypeINFO: + return "[i] " + case LogTypeWARN: + return "[!] " + case LogTypeERROR: + return "[x] " + case LogTypeSUCCESS: + return "[ok] " + case LogTypeOUTPUT: + } + return "" +} diff --git a/log/writer.go b/log/writer.go index e280ff089..362d965b7 100644 --- a/log/writer.go +++ b/log/writer.go @@ -130,13 +130,13 @@ func (w *writer) Write(header, msg []byte, logEnabled bool, logVerbosity int32, // If showTimestamp is set to true, log fullMsg with header // If log type is OUTPUT skip the header - if w.showTimestamp && logType != logTypeOUTPUT { + if w.showTimestamp && logType != string(LogTypeOUTPUT) { msg = fullMsg } // write to stdout/stderr if quiet mode is not set and logEnabled is true if !w.quiet && logEnabled { - if logType == logTypeOUTPUT { + if logType == string(LogTypeOUTPUT) { w.stdoutWriter(msg) } else { w.stderrWriter(msg)