From c7a51a1614127198b6ff4bf14b2501fce6c7b9f5 Mon Sep 17 00:00:00 2001 From: Jo Date: Mon, 10 Apr 2023 18:26:09 +0200 Subject: [PATCH] fix(text): ensure error logs go to stderr (#2105) ensure Error logs go to stderr --- aur_install_test.go | 16 +++++++++------- local_install_test.go | 12 +++++------- pkg/db/ialpm/alpm_test.go | 2 +- pkg/dep/dep_graph_test.go | 2 +- pkg/query/query_builder_test.go | 3 ++- pkg/query/source_test.go | 2 +- pkg/settings/migrations_test.go | 6 +++--- pkg/settings/runtime.go | 4 ++-- pkg/text/print.go | 2 +- pkg/text/service.go | 32 +++++++++++++++++--------------- pkg/upgrade/service_test.go | 11 ++++++----- pkg/upgrade/sources_test.go | 5 +++-- pkg/vcs/vcs_test.go | 6 +++--- print.go | 2 +- sync_test.go | 10 +++++----- 15 files changed, 60 insertions(+), 55 deletions(-) diff --git a/aur_install_test.go b/aur_install_test.go index 8be928a9..255b1a36 100644 --- a/aur_install_test.go +++ b/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") diff --git a/local_install_test.go b/local_install_test.go index 5f1b90a9..f43615d5 100644 --- a/local_install_test.go +++ b/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{ diff --git a/pkg/db/ialpm/alpm_test.go b/pkg/db/ialpm/alpm_test.go index c8897293..cccc9f23 100644 --- a/pkg/db/ialpm/alpm_test.go +++ b/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) diff --git a/pkg/dep/dep_graph_test.go b/pkg/dep/dep_graph_test.go index 278bcb6e..4a8c89a9 100644 --- a/pkg/dep/dep_graph_test.go +++ b/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) diff --git a/pkg/query/query_builder_test.go b/pkg/query/query_builder_test.go index e7e2e3be..19af1ca8 100644 --- a/pkg/query/query_builder_test.go +++ b/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{} diff --git a/pkg/query/source_test.go b/pkg/query/source_test.go index d6ba69cb..9cad2756 100644 --- a/pkg/query/source_test.go +++ b/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{} diff --git a/pkg/settings/migrations_test.go b/pkg/settings/migrations_test.go index 2c8d960c..dee0f7ea 100644 --- a/pkg/settings/migrations_test.go +++ b/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"), }, } diff --git a/pkg/settings/runtime.go b/pkg/settings/runtime.go index 26c98cfb..275da79c 100644 --- a/pkg/settings/runtime.go +++ b/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 diff --git a/pkg/text/print.go b/pkg/text/print.go index f2b6d320..ae939bfc 100644 --- a/pkg/text/print.go +++ b/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{}) { diff --git a/pkg/text/service.go b/pkg/text/service.go index 1e448c39..29733b37 100644 --- a/pkg/text/service.go +++ b/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,11 +70,11 @@ 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 { @@ -80,13 +82,13 @@ func (l *Logger) SprintError(a ...any) string { } 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...) } diff --git a/pkg/upgrade/service_test.go b/pkg/upgrade/service_test.go index bfcc705c..c24bd5c4 100644 --- a/pkg/upgrade/service_test.go +++ b/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,7 +457,7 @@ 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{ @@ -464,7 +465,7 @@ func TestUpgradeService_GraphUpgradesNoUpdates(t *testing.T) { 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) diff --git a/pkg/upgrade/sources_test.go b/pkg/upgrade/sources_test.go index 62dbc4d0..ddeec512 100644 --- a/pkg/upgrade/sources_test.go +++ b/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) }) diff --git a/pkg/vcs/vcs_test.go b/pkg/vcs/vcs_test.go index ab073e10..53e87463 100644 --- a/pkg/vcs/vcs_test.go +++ b/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) diff --git a/print.go b/print.go index 341a7796..9f9ae7e7 100644 --- a/print.go +++ b/print.go @@ -105,7 +105,7 @@ func printUpdateList(ctx context.Context, cfg *settings.Configuration, cmdArgs * quietMode := cmdArgs.ExistsArg("q", "quiet") // TODO: handle quiet mode in a better way - logger := text.NewLogger(io.Discard, os.Stdin, cfg.Debug, "update-list") + logger := text.NewLogger(io.Discard, os.Stderr, os.Stdin, cfg.Debug, "update-list") dbExecutor.SetLogger(logger.Child("db")) settings.NoConfirm = true diff --git a/sync_test.go b/sync_test.go index 52f7f9e3..3c36806a 100644 --- a/sync_test.go +++ b/sync_test.go @@ -107,7 +107,7 @@ func TestSyncUpgrade(t *testing.T) { NewInstallEngine: true, RemoveMake: "no", Runtime: &settings.Runtime{ - Logger: text.NewLogger(io.Discard, strings.NewReader("\n"), true, "test"), + Logger: text.NewLogger(io.Discard, os.Stderr, strings.NewReader("\n"), true, "test"), CmdBuilder: cmdBuilder, VCSStore: &vcs.Mock{}, AURCache: &mockaur.MockAUR{ @@ -221,7 +221,7 @@ func TestSyncUpgrade_IgnoreAll(t *testing.T) { NewInstallEngine: true, RemoveMake: "no", Runtime: &settings.Runtime{ - Logger: text.NewLogger(io.Discard, strings.NewReader("1\n"), true, "test"), + Logger: text.NewLogger(io.Discard, os.Stderr, strings.NewReader("1\n"), true, "test"), CmdBuilder: cmdBuilder, VCSStore: &vcs.Mock{}, AURCache: &mockaur.MockAUR{ @@ -352,7 +352,7 @@ func TestSyncUpgrade_IgnoreOne(t *testing.T) { NewInstallEngine: true, RemoveMake: "no", Runtime: &settings.Runtime{ - Logger: text.NewLogger(io.Discard, strings.NewReader("1\n"), true, "test"), + Logger: text.NewLogger(io.Discard, os.Stderr, strings.NewReader("1\n"), true, "test"), CmdBuilder: cmdBuilder, VCSStore: &vcs.Mock{}, AURCache: &mockaur.MockAUR{ @@ -517,7 +517,7 @@ pkgname = python-vosk RemoveMake: "no", BuildDir: tmpDir, Runtime: &settings.Runtime{ - Logger: text.NewLogger(io.Discard, strings.NewReader("\n\n\n\n"), true, "test"), + Logger: text.NewLogger(io.Discard, os.Stderr, strings.NewReader("\n\n\n\n"), true, "test"), CmdBuilder: cmdBuilder, VCSStore: &vcs.Mock{}, AURCache: &mockaur.MockAUR{ @@ -702,7 +702,7 @@ func TestSyncUpgrade_NoCombinedUpgrade(t *testing.T) { RemoveMake: "no", CombinedUpgrade: false, Runtime: &settings.Runtime{ - Logger: text.NewLogger(io.Discard, strings.NewReader("1\n"), true, "test"), + Logger: text.NewLogger(io.Discard, os.Stderr, strings.NewReader("1\n"), true, "test"), CmdBuilder: cmdBuilder, VCSStore: &vcs.Mock{}, AURCache: &mockaur.MockAUR{