feat(ui): add optional 'count' column to CLI summary output

This adds an optional count column for Trend metrics in the CLI summary
output, while refactoring parts of ui/summary.go for code style
(avoiding globals, explicit validation, etc.) and negligible performance
improvements (using a map for column lookups).

To enable it run e.g. `k6 run --summary-trend-stats 'avg,p(99.99),count' test.js`.

Closes #1087

cmd/config.go

@@ -44,6 +44,7 @@ import (
 	"github.com/loadimpact/k6/stats/influxdb"
 	"github.com/loadimpact/k6/stats/kafka"
 	"github.com/loadimpact/k6/stats/statsd/common"
+	"github.com/loadimpact/k6/ui"
 )
 
 // configFlagSet returns a FlagSet with the default run configuration flags.
@@ -309,17 +310,30 @@ func getConsolidatedConfig(fs afero.Fs, cliConf Config, runner lib.Runner) (conf
 	conf = conf.Apply(envConf).Apply(cliConf)
 	conf = applyDefault(conf)
 
+	// TODO(imiric): Move this validation where it makes sense in the configuration
+	// refactor of #883. Currently we validate CLI flags in cmd.getOptions. Yet
+	// there is no place where configuration is validated after applying all other
+	// sources, like environment variables (besides inline in the runCmd itself...).
+	// So this repeats the trend stats validation in case other sources overrode our
+	// default value.
+	if err = ui.ValidateSummary(conf.SummaryTrendStats); err != nil {
+		return conf, err
+	}
+
 	return conf, nil
 }
 
-// applyDefault applys default options value if it is not specified by any mechenisms. This happens with types
-// which does not support by "gopkg.in/guregu/null.v3".
+// applyDefault applies the default options value if it is not specified.
+// This happens with types which are not supported by "gopkg.in/guregu/null.v3".
 //
 // Note that if you add option default value here, also add it in command line argument help text.
 func applyDefault(conf Config) Config {
 	if conf.Options.SystemTags == nil {
 		conf = conf.Apply(Config{Options: lib.Options{SystemTags: &stats.DefaultSystemTagSet}})
 	}
+	if len(conf.Options.SummaryTrendStats) == 0 {
+		conf = conf.Apply(Config{Options: lib.Options{SummaryTrendStats: lib.DefaultSummaryTrendStats}})
+	}
 	return conf
 }
 

cmd/config_consolidation_test.go

@@ -412,6 +412,20 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
 				)
 			},
 		},
+		// Test summary trend stats
+		{opts{}, exp{}, func(t *testing.T, c Config) {
+			assert.Equal(t, lib.DefaultSummaryTrendStats, c.Options.SummaryTrendStats)
+		}},
+		{opts{cli: []string{"--summary-trend-stats", `""`}}, exp{}, func(t *testing.T, c Config) {
+			assert.Equal(t, lib.DefaultSummaryTrendStats, c.Options.SummaryTrendStats)
+		}},
+		{
+			opts{runner: &lib.Options{SummaryTrendStats: []string{"avg", "p(90)", "count"}}},
+			exp{},
+			func(t *testing.T, c Config) {
+				assert.Equal(t, []string{"avg", "p(90)", "count"}, c.Options.SummaryTrendStats)
+			},
+		},
 		//TODO: test for differences between flagsets
 		//TODO: more tests in general, especially ones not related to execution parameters...
 	}

cmd/options.go

@@ -63,7 +63,14 @@ func optionFlagSet() *pflag.FlagSet {
 	flags.Duration("min-iteration-duration", 0, "minimum amount of time k6 will take executing a single iteration")
 	flags.BoolP("throw", "w", false, "throw warnings (like failed http requests) as errors")
 	flags.StringSlice("blacklist-ip", nil, "blacklist an `ip range` from being called")
-	flags.StringSlice("summary-trend-stats", nil, "define `stats` for trend metrics (response times), one or more as 'avg,p(95),...'")
+
+	// The comment about system-tags also applies for summary-trend-stats. The default values
+	// are set in applyDefault().
+	sumTrendStatsHelp := fmt.Sprintf(
+		"define `stats` for trend metrics (response times), one or more as 'avg,p(95),...' (default '%s')",
+		strings.Join(lib.DefaultSummaryTrendStats, ","),
+	)
+	flags.StringSlice("summary-trend-stats", nil, sumTrendStatsHelp)
 	flags.String("summary-time-unit", "", "define the time unit used to display the trend stats. Possible units are: 's', 'ms' and 'us'")
 	// system-tags must have a default value, but we can't specify it here, otherwiese, it will always override others.
 	// set it to nil here, and add the default in applyDefault() instead.
@@ -143,16 +150,15 @@ func getOptions(flags *pflag.FlagSet) (lib.Options, error) {
 		opts.BlacklistIPs = append(opts.BlacklistIPs, net)
 	}
 
-	trendStatStrings, err := flags.GetStringSlice("summary-trend-stats")
-	if err != nil {
-		return opts, err
-	}
-	for _, s := range trendStatStrings {
-		if err := ui.VerifyTrendColumnStat(s); err != nil {
-			return opts, errors.Wrapf(err, "stat '%s'", s)
+	if flags.Changed("summary-trend-stats") {
+		trendStats, errSts := flags.GetStringSlice("summary-trend-stats")
+		if errSts != nil {
+			return opts, errSts
 		}
-
-		opts.SummaryTrendStats = append(opts.SummaryTrendStats, s)
+		if errSts = ui.ValidateSummary(trendStats); err != nil {
+			return opts, errSts
+		}
+		opts.SummaryTrendStats = trendStats
 	}
 
 	summaryTimeUnit, err := flags.GetString("summary-time-unit")

cmd/run.go

@@ -176,11 +176,6 @@ a commandline interface for interacting with it.`,
 			return ExitCode{cerr, invalidConfigErrorCode}
 		}
 
-		// If summary trend stats are defined, update the UI to reflect them
-		if len(conf.SummaryTrendStats) > 0 {
-			ui.UpdateTrendColumns(conf.SummaryTrendStats)
-		}
-
 		// Write options back to the runner too.
 		if err = r.SetOptions(conf.Options); err != nil {
 			return err
@@ -448,12 +443,15 @@ a commandline interface for interacting with it.`,
 		// Print the end-of-test summary.
 		if !conf.NoSummary.Bool {
 			fprintf(stdout, "\n")
-			ui.Summarize(stdout, "", ui.SummaryData{
-				Opts:    conf.Options,
-				Root:    engine.Executor.GetRunner().GetDefaultGroup(),
-				Metrics: engine.Metrics,
-				Time:    engine.Executor.GetTime(),
+
+			s := ui.NewSummary(conf.SummaryTrendStats)
+			s.SummarizeMetrics(stdout, "", ui.SummaryData{
+				Metrics:   engine.Metrics,
+				RootGroup: engine.Executor.GetRunner().GetDefaultGroup(),
+				Time:      engine.Executor.GetTime(),
+				TimeUnit:  conf.Options.SummaryTimeUnit.String,
 			})
+
 			fprintf(stdout, "\n")
 		}
 

lib/options.go

@@ -39,6 +39,10 @@ import (
 // iterations+vus, or stages)
 const DefaultSchedulerName = "default"
 
+// DefaultSummaryTrendStats are the default trend columns shown in the test summary output
+// nolint: gochecknoglobals
+var DefaultSummaryTrendStats = []string{"avg", "min", "med", "max", "p(90)", "p(95)"}
+
 // Describes a TLS version. Serialised to/from JSON as a string, eg. "tls1.2".
 type TLSVersion int