Add new testing to log.fatal cases (#99)

Add testing across log.fatal cases, and a few edge-cases

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
 	}

cmd/analyze_test.go

@@ -1,10 +1,19 @@
 package cmd
 
-import "testing"
+import (
+	"rare/cmd/helpers"
+	"testing"
+)
 
 func TestAnalyze(t *testing.T) {
 	testCommandSet(t, analyzeCommand(),
 		`-m (\d+) testdata/graph.txt`,
 		`-x -m (\d+) testdata/graph.txt`,
 	)
 }
+
+func TestAnalyzeParseFatals(t *testing.T) {
+	catchLogFatal(t, helpers.ExitCodeInvalidUsage, func() {
+		testCommand(analyzeCommand(), "--quantile bla testdata/graph.txt")
+	})
+}

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
+}

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)
 	}

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))
+}

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
 }

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)
 }

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
 }

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")
+	})
 }

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
 }

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) {

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
+}

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)
 		}
 	}
 

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`)
+	})
+}

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{}) {