Skip to content

Commit

Permalink
Fix: still warn on zero verbosity (#68)
Browse files Browse the repository at this point in the history
* 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 <willmurphyscode@users.noreply.github.com>

* feat: respect NO_TTY env var when setting up UI

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>

* test: unit test for warn logging

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>

---------

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>
  • Loading branch information
willmurphyscode authored Nov 15, 2024
1 parent 9644c74 commit 6a13fc0
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
16 changes: 15 additions & 1 deletion logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
33 changes: 33 additions & 0 deletions logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -258,6 +271,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 {
Expand Down

0 comments on commit 6a13fc0

Please # to comment.