diff --git a/cmd/analyze.go b/cmd/analyze.go index e3aede9c..6a5f4f25 100644 --- a/cmd/analyze.go +++ b/cmd/analyze.go @@ -1,11 +1,11 @@ package cmd import ( - "log" "rare/cmd/helpers" "rare/pkg/aggregation" "rare/pkg/color" "rare/pkg/humanize" + "rare/pkg/logger" "rare/pkg/multiterm" "strconv" @@ -43,7 +43,7 @@ func parseStringSet(vals []string) []float64 { for idx, val := range vals { parsedVal, err := strconv.ParseFloat(val, 64) if err != nil { - log.Fatalf("%s is not a number: %v", val, err) + logger.Fatalf(helpers.ExitCodeInvalidUsage, "%s is not a number: %v", val, err) } ret[idx] = parsedVal } diff --git a/cmd/analyze_test.go b/cmd/analyze_test.go index 10261bc6..3cc17c2c 100644 --- a/cmd/analyze_test.go +++ b/cmd/analyze_test.go @@ -1,6 +1,9 @@ package cmd -import "testing" +import ( + "rare/cmd/helpers" + "testing" +) func TestAnalyze(t *testing.T) { testCommandSet(t, analyzeCommand(), @@ -8,3 +11,9 @@ func TestAnalyze(t *testing.T) { `-x -m (\d+) testdata/graph.txt`, ) } + +func TestAnalyzeParseFatals(t *testing.T) { + catchLogFatal(t, helpers.ExitCodeInvalidUsage, func() { + testCommand(analyzeCommand(), "--quantile bla testdata/graph.txt") + }) +} diff --git a/cmd/commands_test.go b/cmd/commands_test.go index 4bffe3dd..bc79cf25 100644 --- a/cmd/commands_test.go +++ b/cmd/commands_test.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "os" + "rare/pkg/logger" "rare/pkg/testutil" "testing" @@ -42,3 +43,21 @@ func testCommand(command *cli.Command, cmd string) error { return app.Run(commandArgs) } + +// Cause logger.fatal* to result in panic() for testability +func catchLogFatal(t *testing.T, expectsCode int, f func()) (code int) { + code = -1 + + oldExit := logger.OsExit + defer func() { + logger.OsExit = oldExit + }() + logger.OsExit = func(v int) { + code = v + panic("logger.osexit") + } + + assert.PanicsWithValue(t, "logger.osexit", f) + assert.Equal(t, expectsCode, code) + return +} diff --git a/cmd/helpers/exitCodes.go b/cmd/helpers/exitCodes.go index 35950671..10289b89 100644 --- a/cmd/helpers/exitCodes.go +++ b/cmd/helpers/exitCodes.go @@ -1,10 +1,6 @@ package helpers import ( - "rare/pkg/aggregation" - "rare/pkg/extractor" - "rare/pkg/extractor/batchers" - "github.com/urfave/cli/v2" ) @@ -13,7 +9,19 @@ const ( ExitCodeInvalidUsage = 2 ) -func DetermineErrorState(b *batchers.Batcher, e *extractor.Extractor, agg aggregation.Aggregator) error { +type ( + BatcherErrors interface { + ReadErrors() int + } + ExtractorSummary interface { + MatchedLines() uint64 + } + AggregationErrors interface { + ParseErrors() uint64 + } +) + +func DetermineErrorState(b BatcherErrors, e ExtractorSummary, agg AggregationErrors) error { if b.ReadErrors() > 0 { return cli.Exit("Read errors", ExitCodeInvalidUsage) } diff --git a/cmd/helpers/exitCodes_test.go b/cmd/helpers/exitCodes_test.go new file mode 100644 index 00000000..0cfdc53c --- /dev/null +++ b/cmd/helpers/exitCodes_test.go @@ -0,0 +1,37 @@ +package helpers + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +type mockExitState struct { + batchErr, aggErr, extSum int +} + +func (s *mockExitState) ReadErrors() int { + return s.batchErr +} + +func (s *mockExitState) ParseErrors() uint64 { + return uint64(s.aggErr) +} + +func (s *mockExitState) MatchedLines() uint64 { + return uint64(s.extSum) +} + +func TestDetermineErrorState(t *testing.T) { + s := mockExitState{0, 0, 1} + assert.NoError(t, DetermineErrorState(&s, &s, &s)) + + s = mockExitState{0, 0, 0} + assert.Error(t, DetermineErrorState(&s, &s, &s)) + + s = mockExitState{0, 1, 1} + assert.Error(t, DetermineErrorState(&s, &s, &s)) + + s = mockExitState{1, 0, 1} + assert.Error(t, DetermineErrorState(&s, &s, &s)) +} diff --git a/cmd/helpers/extractorBuilder.go b/cmd/helpers/extractorBuilder.go index 8a109601..eba4a7ba 100644 --- a/cmd/helpers/extractorBuilder.go +++ b/cmd/helpers/extractorBuilder.go @@ -36,23 +36,23 @@ func BuildBatcherFromArguments(c *cli.Context) *batchers.Batcher { ) if batchSize < 1 { - logger.Fatalf("Batch size must be >= 1, is %d", batchSize) + logger.Fatalf(ExitCodeInvalidUsage, "Batch size must be >= 1, is %d", batchSize) } if concurrentReaders < 1 { - logger.Fatalf("Must have at least 1 reader") + logger.Fatalf(ExitCodeInvalidUsage, "Must have at least 1 reader") } if followPoll && !follow { - logger.Fatalf("Follow (-f) must be enabled for --poll") + logger.Fatalf(ExitCodeInvalidUsage, "Follow (-f) must be enabled for --poll") } if followTail && !follow { - logger.Fatalf("Follow (-f) must be enabled for --tail") + logger.Fatalf(ExitCodeInvalidUsage, "Follow (-f) must be enabled for --tail") } fileglobs := c.Args().Slice() if len(fileglobs) == 0 || fileglobs[0] == "-" { // Read from stdin if gunzip { - logger.Fatalln("Cannot decompress (-z) with stdin") + logger.Fatalln(ExitCodeInvalidUsage, "Cannot decompress (-z) with stdin") } if follow { logger.Println("Cannot follow a stdin stream, not a file") @@ -88,14 +88,14 @@ func BuildExtractorFromArgumentsEx(c *cli.Context, batcher *batchers.Batcher, se if len(ignoreSlice) > 0 { ignoreExp, err := extractor.NewIgnoreExpressions(ignoreSlice...) if err != nil { - logger.Fatalln(err) + logger.Fatalln(ExitCodeInvalidUsage, err) } config.Ignore = ignoreExp } ret, err := extractor.New(batcher.BatchChan(), &config) if err != nil { - logger.Fatalln(err) + logger.Fatalln(ExitCodeInvalidUsage, err) } return ret } diff --git a/cmd/helpers/extractorBuilder_test.go b/cmd/helpers/extractorBuilder_test.go index 9ff60c4b..c0a776ad 100644 --- a/cmd/helpers/extractorBuilder_test.go +++ b/cmd/helpers/extractorBuilder_test.go @@ -1,6 +1,7 @@ package helpers import ( + "rare/pkg/testutil" "testing" "github.com/stretchr/testify/assert" @@ -48,8 +49,33 @@ func TestBuildingExtractorFromContext(t *testing.T) { cmd, } - app.Run([]string{"app", "test"}) - app.Run([]string{"app", "test", "-i", "{eq {0} abc}", "../testdata/log.txt"}) - app.Run([]string{"app", "test", "-f", "../testdata/log.txt"}) + runApp := func(args string) error { + return app.Run(append([]string{"app", "test"}, testutil.SplitQuotedString(args)...)) + } + + assert.NoError(t, runApp("")) + assert.NoError(t, runApp(`-I -i "{eq {0} abc}" ../testdata/log.txt`)) + assert.NoError(t, runApp(`-f ../testdata/log.txt`)) + testLogFatal(t, 2, func() { + runApp("--batch 0 ../testdata/log.txt") + }) + testLogFatal(t, 2, func() { + runApp("--readers 0 ../testdata/log.txt") + }) + testLogFatal(t, 2, func() { + runApp("--poll ../testdata/log.txt") + }) + testLogFatal(t, 2, func() { + runApp("--tail ../testdata/log.txt") + }) + testLogFatal(t, 2, func() { + runApp("-z -") + }) + testLogFatal(t, 2, func() { + runApp(`-m ".(" -`) + }) + testLogFatal(t, 2, func() { + runApp(`-i "{0" -`) + }) assert.Equal(t, 3, actionCalled) } diff --git a/cmd/helpers/scaler.go b/cmd/helpers/scaler.go index 8de82cda..dc12ed46 100644 --- a/cmd/helpers/scaler.go +++ b/cmd/helpers/scaler.go @@ -27,7 +27,7 @@ func BuildScaler(scalerName string) (termscaler.Scaler, error) { func BuildScalerOrFail(scalerName string) termscaler.Scaler { s, err := BuildScaler(scalerName) if err != nil { - logger.Fatal(err) + logger.Fatal(ExitCodeInvalidUsage, err) } return s } diff --git a/cmd/helpers/scaler_test.go b/cmd/helpers/scaler_test.go index 139f1303..6739aa3e 100644 --- a/cmd/helpers/scaler_test.go +++ b/cmd/helpers/scaler_test.go @@ -20,4 +20,7 @@ func TestBuildScaler(t *testing.T) { func TestBuildScalerOrFail(t *testing.T) { assert.NotNil(t, BuildScalerOrFail("")) assert.NotNil(t, BuildScalerOrFail("linear")) + testLogFatal(t, 2, func() { + BuildScalerOrFail("fake") + }) } diff --git a/cmd/helpers/sorting.go b/cmd/helpers/sorting.go index fe32fcca..3c866c3c 100644 --- a/cmd/helpers/sorting.go +++ b/cmd/helpers/sorting.go @@ -31,7 +31,7 @@ func DefaultSortFlagWithDefault(dflt string) *cli.StringFlag { func BuildSorterOrFail(fullName string) sorting.NameValueSorter { sorter, err := BuildSorter(fullName) if err != nil { - logger.Fatal(err) + logger.Fatal(ExitCodeInvalidUsage, err) } return sorter } diff --git a/cmd/helpers/sorting_test.go b/cmd/helpers/sorting_test.go index 82d2743a..61701296 100644 --- a/cmd/helpers/sorting_test.go +++ b/cmd/helpers/sorting_test.go @@ -13,6 +13,9 @@ func TestBuildSorter(t *testing.T) { assert.NotNil(t, BuildSorterOrFail("contextual")) assert.NotNil(t, BuildSorterOrFail("value")) assert.NotNil(t, BuildSorterOrFail("value:reverse")) + testLogFatal(t, 2, func() { + BuildSorterOrFail("fake") + }) } func TestOrderResults(t *testing.T) { diff --git a/cmd/helpers/util_test.go b/cmd/helpers/util_test.go new file mode 100644 index 00000000..567d8f07 --- /dev/null +++ b/cmd/helpers/util_test.go @@ -0,0 +1,25 @@ +package helpers + +import ( + "rare/pkg/logger" + "testing" + + "github.com/stretchr/testify/assert" +) + +func testLogFatal(t *testing.T, expectsCode int, f func()) (code int) { + code = -1 + + oldExit := logger.OsExit + defer func() { + logger.OsExit = oldExit + }() + logger.OsExit = func(v int) { + code = v + panic("logger.osexit") + } + + assert.PanicsWithValue(t, "logger.osexit", f) + assert.Equal(t, expectsCode, code) + return +} diff --git a/cmd/reduce.go b/cmd/reduce.go index b332d5ef..07efac3b 100644 --- a/cmd/reduce.go +++ b/cmd/reduce.go @@ -37,7 +37,7 @@ func reduceFunction(c *cli.Context) error { for _, group := range groupExpr { name, val := parseKeyValue(group) if err := aggr.AddGroupExpr(name, val); err != nil { - logger.Fatalf("Error compiling group expression %s: %s", group, err) + logger.Fatalf(helpers.ExitCodeInvalidUsage, "Error compiling group expression %s: %s", group, err) } } @@ -46,7 +46,7 @@ func reduceFunction(c *cli.Context) error { for _, expr := range accumExprs { name, initial, val := parseKeyValInitial(expr, defaultInitial) if err := aggr.AddDataExpr(name, val, initial); err != nil { - logger.Fatalf("Error compiling expression %s: %s", expr, err) + logger.Fatalf(helpers.ExitCodeInvalidUsage, "Error compiling expression %s: %s", expr, err) } else { if len(name) > maxKeylen { maxKeylen = len(name) @@ -61,7 +61,7 @@ func reduceFunction(c *cli.Context) error { } if sort != "" { if err := aggr.SetSort(sort); err != nil { - logger.Fatalf("Error setting sort: %s", err) + logger.Fatalf(helpers.ExitCodeInvalidUsage, "Error setting sort: %s", err) } } diff --git a/cmd/reduce_test.go b/cmd/reduce_test.go index d8ceba88..c560a2be 100644 --- a/cmd/reduce_test.go +++ b/cmd/reduce_test.go @@ -47,3 +47,15 @@ func TestParseKIV(t *testing.T) { assert.Equal(t, "1", i) assert.Equal(t, "efg", v) } + +func TestReduceFatals(t *testing.T) { + catchLogFatal(t, 2, func() { + testCommand(reduceCommand(), `-m (\d+) -g {0 -a {0} testdata/log.txt`) + }) + catchLogFatal(t, 2, func() { + testCommand(reduceCommand(), `-m (\d+) -g {0} -a {0 testdata/log.txt`) + }) + catchLogFatal(t, 2, func() { + testCommand(reduceCommand(), `-m (\d+) -g {0} -a {0} --sort {0 testdata/log.txt`) + }) +} diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 55254cb8..e0b48318 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -13,6 +13,9 @@ var logBuffer *bytes.Buffer const logPrefix = "[Log] " +// Allow overriding exit for unit tests +var OsExit = os.Exit + func init() { resetLogger() } @@ -29,25 +32,28 @@ func DeferLogs() { func ImmediateLogs() { if logBuffer != nil { os.Stderr.Write(logBuffer.Bytes()) - logBuffer = nil resetLogger() } } func resetLogger() { + logBuffer = nil logger = log.New(os.Stderr, logPrefix, 0) } -func Fatalln(s interface{}) { - logger.Fatalln(s) +func Fatalln(code int, s interface{}) { + logger.Println(s) + OsExit(code) } -func Fatal(v ...interface{}) { - logger.Fatal(v...) +func Fatal(code int, v ...interface{}) { + logger.Print(v...) + OsExit(code) } -func Fatalf(s string, args ...interface{}) { - Fatalln(fmt.Sprintf(s, args...)) +func Fatalf(code int, s string, args ...interface{}) { + logger.Printf(s, args...) + OsExit(code) } func Println(s interface{}) { diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index efcc2d39..bfc9f85e 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -1,10 +1,45 @@ package logger -import "testing" +import ( + "bytes" + "log" + "rare/pkg/testutil" + "testing" + + "github.com/stretchr/testify/assert" +) func TestLog(t *testing.T) { Println("Howdy") DeferLogs() Println("Buffered") + Print("Hi") + Printf("Hi %v", 22) ImmediateLogs() } + +func TestLogFatal(t *testing.T) { + exits := 0 + defer testutil.RestoreGlobals() + testutil.SwitchGlobal(&OsExit, func(code int) { + exits++ + }) + + Fatal(1, "boom") + Fatalln(1, "boom2") + Fatalf(1, "Boom %v", "there") + + assert.Equal(t, 3, exits) +} + +func TestLogCapture(t *testing.T) { + logBuffer = new(bytes.Buffer) + logger = log.New(logBuffer, "", 0) + defer resetLogger() + + Println("Hello") + Print("there") + Printf("bob %v", 22) + + assert.Equal(t, "Hello\nthere\nbob 22\n", logBuffer.String()) +} diff --git a/pkg/testutil/texGenerator_test.go b/pkg/testutil/texGenerator_test.go index 8af482ae..e9a6eeab 100644 --- a/pkg/testutil/texGenerator_test.go +++ b/pkg/testutil/texGenerator_test.go @@ -1,6 +1,7 @@ package testutil import ( + "io" "testing" "github.com/stretchr/testify/assert" @@ -31,4 +32,12 @@ func TestGeneratesData(t *testing.T) { for i := 0; i < n; i++ { assert.NotZero(t, buf[i]) } + + zero(buf) + assert.NoError(t, rg.Close()) + n, err = rg.Read(buf) + assert.Zero(t, n) + assert.ErrorIs(t, err, io.EOF) + + assert.ErrorIs(t, rg.Close(), io.EOF) } diff --git a/pkg/testutil/textGenerator.go b/pkg/testutil/textGenerator.go index ee80145f..c7e9a5d0 100644 --- a/pkg/testutil/textGenerator.go +++ b/pkg/testutil/textGenerator.go @@ -2,10 +2,12 @@ package testutil import ( "io" + "sync/atomic" ) type textGeneratingReader struct { maxChunk int + closed int32 } var _ io.Reader = &textGeneratingReader{} @@ -13,14 +15,18 @@ var _ io.Reader = &textGeneratingReader{} var validText []byte = []byte("abcdefghijklmnopqrstuvwxyz\n") // NewTextGenerator creates a io.reader that generates random alphaetical text separated by new-lines -// Will generate infinitely -func NewTextGenerator(maxReadSize int) io.Reader { +// Will generate infinitely until closed +func NewTextGenerator(maxReadSize int) io.ReadCloser { return &textGeneratingReader{ maxChunk: maxReadSize, } } func (s *textGeneratingReader) Read(buf []byte) (int, error) { + if atomic.LoadInt32(&s.closed) > 0 { + return 0, io.EOF + } + size := len(buf) if size > s.maxChunk { size = s.maxChunk @@ -32,3 +38,11 @@ func (s *textGeneratingReader) Read(buf []byte) (int, error) { return size, nil } + +// Close, next Read() will return EOF (thread-safe, for testing) +func (s *textGeneratingReader) Close() error { + if atomic.SwapInt32(&s.closed, 1) > 0 { + return io.EOF + } + return nil +}