fix(text): ensure error logs go to stderr (Jguer#2105)

ensure Error logs go to stderr

aur_install_test.go

@@ -21,7 +21,9 @@ import (
  "github.com/Jguer/yay/v12/pkg/vcs"
 )
 
-var testLogger = text.NewLogger(io.Discard, strings.NewReader(""), true, "test")
+func NewTestLogger() *text.Logger {
+ return text.NewLogger(io.Discard, io.Discard, strings.NewReader(""), true, "test")
+}
 
 func ptrString(s string) *string {
  return &s
@@ -131,7 +133,7 @@ func TestInstaller_InstallNeeded(t *testing.T) {
 
  cmdBuilder.Runner = mockRunner
 
- installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)
+ installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger())
 
  cmdArgs := parser.MakeArguments()
  cmdArgs.AddArg("needed")
@@ -405,7 +407,7 @@ func TestInstaller_InstallMixedSourcesAndLayers(t *testing.T) {
 
  cmdBuilder.Runner = mockRunner
 
- installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)
+ installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger())
 
  cmdArgs := parser.MakeArguments()
  cmdArgs.AddTarget("yay")
@@ -458,7 +460,7 @@ func TestInstaller_RunPostHooks(t *testing.T) {
 
  cmdBuilder.Runner = mockRunner
 
- installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)
+ installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger())
 
  called := false
  hook := func(ctx context.Context) error {
@@ -588,7 +590,7 @@ func TestInstaller_CompileFailed(t *testing.T) {
 
  cmdBuilder.Runner = mockRunner
 
- installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)
+ installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger())
 
  cmdArgs := parser.MakeArguments()
  cmdArgs.AddArg("needed")
@@ -746,7 +748,7 @@ func TestInstaller_InstallSplitPackage(t *testing.T) {
 
  cmdBuilder.Runner = mockRunner
 
- installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, testLogger)
+ installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, false, NewTestLogger())
 
  cmdArgs := parser.MakeArguments()
  cmdArgs.AddTarget("jellyfin")
@@ -884,7 +886,7 @@ func TestInstaller_InstallDownloadOnly(t *testing.T) {
 
  cmdBuilder.Runner = mockRunner
 
- installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, true, testLogger)
+ installer := NewInstaller(mockDB, cmdBuilder, &vcs.Mock{}, parser.ModeAny, true, NewTestLogger())
 
  cmdArgs := parser.MakeArguments()
  cmdArgs.AddTarget("yay")

local_install_test.go

