From a4f9060903da76801b2e6a905418a62a7f6b6419 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 1 Jun 2023 18:02:57 +0200 Subject: [PATCH] fix golangci-lint issues golangci-lint v1.52.2 in its default configuration pointed out several issues that are worth fixing: internal/verbosity/verbosity_test.go:26:18: Error return value of `vs.verbosity.Set` is not checked (errcheck) vs.verbosity.Set("2") ^ internal/verbosity/verbosity_test.go:41:16: Error return value of `vs.vmodule.Set` is not checked (errcheck) vs.vmodule.Set("verbosity_test=2") ^ internal/verbosity/verbosity_test.go:84:16: Error return value of `vs.vmodule.Set` is not checked (errcheck) vs.vmodule.Set(pat) ^ internal/verbosity/verbosity.go:291:2: S1017: should replace this if statement with an unconditional strings.TrimSuffix (gosimple) if strings.HasSuffix(file, ".go") { ^ textlogger/options.go:140:19: Error return value of `(flag.Value).Set` is not checked (errcheck) c.Verbosity().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) ^ textlogger/textlogger.go:137:26: Error return value of `l.config.co.output.Write` is not checked (errcheck) l.config.co.output.Write(b.Bytes()) ^ textlogger/textlogger.go:141:26: Error return value of `l.config.co.output.Write` is not checked (errcheck) l.config.co.output.Write(data) ^ ktesting/options.go:163:18: Error return value of `(flag.Value).Set` is not checked (errcheck) c.vstate.V().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) ^ klogr/calldepth-test/call_depth_main_test.go:21:22: Error return value of `flag.CommandLine.Set` is not checked (errcheck) flag.CommandLine.Set("v", "10") ^ klogr/calldepth-test/call_depth_main_test.go:22:22: Error return value of `flag.CommandLine.Set` is not checked (errcheck) flag.CommandLine.Set("skip_headers", "false") ^ klogr/calldepth-test/call_depth_main_test.go:23:22: Error return value of `flag.CommandLine.Set` is not checked (errcheck) flag.CommandLine.Set("logtostderr", "false") ^ klog.go:903:34: Error return value of `(io.Writer).Write` is not checked (errcheck) l.file[severity.InfoLog].Write(data) ^ klog.go:913:20: Error return value of `(io.Writer).Write` is not checked (errcheck) l.file[s].Write(data) ^ klog.go:917:37: Error return value of `(io.Writer).Write` is not checked (errcheck) l.file[severity.FatalLog].Write(data) ^ klog.go:952:12: Error return value of `f.Write` is not checked (errcheck) f.Write(trace) ^ klog.go:1208:13: Error return value of `file.Sync` is not checked (errcheck) file.Sync() // ignore error ^ klog_file.go:113:14: Error return value of `os.Symlink` is not checked (errcheck) os.Symlink(name, symlink) // ignore err ^ klog_test.go:328:23: Error return value of `logging.verbosity.Set` is not checked (errcheck) logging.verbosity.Set("2") ^ klog_test.go:343:21: Error return value of `logging.vmodule.Set` is not checked (errcheck) logging.vmodule.Set("klog_test=2") ^ klog_test.go:367:21: Error return value of `logging.vmodule.Set` is not checked (errcheck) logging.vmodule.Set("notthisfile=2") ^ klog_test.go:459:21: Error return value of `logging.vmodule.Set` is not checked (errcheck) logging.vmodule.Set(pat) ^ klog_test.go:807:23: Error return value of `logging.verbosity.Set` is not checked (errcheck) logging.verbosity.Set("0") ^ klog_test.go:878:9: Error return value of `fs1.Set` is not checked (errcheck) fs1.Set("log_dir", "/test1") ^ klog_test.go:879:9: Error return value of `fs1.Set` is not checked (errcheck) fs1.Set("log_file_max_size", "1") ^ klog_test.go:885:9: Error return value of `fs2.Set` is not checked (errcheck) fs2.Set("log_file_max_size", "2048") ^ klog_test.go:1173:23: Error return value of `logging.verbosity.Set` is not checked (errcheck) logging.verbosity.Set("2") ^ klogr.go:35:2: field `level` is unused (unused) level int ^ klog.go:1287:2: S1017: should replace this if statement with an unconditional strings.TrimSuffix (gosimple) if strings.HasSuffix(file, ".go") { ^ klog.go:521:2: S1001: should use copy(to, from) instead of a loop (gosimple) for i := range s.vmodule.filter { ^ klog_test.go:79:2: S1001: should copy arrays using assignment instead of using a loop (gosimple) for i, w := range writers { ^ klogr/klogr.go:118:16: Error return value of `encoder.Encode` is not checked (errcheck) encoder.Encode(value) ^ klogr/klogr_test.go:208:8: Error return value of `fs.Set` is not checked (errcheck) fs.Set("skip_headers", "true") ^ klogr/klogr.go:119:27: S1030: should use buffer.String() instead of string(buffer.Bytes()) (gosimple) return strings.TrimSpace(string(buffer.Bytes())) ^ ktesting/testinglogger_test.go:176:8: Error return value of `fs.Set` is not checked (errcheck) fs.Set("alsologtostderr", "false") ^ ktesting/testinglogger_test.go:177:8: Error return value of `fs.Set` is not checked (errcheck) fs.Set("logtostderr", "false") ^ internal/buffer/buffer.go:40:2: field `next` is unused (unused) next *Buffer ^ internal/clock/clock.go:122:9: SA1015: using time.Tick leaks the underlying ticker, consider using it only in endless functions, tests and the main package, and use time.NewTicker here (staticcheck) return time.Tick(d) ^ exit_test.go:31:16: Error return value of `flag.Set` is not checked (errcheck) defer flag.Set("skip_headers", "false") ^ format_test.go:46:5: S1003: should use strings.Contains(str, "kind is config") instead (gosimple) if strings.Index(str, "kind is config") >= 0 { ^ --- examples/benchmarks/benchmarks_test.go | 11 +++---- examples/coexist_glog/coexist_glog.go | 5 ++-- examples/flushing/flushing_test.go | 6 ++-- examples/go.mod | 2 +- examples/klogr/main.go | 3 +- examples/log_file/usage_log_file.go | 5 ++-- examples/set_output/usage_set_output.go | 6 ++-- .../structured_logging/structured_logging.go | 5 ++-- examples/util/require/require.go | 7 +++++ exit_test.go | 13 ++++---- format_test.go | 2 +- internal/buffer/buffer.go | 3 +- internal/clock/clock.go | 21 ++----------- internal/clock/testing/fake_clock.go | 7 ----- internal/test/require/require.go | 26 ++++++++++++++++ internal/verbosity/verbosity.go | 4 +-- internal/verbosity/verbosity_test.go | 8 +++-- klog.go | 30 ++++++++----------- klog_file.go | 4 +-- klog_test.go | 23 +++++++------- klogr.go | 1 - klogr/calldepth-test/call_depth_main_test.go | 11 +++---- klogr/klogr.go | 6 ++-- klogr/klogr_test.go | 3 +- ktesting/options.go | 3 +- ktesting/testinglogger_test.go | 9 +++--- textlogger/options.go | 3 +- textlogger/textlogger.go | 4 +-- 28 files changed, 126 insertions(+), 105 deletions(-) create mode 100644 examples/util/require/require.go create mode 100644 internal/test/require/require.go diff --git a/examples/benchmarks/benchmarks_test.go b/examples/benchmarks/benchmarks_test.go index a343efe08..ecb57ca9b 100644 --- a/examples/benchmarks/benchmarks_test.go +++ b/examples/benchmarks/benchmarks_test.go @@ -27,6 +27,7 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" + "k8s.io/klog/examples/util/require" "k8s.io/klog/v2" ) @@ -38,11 +39,11 @@ func init() { // klog gets configured so that it writes to a single output file that // will be set during tests with SetOutput. klog.InitFlags(nil) - flag.Set("v", fmt.Sprintf("%d", verbosityThreshold)) - flag.Set("log_file", "/dev/null") - flag.Set("logtostderr", "false") - flag.Set("alsologtostderr", "false") - flag.Set("stderrthreshold", "10") + require.NoError(flag.Set("v", fmt.Sprintf("%d", verbosityThreshold))) + require.NoError(flag.Set("log_file", "/dev/null")) + require.NoError(flag.Set("logtostderr", "false")) + require.NoError(flag.Set("alsologtostderr", "false")) + require.NoError(flag.Set("stderrthreshold", "10")) } type testcase struct { diff --git a/examples/coexist_glog/coexist_glog.go b/examples/coexist_glog/coexist_glog.go index 960a9b931..9f3fd057f 100644 --- a/examples/coexist_glog/coexist_glog.go +++ b/examples/coexist_glog/coexist_glog.go @@ -4,11 +4,12 @@ import ( "flag" "github.com/golang/glog" + "k8s.io/klog/examples/util/require" "k8s.io/klog/v2" ) func main() { - flag.Set("alsologtostderr", "true") + require.NoError(flag.Set("alsologtostderr", "true")) flag.Parse() klogFlags := flag.NewFlagSet("klog", flag.ExitOnError) @@ -19,7 +20,7 @@ func main() { f2 := klogFlags.Lookup(f1.Name) if f2 != nil { value := f1.Value.String() - f2.Value.Set(value) + require.NoError(f2.Value.Set(value)) } }) diff --git a/examples/flushing/flushing_test.go b/examples/flushing/flushing_test.go index 3d46fbcab..18a4a4572 100644 --- a/examples/flushing/flushing_test.go +++ b/examples/flushing/flushing_test.go @@ -5,6 +5,8 @@ import ( "testing" "go.uber.org/goleak" + + "k8s.io/klog/examples/util/require" "k8s.io/klog/v2" ) @@ -13,8 +15,8 @@ func main() { // By default klog writes to stderr. Setting logtostderr to false makes klog // write to a log file. - flag.Set("logtostderr", "false") - flag.Set("log_file", "myfile.log") + require.NoError(flag.Set("logtostderr", "false")) + require.NoError(flag.Set("log_file", "myfile.log")) flag.Parse() // Info writes the first log message. When the first log file is created, diff --git a/examples/go.mod b/examples/go.mod index b7750acfb..4fee663f1 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -1,4 +1,4 @@ -module example +module k8s.io/klog/examples go 1.13 diff --git a/examples/klogr/main.go b/examples/klogr/main.go index 5de2b5f52..db5539aa5 100644 --- a/examples/klogr/main.go +++ b/examples/klogr/main.go @@ -3,6 +3,7 @@ package main import ( "flag" + "k8s.io/klog/examples/util/require" "k8s.io/klog/v2" "k8s.io/klog/v2/klogr" ) @@ -17,7 +18,7 @@ func (e myError) Error() string { func main() { klog.InitFlags(nil) - flag.Set("v", "3") + require.NoError(flag.Set("v", "3")) flag.Parse() log := klogr.New().WithName("MyName").WithValues("user", "you") log.Info("hello", "val1", 1, "val2", map[string]int{"k": 1}) diff --git a/examples/log_file/usage_log_file.go b/examples/log_file/usage_log_file.go index bff48e043..4badabc6f 100644 --- a/examples/log_file/usage_log_file.go +++ b/examples/log_file/usage_log_file.go @@ -3,6 +3,7 @@ package main import ( "flag" + "k8s.io/klog/examples/util/require" "k8s.io/klog/v2" ) @@ -10,8 +11,8 @@ func main() { klog.InitFlags(nil) // By default klog writes to stderr. Setting logtostderr to false makes klog // write to a log file. - flag.Set("logtostderr", "false") - flag.Set("log_file", "myfile.log") + require.NoError(flag.Set("logtostderr", "false")) + require.NoError(flag.Set("log_file", "myfile.log")) flag.Parse() klog.Info("nice to meet you") klog.Flush() diff --git a/examples/set_output/usage_set_output.go b/examples/set_output/usage_set_output.go index 3cdd3884f..99ad8d01a 100644 --- a/examples/set_output/usage_set_output.go +++ b/examples/set_output/usage_set_output.go @@ -4,13 +4,15 @@ import ( "bytes" "flag" "fmt" + + "k8s.io/klog/examples/util/require" "k8s.io/klog/v2" ) func main() { klog.InitFlags(nil) - flag.Set("logtostderr", "false") - flag.Set("alsologtostderr", "false") + require.NoError(flag.Set("logtostderr", "false")) + require.NoError(flag.Set("alsologtostderr", "false")) flag.Parse() buf := new(bytes.Buffer) diff --git a/examples/structured_logging/structured_logging.go b/examples/structured_logging/structured_logging.go index 6e80bd936..b2ae83476 100644 --- a/examples/structured_logging/structured_logging.go +++ b/examples/structured_logging/structured_logging.go @@ -25,8 +25,9 @@ func main() { flag.Parse() someData := MyStruct{ - Name: "hello", - Data: "world", + Name: "hello", + Data: "world", + internal: 42, } longData := MyStruct{ diff --git a/examples/util/require/require.go b/examples/util/require/require.go new file mode 100644 index 000000000..0c458553b --- /dev/null +++ b/examples/util/require/require.go @@ -0,0 +1,7 @@ +package require + +func NoError(err error) { + if err != nil { + panic(err) + } +} diff --git a/exit_test.go b/exit_test.go index e295eb8e8..8c0a03759 100644 --- a/exit_test.go +++ b/exit_test.go @@ -27,12 +27,15 @@ func ExampleFlushAndExit() { var fs flag.FlagSet klog.InitFlags(&fs) - fs.Set("skip_headers", "true") - defer flag.Set("skip_headers", "false") - fs.Set("logtostderr", "false") - defer fs.Set("logtostderr", "true") + state := klog.CaptureState() + defer state.Restore() + if err := fs.Set("skip_headers", "true"); err != nil { + panic(err) + } + if err := fs.Set("logtostderr", "false"); err != nil { + panic(err) + } klog.SetOutput(os.Stdout) - defer klog.SetOutput(nil) klog.OsExit = func(exitCode int) { fmt.Printf("os.Exit(%d)\n", exitCode) } diff --git a/format_test.go b/format_test.go index cd1914581..490c96630 100644 --- a/format_test.go +++ b/format_test.go @@ -43,7 +43,7 @@ func TestFormat(t *testing.T) { `, klog.Format(obj).(fmt.Stringer).String(), "Format(config).String()") // fmt.Sprintf would call String if it was available. str := fmt.Sprintf("%s", klog.Format(obj).(logr.Marshaler).MarshalLog()) - if strings.Index(str, "kind is config") >= 0 { + if strings.Contains(str, "kind is config") { t.Errorf("fmt.Sprintf called TypeMeta.String for klog.Format(obj).MarshalLog():\n%s", str) } diff --git a/internal/buffer/buffer.go b/internal/buffer/buffer.go index f325ded5e..475e1bf3d 100644 --- a/internal/buffer/buffer.go +++ b/internal/buffer/buffer.go @@ -36,8 +36,7 @@ var ( // use. It also provides some helper methods for output formatting. type Buffer struct { bytes.Buffer - Tmp [64]byte // temporary byte array for creating headers. - next *Buffer + Tmp [64]byte // temporary byte array for creating headers. } var buffers = sync.Pool{ diff --git a/internal/clock/clock.go b/internal/clock/clock.go index b8b6af5c8..cc11bb480 100644 --- a/internal/clock/clock.go +++ b/internal/clock/clock.go @@ -39,16 +39,6 @@ type Clock interface { // Sleep sleeps for the provided duration d. // Consider making the sleep interruptible by using 'select' on a context channel and a timer channel. Sleep(d time.Duration) - // Tick returns the channel of a new Ticker. - // This method does not allow to free/GC the backing ticker. Use - // NewTicker from WithTicker instead. - Tick(d time.Duration) <-chan time.Time -} - -// WithTicker allows for injecting fake or real clocks into code that -// needs to do arbitrary things based on time. -type WithTicker interface { - Clock // NewTicker returns a new Ticker. NewTicker(time.Duration) Ticker } @@ -66,7 +56,7 @@ type WithDelayedExecution interface { // WithTickerAndDelayedExecution allows for injecting fake or real clocks // into code that needs Ticker and AfterFunc functionality type WithTickerAndDelayedExecution interface { - WithTicker + Clock // AfterFunc executes f in its own goroutine after waiting // for d duration and returns a Timer whose channel can be // closed by calling Stop() on the Timer. @@ -79,7 +69,7 @@ type Ticker interface { Stop() } -var _ = WithTicker(RealClock{}) +var _ Clock = RealClock{} // RealClock really calls time.Now() type RealClock struct{} @@ -115,13 +105,6 @@ func (RealClock) AfterFunc(d time.Duration, f func()) Timer { } } -// Tick is the same as time.Tick(d) -// This method does not allow to free/GC the backing ticker. Use -// NewTicker instead. -func (RealClock) Tick(d time.Duration) <-chan time.Time { - return time.Tick(d) -} - // NewTicker returns a new Ticker. func (RealClock) NewTicker(d time.Duration) Ticker { return &realTicker{ diff --git a/internal/clock/testing/fake_clock.go b/internal/clock/testing/fake_clock.go index a91114a58..392f81e04 100644 --- a/internal/clock/testing/fake_clock.go +++ b/internal/clock/testing/fake_clock.go @@ -25,7 +25,6 @@ import ( var ( _ = clock.PassiveClock(&FakePassiveClock{}) - _ = clock.WithTicker(&FakeClock{}) _ = clock.Clock(&IntervalClock{}) ) @@ -275,12 +274,6 @@ func (*IntervalClock) AfterFunc(d time.Duration, f func()) clock.Timer { panic("IntervalClock doesn't implement AfterFunc") } -// Tick is unimplemented, will panic. -// TODO: make interval clock use FakeClock so this can be implemented. -func (*IntervalClock) Tick(d time.Duration) <-chan time.Time { - panic("IntervalClock doesn't implement Tick") -} - // NewTicker has no implementation yet and is omitted. // TODO: make interval clock use FakeClock so this can be implemented. func (*IntervalClock) NewTicker(d time.Duration) clock.Ticker { diff --git a/internal/test/require/require.go b/internal/test/require/require.go new file mode 100644 index 000000000..93a869260 --- /dev/null +++ b/internal/test/require/require.go @@ -0,0 +1,26 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package require + +import "testing" + +func NoError(tb testing.TB, err error) { + if err != nil { + tb.Helper() + tb.Fatalf("Unexpected error: %v", err) + } +} diff --git a/internal/verbosity/verbosity.go b/internal/verbosity/verbosity.go index 309cdfaa9..40ec27d87 100644 --- a/internal/verbosity/verbosity.go +++ b/internal/verbosity/verbosity.go @@ -288,9 +288,7 @@ func (vs *VState) setV(pc uintptr) Level { fn := runtime.FuncForPC(pc) file, _ := fn.FileLine(pc) // The file is something like /a/b/c/d.go. We want just the d. - if strings.HasSuffix(file, ".go") { - file = file[:len(file)-3] - } + file = strings.TrimSuffix(file, ".go") if slash := strings.LastIndex(file, "/"); slash >= 0 { file = file[slash+1:] } diff --git a/internal/verbosity/verbosity_test.go b/internal/verbosity/verbosity_test.go index 766147b27..5735d486c 100644 --- a/internal/verbosity/verbosity_test.go +++ b/internal/verbosity/verbosity_test.go @@ -19,11 +19,13 @@ package verbosity import ( "testing" + + "k8s.io/klog/v2/internal/test/require" ) func TestV(t *testing.T) { vs := New() - vs.verbosity.Set("2") + require.NoError(t, vs.verbosity.Set("2")) depth := 0 if !vs.Enabled(1, depth) { t.Error("not enabled for 1") @@ -38,7 +40,7 @@ func TestV(t *testing.T) { func TestVmoduleOn(t *testing.T) { vs := New() - vs.vmodule.Set("verbosity_test=2") + require.NoError(t, vs.vmodule.Set("verbosity_test=2")) depth := 0 if !vs.Enabled(1, depth) { t.Error("not enabled for 1") @@ -81,7 +83,7 @@ var vGlobs = map[string]bool{ // Test that vmodule globbing works as advertised. func testVmoduleGlob(pat string, match bool, t *testing.T) { vs := New() - vs.vmodule.Set(pat) + require.NoError(t, vs.vmodule.Set(pat)) depth := 0 actual := vs.Enabled(2, depth) if actual != match { diff --git a/klog.go b/klog.go index c18576e3e..e50b1f2fa 100644 --- a/klog.go +++ b/klog.go @@ -518,9 +518,7 @@ type settings struct { func (s settings) deepCopy() settings { // vmodule is a slice and would be shared, so we have copy it. filter := make([]modulePat, len(s.vmodule.filter)) - for i := range s.vmodule.filter { - filter[i] = s.vmodule.filter[i] - } + copy(filter, s.vmodule.filter) s.vmodule.filter = filter if s.logger != nil { @@ -900,7 +898,7 @@ func (l *loggingT) output(s severity.Severity, logger *logWriter, buf *buffer.Bu l.exit(err) } } - l.file[severity.InfoLog].Write(data) + _, _ = l.file[severity.InfoLog].Write(data) } else { if l.file[s] == nil { if err := l.createFiles(s); err != nil { @@ -910,20 +908,20 @@ func (l *loggingT) output(s severity.Severity, logger *logWriter, buf *buffer.Bu } if l.oneOutput { - l.file[s].Write(data) + _, _ = l.file[s].Write(data) } else { switch s { case severity.FatalLog: - l.file[severity.FatalLog].Write(data) + _, _ = l.file[severity.FatalLog].Write(data) fallthrough case severity.ErrorLog: - l.file[severity.ErrorLog].Write(data) + _, _ = l.file[severity.ErrorLog].Write(data) fallthrough case severity.WarningLog: - l.file[severity.WarningLog].Write(data) + _, _ = l.file[severity.WarningLog].Write(data) fallthrough case severity.InfoLog: - l.file[severity.InfoLog].Write(data) + _, _ = l.file[severity.InfoLog].Write(data) } } } @@ -949,7 +947,7 @@ func (l *loggingT) output(s severity.Severity, logger *logWriter, buf *buffer.Bu logExitFunc = func(error) {} // If we get a write error, we'll still exit below. for log := severity.FatalLog; log >= severity.InfoLog; log-- { if f := l.file[log]; f != nil { // Can be nil if -logtostderr is set. - f.Write(trace) + _, _ = f.Write(trace) } } l.mu.Unlock() @@ -1105,7 +1103,7 @@ const flushInterval = 5 * time.Second // flushDaemon periodically flushes the log file buffers. type flushDaemon struct { mu sync.Mutex - clock clock.WithTicker + clock clock.Clock flush func() stopC chan struct{} stopDone chan struct{} @@ -1113,7 +1111,7 @@ type flushDaemon struct { // newFlushDaemon returns a new flushDaemon. If the passed clock is nil, a // clock.RealClock is used. -func newFlushDaemon(flush func(), tickClock clock.WithTicker) *flushDaemon { +func newFlushDaemon(flush func(), tickClock clock.Clock) *flushDaemon { if tickClock == nil { tickClock = clock.RealClock{} } @@ -1204,8 +1202,8 @@ func (l *loggingT) flushAll() { for s := severity.FatalLog; s >= severity.InfoLog; s-- { file := l.file[s] if file != nil { - file.Flush() // ignore error - file.Sync() // ignore error + _ = file.Flush() // ignore error + _ = file.Sync() // ignore error } } if logging.loggerOptions.flush != nil { @@ -1284,9 +1282,7 @@ func (l *loggingT) setV(pc uintptr) Level { fn := runtime.FuncForPC(pc) file, _ := fn.FileLine(pc) // The file is something like /a/b/c/d.go. We want just the d. - if strings.HasSuffix(file, ".go") { - file = file[:len(file)-3] - } + file = strings.TrimSuffix(file, ".go") if slash := strings.LastIndex(file, "/"); slash >= 0 { file = file[slash+1:] } diff --git a/klog_file.go b/klog_file.go index 1025d644f..8bee16204 100644 --- a/klog_file.go +++ b/klog_file.go @@ -109,8 +109,8 @@ func create(tag string, t time.Time, startup bool) (f *os.File, filename string, f, err := openOrCreate(fname, startup) if err == nil { symlink := filepath.Join(dir, link) - os.Remove(symlink) // ignore err - os.Symlink(name, symlink) // ignore err + _ = os.Remove(symlink) // ignore err + _ = os.Symlink(name, symlink) // ignore err return f, fname, nil } lastErr = err diff --git a/klog_test.go b/klog_test.go index 7b6ebdb6f..a9ddc5a77 100644 --- a/klog_test.go +++ b/klog_test.go @@ -40,6 +40,7 @@ import ( testingclock "k8s.io/klog/v2/internal/clock/testing" "k8s.io/klog/v2/internal/severity" "k8s.io/klog/v2/internal/test" + "k8s.io/klog/v2/internal/test/require" ) // TODO: This test package should be refactored so that tests cannot @@ -76,9 +77,7 @@ func (l *loggingT) swap(writers [severity.NumSeverity]flushSyncWriter) (old [sev l.mu.Lock() defer l.mu.Unlock() old = l.file - for i, w := range writers { - logging.file[i] = w - } + logging.file = writers return } @@ -325,7 +324,7 @@ func TestV(t *testing.T) { defer CaptureState().Restore() setFlags() defer logging.swap(logging.newBuffers()) - logging.verbosity.Set("2") + require.NoError(t, logging.verbosity.Set("2")) V(2).Info("test") if !contains(severity.InfoLog, "I", t) { t.Errorf("Info has wrong character: %q", contents(severity.InfoLog)) @@ -340,7 +339,7 @@ func TestVmoduleOn(t *testing.T) { defer CaptureState().Restore() setFlags() defer logging.swap(logging.newBuffers()) - logging.vmodule.Set("klog_test=2") + require.NoError(t, logging.vmodule.Set("klog_test=2")) if !V(1).Enabled() { t.Error("V not enabled for 1") } @@ -364,7 +363,7 @@ func TestVmoduleOff(t *testing.T) { defer CaptureState().Restore() setFlags() defer logging.swap(logging.newBuffers()) - logging.vmodule.Set("notthisfile=2") + require.NoError(t, logging.vmodule.Set("notthisfile=2")) for i := 1; i <= 3; i++ { if V(Level(i)).Enabled() { t.Errorf("V enabled for %d", i) @@ -456,7 +455,7 @@ func testVmoduleGlob(pat string, match bool, t *testing.T) { defer CaptureState().Restore() setFlags() defer logging.swap(logging.newBuffers()) - logging.vmodule.Set(pat) + require.NoError(t, logging.vmodule.Set(pat)) if V(2).Enabled() != match { t.Errorf("incorrect match for %q: got %#v expected %#v", pat, V(2), match) } @@ -804,7 +803,7 @@ func BenchmarkLogs(b *testing.B) { } defer os.Remove(testFile.Name()) - logging.verbosity.Set("0") + require.NoError(b, logging.verbosity.Set("0")) logging.toStderr = false logging.alsoToStderr = false logging.stderrThreshold = severityValue{ @@ -875,14 +874,14 @@ func TestInitFlags(t *testing.T) { fs1 := flag.NewFlagSet("test1", flag.PanicOnError) InitFlags(fs1) - fs1.Set("log_dir", "/test1") - fs1.Set("log_file_max_size", "1") + require.NoError(t, fs1.Set("log_dir", "/test1")) + require.NoError(t, fs1.Set("log_file_max_size", "1")) fs2 := flag.NewFlagSet("test2", flag.PanicOnError) InitFlags(fs2) if logging.logDir != "/test1" { t.Fatalf("Expected log_dir to be %q, got %q", "/test1", logging.logDir) } - fs2.Set("log_file_max_size", "2048") + require.NoError(t, fs2.Set("log_file_max_size", "2048")) if logging.logFileMaxSizeMB != 2048 { t.Fatal("Expected log_file_max_size to be 2048") } @@ -1170,7 +1169,7 @@ second value line`}, }, } - logging.verbosity.Set("2") + require.NoError(t, logging.verbosity.Set("2")) for l := Level(0); l < Level(4); l++ { for _, data := range testDataInfo { diff --git a/klogr.go b/klogr.go index 15de00e21..784f7de13 100644 --- a/klogr.go +++ b/klogr.go @@ -32,7 +32,6 @@ func NewKlogr() Logger { // klogger is a subset of klogr/klogr.go. It had to be copied to break an // import cycle (klogr wants to use klog, and klog wants to use klogr). type klogger struct { - level int callDepth int prefix string values []interface{} diff --git a/klogr/calldepth-test/call_depth_main_test.go b/klogr/calldepth-test/call_depth_main_test.go index 6d7e59f81..83fe4309e 100644 --- a/klogr/calldepth-test/call_depth_main_test.go +++ b/klogr/calldepth-test/call_depth_main_test.go @@ -13,16 +13,17 @@ import ( "testing" "k8s.io/klog/v2" + "k8s.io/klog/v2/internal/test/require" "k8s.io/klog/v2/klogr" ) func TestCallDepth(t *testing.T) { klog.InitFlags(nil) - flag.CommandLine.Set("v", "10") - flag.CommandLine.Set("skip_headers", "false") - flag.CommandLine.Set("logtostderr", "false") - flag.CommandLine.Set("alsologtostderr", "false") - flag.CommandLine.Set("stderrthreshold", "10") + require.NoError(t, flag.CommandLine.Set("v", "10")) + require.NoError(t, flag.CommandLine.Set("skip_headers", "false")) + require.NoError(t, flag.CommandLine.Set("logtostderr", "false")) + require.NoError(t, flag.CommandLine.Set("alsologtostderr", "false")) + require.NoError(t, flag.CommandLine.Set("stderrthreshold", "10")) flag.Parse() t.Run("call-depth", func(t *testing.T) { diff --git a/klogr/klogr.go b/klogr/klogr.go index db5b5c789..d80e44fdc 100644 --- a/klogr/klogr.go +++ b/klogr/klogr.go @@ -115,8 +115,10 @@ func pretty(value interface{}) string { buffer := &bytes.Buffer{} encoder := json.NewEncoder(buffer) encoder.SetEscapeHTML(false) - encoder.Encode(value) - return strings.TrimSpace(string(buffer.Bytes())) + if err := encoder.Encode(value); err != nil { + return fmt.Sprintf("<>", err) + } + return strings.TrimSpace(buffer.String()) } func (l *klogger) Info(level int, msg string, kvList ...interface{}) { diff --git a/klogr/klogr_test.go b/klogr/klogr_test.go index 53b9e450d..621017d0b 100644 --- a/klogr/klogr_test.go +++ b/klogr/klogr_test.go @@ -8,6 +8,7 @@ import ( "testing" "k8s.io/klog/v2" + "k8s.io/klog/v2/internal/test/require" "k8s.io/klog/v2/test" "github.com/go-logr/logr" @@ -205,7 +206,7 @@ func testOutput(t *testing.T, format string) { func TestOutput(t *testing.T) { fs := test.InitKlog(t) - fs.Set("skip_headers", "true") + require.NoError(t, fs.Set("skip_headers", "true")) formats := []string{ formatNew, diff --git a/ktesting/options.go b/ktesting/options.go index d039c40b0..a1b4f6af0 100644 --- a/ktesting/options.go +++ b/ktesting/options.go @@ -160,7 +160,8 @@ func NewConfig(opts ...ConfigOption) *Config { } c.vstate = verbosity.New() - c.vstate.V().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) + // Cannot fail for this input. + _ = c.vstate.V().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) return c } diff --git a/ktesting/testinglogger_test.go b/ktesting/testinglogger_test.go index 6de1834d4..1ec709b50 100644 --- a/ktesting/testinglogger_test.go +++ b/ktesting/testinglogger_test.go @@ -18,6 +18,7 @@ import ( "testing" "k8s.io/klog/v2" + "k8s.io/klog/v2/internal/test/require" "k8s.io/klog/v2/ktesting" ) @@ -173,10 +174,10 @@ func TestStop(t *testing.T) { var output bytes.Buffer var fs flag.FlagSet klog.InitFlags(&fs) - fs.Set("alsologtostderr", "false") - fs.Set("logtostderr", "false") - fs.Set("stderrthreshold", "FATAL") - fs.Set("one_output", "true") + require.NoError(t, fs.Set("alsologtostderr", "false")) + require.NoError(t, fs.Set("logtostderr", "false")) + require.NoError(t, fs.Set("stderrthreshold", "FATAL")) + require.NoError(t, fs.Set("one_output", "true")) klog.SetOutput(&output) var logger klog.Logger diff --git a/textlogger/options.go b/textlogger/options.go index db4cbebdc..83eeef407 100644 --- a/textlogger/options.go +++ b/textlogger/options.go @@ -137,7 +137,8 @@ func NewConfig(opts ...ConfigOption) *Config { opt(&c.co) } - c.Verbosity().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) + // Cannot fail for this input. + _ = c.Verbosity().Set(strconv.FormatInt(int64(c.co.verbosityDefault), 10)) return c } diff --git a/textlogger/textlogger.go b/textlogger/textlogger.go index 3c0e079ad..857c76922 100644 --- a/textlogger/textlogger.go +++ b/textlogger/textlogger.go @@ -134,11 +134,11 @@ func (l *tlogger) print(err error, s severity.Severity, msg string, kvList []int if b.Len() == 0 || b.Bytes()[b.Len()-1] != '\n' { b.WriteByte('\n') } - l.config.co.output.Write(b.Bytes()) + _, _ = l.config.co.output.Write(b.Bytes()) } func (l *tlogger) WriteKlogBuffer(data []byte) { - l.config.co.output.Write(data) + _, _ = l.config.co.output.Write(data) } // WithName returns a new logr.Logger with the specified name appended. klogr