From ee7b8910106fa81027ff754196084d18ed026beb Mon Sep 17 00:00:00 2001 From: Will Murphy Date: Wed, 16 Oct 2024 10:54:08 -0400 Subject: [PATCH 1/3] fix: allocate stderr logger if log.Warn needed Previously, there was an issue where if there was no TTY, and verbosity was zero, the logger would be initialized with io.Discard instead of stderr. Log to stderr unless "--quiet" is passed. Signed-off-by: Will Murphy --- logging.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logging.go b/logging.go index d889013..79c2d8a 100644 --- a/logging.go +++ b/logging.go @@ -41,7 +41,7 @@ func DefaultLogger(clioCfg Config, store redact.Store) (logger.Logger, error) { l, err := logrus.New( logrus.Config{ - EnableConsole: cfg.Verbosity > 0 && !cfg.Quiet, + EnableConsole: !cfg.Quiet, FileLocation: cfg.FileLocation, Level: cfg.Level, Formatter: adaptLogFormatter(logrus.DefaultTextFormatter()), From adf59de429347ac1bcdb9fbbbfb836a8a445c14c Mon Sep 17 00:00:00 2001 From: Will Murphy Date: Wed, 16 Oct 2024 11:22:33 -0400 Subject: [PATCH 2/3] feat: respect NO_TTY env var when setting up UI Signed-off-by: Will Murphy --- logging.go | 14 ++++++++++++++ logging_test.go | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/logging.go b/logging.go index 79c2d8a..2e17d77 100644 --- a/logging.go +++ b/logging.go @@ -163,6 +163,10 @@ func (l *LoggingConfig) selectLevel() (logger.Level, error) { } func (l *LoggingConfig) AllowUI(stdin fs.File) bool { + if forceNoTTY(os.Getenv("NO_TTY")) { + return false + } + pipedInput, err := isPipedInput(stdin) if err != nil || pipedInput { // since we can't tell if there was piped input we assume that there could be to disable the ETUI @@ -188,6 +192,16 @@ func (l *LoggingConfig) AllowUI(stdin fs.File) bool { return l.Verbosity == 0 } +func forceNoTTY(noTTYenvVar string) bool { + switch strings.ToLower(noTTYenvVar) { + case "1": + return true + case "true": + return true + } + return false +} + // isPipedInput returns true if there is no input device, which means the user **may** be providing input via a pipe. func isPipedInput(stdin fs.File) (bool, error) { if stdin == nil { diff --git a/logging_test.go b/logging_test.go index ecb3ee6..df1a713 100644 --- a/logging_test.go +++ b/logging_test.go @@ -258,6 +258,26 @@ func TestLoggingConfig_AllowUI(t *testing.T) { } } +func TestForceNoTTY(t *testing.T) { + tests := []struct { + envVarValue string + want bool + }{ + {"", false}, + {"0", false}, + {"false", false}, + {"other-string", false}, + {"1", true}, + {"true", true}, + } + + for _, tt := range tests { + t.Run(tt.envVarValue, func(t *testing.T) { + assert.Equal(t, tt.want, forceNoTTY(tt.envVarValue)) + }) + } +} + func Test_isPipedInput(t *testing.T) { tests := []struct { From b98ebbbc6668c2fc54f65238cb0772ed12a6bd3d Mon Sep 17 00:00:00 2001 From: Will Murphy Date: Wed, 16 Oct 2024 11:35:35 -0400 Subject: [PATCH 3/3] test: unit test for warn logging Signed-off-by: Will Murphy --- logging_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/logging_test.go b/logging_test.go index df1a713..0c86857 100644 --- a/logging_test.go +++ b/logging_test.go @@ -83,6 +83,19 @@ func Test_newLogger(t *testing.T) { assert.Equal(t, "[0000] INFO test *******\n", stripAnsi(buf.String())) }, }, + { + name: "warn logging is not discarded", + cfg: &LoggingConfig{Level: "warn"}, + assertLogger: func(log logger.Logger) { + c, ok := log.(logger.Controller) + require.True(t, ok) + out := c.GetOutput() + // attempt to cast as *os.File, which will fail if output + // has defaulted to io.Discard + _, ok = out.(*os.File) + require.True(t, ok) + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {