refactor(ui): make summary validation a standalone function, remove e…

…rror handling Resolves: [1], [2] [1]: #1143 (comment) [2]: #1143 (comment)
grafana · Oct 15, 2019 · b4a776d · b4a776d
1 parent dd29bd3
commit b4a776d
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 52 deletions.
diff --git a/cmd/options.go b/cmd/options.go
@@ -147,7 +147,8 @@ func getOptions(flags *pflag.FlagSet) (lib.Options, error) {
 	if err != nil {
 		return opts, err
 	}
-	if _, summaryErr := ui.NewSummary(trendStatStrings); summaryErr != nil {
+
+	if summaryErr := ui.ValidateSummary(trendStatStrings); summaryErr != nil {
 		return opts, summaryErr
 	}
 

diff --git a/cmd/run.go b/cmd/run.go
@@ -62,7 +62,6 @@ const (
 	genericTimeoutErrorCode     = 102
 	genericEngineErrorCode      = 103
 	invalidConfigErrorCode      = 104
-	summaryErrorCode            = 105
 )
 
 var (
@@ -443,14 +442,9 @@ a commandline interface for interacting with it.`,
 
 		// Print the end-of-test summary.
 		if !conf.NoSummary.Bool {
-			s, err := ui.NewSummary(conf.SummaryTrendStats)
-			if err != nil {
-				// Shouldn't happen, config should be valid at this point
-				logrus.WithError(err).Error("Summary error")
-				return ExitCode{errors.New("summary error"), summaryErrorCode}
-			}
-
 			fprintf(stdout, "\n")
+
+			s := ui.NewSummary(conf.SummaryTrendStats)
 			s.SummarizeMetrics(stdout, "", ui.SummaryData{
 				Metrics:   engine.Metrics,
 				RootGroup: engine.Executor.GetRunner().GetDefaultGroup(),

diff --git a/ui/summary.go b/ui/summary.go
@@ -64,15 +64,8 @@ type Summary struct {
 	trendValueResolvers map[string]func(s *stats.TrendSink) interface{}
 }
 
-// NewSummary returns a new Summary struct. It returns an error if the
-// passed stats fields cannot be used as a trend column.
-func NewSummary(sts []string) (*Summary, error) {
-	tc := []string{"avg", "min", "med", "max", "p(90)", "p(95)"}
-	if len(sts) > 0 {
-		tc = sts
-	}
-
-	resolvers := map[string]func(s *stats.TrendSink) interface{}{
+func getDefaultResolvers() map[string]func(s *stats.TrendSink) interface{} {
+	return map[string]func(s *stats.TrendSink) interface{}{
 		"avg":   func(s *stats.TrendSink) interface{} { return s.Avg },
 		"min":   func(s *stats.TrendSink) interface{} { return s.Min },
 		"med":   func(s *stats.TrendSink) interface{} { return s.Med },
@@ -81,18 +74,24 @@ func NewSummary(sts []string) (*Summary, error) {
 		"p(95)": func(s *stats.TrendSink) interface{} { return s.P(0.95) },
 		"count": func(s *stats.TrendSink) interface{} { return s.Count },
 	}
+}
 
-	s := Summary{trendColumns: tc, trendValueResolvers: resolvers}
-	if err := s.Validate(); err != nil {
-		return nil, err
+// NewSummary returns a new Summary instance, used for writing a
+// summary/report of the test metrics data.
+func NewSummary(sts []string) *Summary {
+	tc := []string{"avg", "min", "med", "max", "p(90)", "p(95)"}
+	if len(sts) > 0 {
+		tc = sts
 	}
 
+	s := Summary{trendColumns: tc, trendValueResolvers: getDefaultResolvers()}
+
 	customResolvers := s.generateCustomTrendValueResolvers(sts)
 	for name, res := range customResolvers {
 		s.trendValueResolvers[name] = res
 	}
 
-	return &s, nil
+	return &s
 }
 
 func (s *Summary) generateCustomTrendValueResolvers(sts []string) map[string]func(s *stats.TrendSink) interface{} {
@@ -109,14 +108,17 @@ func (s *Summary) generateCustomTrendValueResolvers(sts []string) map[string]fun
 	return resolvers
 }
 
-// Validate checks if defined trend columns are valid
-func (s *Summary) Validate() error {
-	for _, stat := range s.trendColumns {
+// ValidateSummary checks if passed trend columns are valid for use in
+// the summary output.
+func ValidateSummary(trendColumns []string) error {
+	resolvers := getDefaultResolvers()
+
+	for _, stat := range trendColumns {
 		if stat == "" {
 			return ErrInvalidStat{stat, errStatEmptyString}
 		}
 
-		if _, exists := s.trendValueResolvers[stat]; exists {
+		if _, exists := resolvers[stat]; exists {
 			continue
 		}
 

diff --git a/ui/summary_test.go b/ui/summary_test.go
@@ -32,31 +32,6 @@ import (
 )
 
 func TestSummary(t *testing.T) {
-	t.Run("NewSummary", func(t *testing.T) {
-		var newSummaryTests = []struct {
-			stats  []string
-			expErr error
-		}{
-			{[]string{}, nil},
-			{[]string{"avg", "min", "med", "max", "p(0)", "p(99)", "p(99.999)", "count"}, nil},
-			{[]string{"avg", "p(err)"}, ErrInvalidStat{"p(err)", errPercentileStatInvalidValue}},
-			{[]string{"nil", "p(err)"}, ErrInvalidStat{"nil", errStatUnknownFormat}},
-			{[]string{"p90"}, ErrInvalidStat{"p90", errStatUnknownFormat}},
-			{[]string{"p(90"}, ErrInvalidStat{"p(90", errStatUnknownFormat}},
-			{[]string{" avg"}, ErrInvalidStat{" avg", errStatUnknownFormat}},
-			{[]string{"avg "}, ErrInvalidStat{"avg ", errStatUnknownFormat}},
-			{[]string{"", "avg "}, ErrInvalidStat{"", errStatEmptyString}},
-		}
-
-		for _, tc := range newSummaryTests {
-			tc := tc
-			t.Run(fmt.Sprintf("%v", tc.stats), func(t *testing.T) {
-				_, err := NewSummary(tc.stats)
-				assert.Equal(t, tc.expErr, err)
-			})
-		}
-	})
-
 	t.Run("SummarizeMetrics", func(t *testing.T) {
 		var (
 			checksOut = "   ✓ checks......: 100.00% ✓ 3   ✗ 0  \n"
@@ -79,7 +54,7 @@ func TestSummary(t *testing.T) {
 			tc := tc
 			t.Run(fmt.Sprintf("%v", tc.stats), func(t *testing.T) {
 				var w bytes.Buffer
-				s, _ := NewSummary(tc.stats)
+				s := NewSummary(tc.stats)
 
 				s.SummarizeMetrics(&w, " ", SummaryData{
 					Metrics:   metrics,
@@ -119,6 +94,31 @@ func TestSummary(t *testing.T) {
 	})
 }
 
+func TestValidateSummary(t *testing.T) {
+	var newSummaryTests = []struct {
+		stats  []string
+		expErr error
+	}{
+		{[]string{}, nil},
+		{[]string{"avg", "min", "med", "max", "p(0)", "p(99)", "p(99.999)", "count"}, nil},
+		{[]string{"avg", "p(err)"}, ErrInvalidStat{"p(err)", errPercentileStatInvalidValue}},
+		{[]string{"nil", "p(err)"}, ErrInvalidStat{"nil", errStatUnknownFormat}},
+		{[]string{"p90"}, ErrInvalidStat{"p90", errStatUnknownFormat}},
+		{[]string{"p(90"}, ErrInvalidStat{"p(90", errStatUnknownFormat}},
+		{[]string{" avg"}, ErrInvalidStat{" avg", errStatUnknownFormat}},
+		{[]string{"avg "}, ErrInvalidStat{"avg ", errStatUnknownFormat}},
+		{[]string{"", "avg "}, ErrInvalidStat{"", errStatEmptyString}},
+	}
+
+	for _, tc := range newSummaryTests {
+		tc := tc
+		t.Run(fmt.Sprintf("%v", tc.stats), func(t *testing.T) {
+			err := ValidateSummary(tc.stats)
+			assert.Equal(t, tc.expErr, err)
+		})
+	}
+}
+
 func createTestTrendSink(count int) *stats.TrendSink {
 	sink := stats.TrendSink{}