@@ -3,7 +3,6 @@ package main
 import (
  "context"
  "fmt"
- "io"
  "os"
  "os/exec"
  "path/filepath"
@@ -20,7 +19,6 @@ import (
  "github.com/Jguer/yay/v12/pkg/settings"
  "github.com/Jguer/yay/v12/pkg/settings/exe"
  "github.com/Jguer/yay/v12/pkg/settings/parser"
- "github.com/Jguer/yay/v12/pkg/text"
  "github.com/Jguer/yay/v12/pkg/vcs"
 )
 
@@ -140,7 +138,7 @@ func TestIntegrationLocalInstall(t *testing.T) {
  config := &settings.Configuration{
  RemoveMake: "no",
  Runtime: &settings.Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), true, "test"),
+ Logger: NewTestLogger(),
  CmdBuilder: cmdBuilder,
  VCSStore: &vcs.Mock{},
  AURCache: &mockaur.MockAUR{
@@ -259,7 +257,7 @@ func TestIntegrationLocalInstallMissingDep(t *testing.T) {
 
  config := &settings.Configuration{
  Runtime: &settings.Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), true, "test"),
+ Logger: NewTestLogger(),
  CmdBuilder: cmdBuilder,
  VCSStore: &vcs.Mock{},
  AURCache: &mockaur.MockAUR{
@@ -417,7 +415,7 @@ func TestIntegrationLocalInstallNeeded(t *testing.T) {
  config := &settings.Configuration{
  RemoveMake: "no",
  Runtime: &settings.Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), true, "test"),
+ Logger: NewTestLogger(),
  CmdBuilder: cmdBuilder,
  VCSStore: &vcs.Mock{},
  AURCache: &mockaur.MockAUR{
@@ -578,7 +576,7 @@ func TestIntegrationLocalInstallGenerateSRCINFO(t *testing.T) {
  RemoveMake: "no",
  Debug: false,
  Runtime: &settings.Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), true, "test"),
+ Logger: NewTestLogger(),
  CmdBuilder: cmdBuilder,
  VCSStore: &vcs.Mock{},
  AURCache: &mockaur.MockAUR{
@@ -714,7 +712,7 @@ func TestIntegrationLocalInstallMissingFiles(t *testing.T) {
  config := &settings.Configuration{
  RemoveMake: "no",
  Runtime: &settings.Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), true, "test"),
+ Logger: NewTestLogger(),
  CmdBuilder: cmdBuilder,
  VCSStore: &vcs.Mock{},
  AURCache: &mockaur.MockAUR{

pkg/db/ialpm/alpm_test.go

@@ -45,7 +45,7 @@ func TestAlpmExecutor(t *testing.T) {
  },
  }
 
- aExec, err := NewExecutor(pacmanConf, text.NewLogger(io.Discard, strings.NewReader(""), false, "test"))
+ aExec, err := NewExecutor(pacmanConf, text.NewLogger(io.Discard, io.Discard, strings.NewReader(""), false, "test"))
  assert.NoError(t, err)
 
  assert.NotNil(t, aExec.conf)

pkg/dep/dep_graph_test.go

@@ -197,7 +197,7 @@ func TestGrapher_GraphFromTargets_jellyfin(t *testing.T) {
  g := NewGrapher(tt.fields.dbExecutor,
  tt.fields.aurCache, false, true,
  tt.fields.noDeps, tt.fields.noCheckDeps, false,
- text.NewLogger(io.Discard, &os.File{}, true, "test"))
+ text.NewLogger(io.Discard, io.Discard, &os.File{}, true, "test"))
  got, err := g.GraphFromTargets(context.Background(), nil, tt.args.targets)
  require.NoError(t, err)
  layers := got.TopoSortedLayerMap(nil)

pkg/query/query_builder_test.go

@@ -2,6 +2,7 @@ package query
 
 import (
  "context"
+ "io"
  "strings"
  "testing"
 
@@ -39,7 +40,7 @@ func TestMixedSourceQueryBuilder(t *testing.T) {
 
  w := &strings.Builder{}
  queryBuilder := NewSourceQueryBuilder(client,
- text.NewLogger(w, strings.NewReader(""), false, "test"),
+ text.NewLogger(w, io.Discard, strings.NewReader(""), false, "test"),
  "votes", parser.ModeAny, "", tc.bottomUp, false, false)
  search := []string{"linux"}
  mockStore := &mockDB{}

pkg/query/source_test.go

@@ -110,7 +110,7 @@ func TestSourceQueryBuilder(t *testing.T) {
  require.NoError(t, err)
 
  queryBuilder := NewSourceQueryBuilder(client,
- text.NewLogger(io.Discard, bytes.NewBufferString(""), false, "test"),
+ text.NewLogger(io.Discard, io.Discard, bytes.NewBufferString(""), false, "test"),
  "votes", parser.ModeAny, "", tc.bottomUp, false, true)
  search := []string{"linux"}
  mockStore := &mockDB{}

pkg/settings/migrations_test.go

@@ -26,7 +26,7 @@ func TestMigrationNothingToDo(t *testing.T) {
  Version: "99.0.0",
  // Create runtime with runtimeVersion
  Runtime: &Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
+ Logger: text.NewLogger(io.Discard, io.Discard, strings.NewReader(""), false, "test"),
  },
  }
 
@@ -51,7 +51,7 @@ func TestProvidesMigrationDo(t *testing.T) {
  config := Configuration{
  Provides: true,
  Runtime: &Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
+ Logger: text.NewLogger(io.Discard, io.Discard, strings.NewReader(""), false, "test"),
  },
  }
 
@@ -133,7 +133,7 @@ func TestProvidesMigration(t *testing.T) {
  Provides: tc.testConfig.Provides,
  // Create runtime with runtimeVersion
  Runtime: &Runtime{
- Logger: text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
+ Logger: text.NewLogger(io.Discard, io.Discard, strings.NewReader(""), false, "test"),
  },
  }
 

pkg/settings/runtime.go

@@ -37,7 +37,7 @@ type Runtime struct {
 }
 
 func BuildRuntime(cfg *Configuration, cmdArgs *parser.Arguments, version string) (*Runtime, error) {
- logger := text.NewLogger(os.Stdout, os.Stdin, cfg.Debug, "runtime")
+ logger := text.NewLogger(os.Stdout, os.Stderr, os.Stdin, cfg.Debug, "runtime")
  cmdBuilder := cfg.CmdBuilder(nil)
 
  httpClient := &http.Client{
@@ -105,7 +105,7 @@ func BuildRuntime(cfg *Configuration, cmdArgs *parser.Arguments, version string)
  VoteClient: voteClient,
  AURCache: aurCache,
  DBExecutor: nil,
- Logger: text.NewLogger(os.Stdout, os.Stdin, cfg.Debug, "runtime"),
+ Logger: text.NewLogger(os.Stdout, os.Stderr, os.Stdin, cfg.Debug, "runtime"),
  }
 
  return runtime, nil

pkg/text/print.go

@@ -20,7 +20,7 @@ const (
 
 var (
  cachedColumnCount = -1
- GlobalLogger = NewLogger(os.Stdout, os.Stdin, false, "global")
+ GlobalLogger = NewLogger(os.Stdout, os.Stderr, os.Stdin, false, "global")
 )
 
 func Debugln(a ...interface{}) {

pkg/text/service.go

@@ -6,23 +6,25 @@ import (
 )
 
 type Logger struct {
- name string
- Debug bool
- w io.Writer
- r io.Reader
+ name string
+ Debug bool
+ stdout io.Writer
+ stderr io.Writer
+ r io.Reader
 }
 
-func NewLogger(w io.Writer, r io.Reader, debug bool, name string) *Logger {
+func NewLogger(stdout, stderr io.Writer, r io.Reader, debug bool, name string) *Logger {
  return &Logger{
- w: w,
- name: name,
- Debug: debug,
- r: r,
+ Debug: debug,
+ name: name,
+ r: r,
+ stderr: stderr,
+ stdout: stdout,
  }
 }
 
 func (l *Logger) Child(name string) *Logger {
- return NewLogger(l.w, l.r, l.Debug, name)
+ return NewLogger(l.stdout, l.stderr, l.r, l.Debug, name)
 }
 
 func (l *Logger) Debugln(a ...any) {
@@ -68,25 +70,25 @@ func (l *Logger) SprintWarn(a ...any) string {
 }
 
 func (l *Logger) Error(a ...any) {
- l.Print(l.SprintError(a...))
+ fmt.Fprint(l.stderr, l.SprintError(a...))
 }
 
 func (l *Logger) Errorln(a ...any) {
- l.Println(l.SprintError(a...))
+ fmt.Fprintln(l.stderr, l.SprintError(a...))
 }
 
 func (l *Logger) SprintError(a ...any) string {
  return fmt.Sprint(append([]interface{}{Bold(Red(smallArrow + " "))}, a...)...)
 }
 
 func (l *Logger) Printf(format string, a ...any) {
- fmt.Fprintf(l.w, format, a...)
+ fmt.Fprintf(l.stdout, format, a...)
 }
 
 func (l *Logger) Println(a ...any) {
- fmt.Fprintln(l.w, a...)
+ fmt.Fprintln(l.stdout, a...)
 }
 
 func (l *Logger) Print(a ...any) {
- fmt.Fprint(l.w, a...)
+ fmt.Fprint(l.stdout, a...)
 }

pkg/upgrade/service_test.go

@@ -3,6 +3,7 @@ package upgrade
 import (
  "context"
  "io"
+ "os"
  "strings"
  "testing"
 
@@ -335,14 +336,14 @@ func TestUpgradeService_GraphUpgrades(t *testing.T) {
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
  grapher := dep.NewGrapher(dbExe, mockAUR,
- false, true, false, false, false, text.NewLogger(tt.fields.output,
+ false, true, false, false, false, text.NewLogger(tt.fields.output, os.Stderr,
  tt.fields.input, true, "test"))
 
  cfg := &settings.Configuration{
  Devel: tt.fields.devel, Mode: parser.ModeAny,
  }
 
- logger := text.NewLogger(tt.fields.output,
+ logger := text.NewLogger(tt.fields.output, os.Stderr,
  tt.fields.input, true, "test")
  u := &UpgradeService{
  log: logger,
@@ -456,15 +457,15 @@ func TestUpgradeService_GraphUpgradesNoUpdates(t *testing.T) {
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
  grapher := dep.NewGrapher(dbExe, mockAUR,
- false, true, false, false, false, text.NewLogger(tt.fields.output,
+ false, true, false, false, false, text.NewLogger(tt.fields.output, os.Stderr,
  tt.fields.input, true, "test"))
 
  cfg := &settings.Configuration{
  Devel: tt.fields.devel,
  Mode: parser.ModeAny,
  }
 
- logger := text.NewLogger(tt.fields.output,
+ logger := text.NewLogger(tt.fields.output, os.Stderr,
  tt.fields.input, true, "test")
  u := &UpgradeService{
  log: logger,
@@ -568,7 +569,7 @@ func TestUpgradeService_Warnings(t *testing.T) {
  },
  }
 
- logger := text.NewLogger(io.Discard,
+ logger := text.NewLogger(io.Discard, os.Stderr,
  strings.NewReader("\n"), true, "test")
  grapher := dep.NewGrapher(dbExe, mockAUR,
  false, true, false, false, false, logger)

pkg/upgrade/sources_test.go

@@ -3,6 +3,7 @@ package upgrade
 import (
  "context"
  "io"
+ "os"
  "strings"
  "testing"
  "time"
@@ -122,7 +123,7 @@ func Test_upAUR(t *testing.T) {
  t.Run(tt.name, func(t *testing.T) {
  t.Parallel()
 
- got := UpAUR(text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
+ got := UpAUR(text.NewLogger(io.Discard, os.Stderr, strings.NewReader(""), false, "test"),
  tt.args.remote, tt.args.aurdata, tt.args.timeUpdate, tt.args.enableDowngrade)
  assert.ElementsMatch(t, tt.want.Repos, got.Repos)
  assert.ElementsMatch(t, tt.want.Up, got.Up)
@@ -228,7 +229,7 @@ func Test_upDevel(t *testing.T) {
  t.Run(tt.name, func(t *testing.T) {
  t.Parallel()
  got := UpDevel(context.Background(),
- text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
+ text.NewLogger(io.Discard, os.Stderr, strings.NewReader(""), false, "test"),
  tt.args.remote, tt.args.aurdata, tt.args.cached)
  assert.ElementsMatch(t, tt.want.Up, got.Up)
  })

pkg/vcs/vcs_test.go

@@ -72,7 +72,7 @@ func TestNewInfoStore(t *testing.T) {
  args: args{
  "/tmp/a.json",
  &exe.CmdBuilder{GitBin: "git", GitFlags: []string{"--a", "--b"}, Runner: &exe.OSRunner{
- Log: text.NewLogger(io.Discard, strings.NewReader(""), true, "test"),
+ Log: text.NewLogger(io.Discard, os.Stderr, strings.NewReader(""), true, "test"),
  }},
  },
  },
@@ -82,7 +82,7 @@ func TestNewInfoStore(t *testing.T) {
  t.Run(tt.name, func(t *testing.T) {
  t.Parallel()
  got := NewInfoStore(tt.args.filePath, tt.args.cmdBuilder,
- text.NewLogger(io.Discard, strings.NewReader(""), true, "test"))
+ text.NewLogger(io.Discard, os.Stderr, strings.NewReader(""), true, "test"))
  assert.NotNil(t, got)
  assert.Equal(t, []string{"--a", "--b"}, got.CmdBuilder.(*exe.CmdBuilder).GitFlags)
  assert.Equal(t, tt.args.cmdBuilder, got.CmdBuilder)
@@ -525,7 +525,7 @@ func TestInfoStore_CleanOrphans(t *testing.T) {
  v := &InfoStore{
  OriginsByPackage: tt.fields.OriginsByPackage,
  FilePath: filePath,
- logger: text.NewLogger(io.Discard, strings.NewReader(""), false, "test"),
+ logger: text.NewLogger(io.Discard, os.Stderr, strings.NewReader(""), false, "test"),
  }
  v.CleanOrphans(tt.args.pkgs)
  assert.Len(t, tt.fields.OriginsByPackage, 3)