Merge pull request #1788 from loadimpact/misc-fixes-for-summary

Misc fixes for summary callback changes

api/server_test.go

@@ -82,7 +82,7 @@ func TestWithEngine(t *testing.T) {
 	logger.SetOutput(testutils.NewTestOutput(t))
 	execScheduler, err := local.NewExecutionScheduler(&minirunner.MiniRunner{}, logger)
 	require.NoError(t, err)
-	engine, err := core.NewEngine(execScheduler, lib.Options{}, logger)
+	engine, err := core.NewEngine(execScheduler, lib.Options{}, lib.RuntimeOptions{}, logger)
 	require.NoError(t, err)
 
 	rw := httptest.NewRecorder()

api/v1/group_routes_test.go

@@ -51,7 +51,7 @@ func TestGetGroups(t *testing.T) {
 
 	execScheduler, err := local.NewExecutionScheduler(&minirunner.MiniRunner{Group: g0}, logger)
 	require.NoError(t, err)
-	engine, err := core.NewEngine(execScheduler, lib.Options{}, logger)
+	engine, err := core.NewEngine(execScheduler, lib.Options{}, lib.RuntimeOptions{}, logger)
 	require.NoError(t, err)
 
 	t.Run("list", func(t *testing.T) {

api/v1/metric_routes_test.go

@@ -45,7 +45,7 @@ func TestGetMetrics(t *testing.T) {
 	logger.SetOutput(testutils.NewTestOutput(t))
 	execScheduler, err := local.NewExecutionScheduler(&minirunner.MiniRunner{}, logger)
 	require.NoError(t, err)
-	engine, err := core.NewEngine(execScheduler, lib.Options{}, logger)
+	engine, err := core.NewEngine(execScheduler, lib.Options{}, lib.RuntimeOptions{}, logger)
 	require.NoError(t, err)
 
 	engine.Metrics = map[string]*stats.Metric{
@@ -88,7 +88,7 @@ func TestGetMetric(t *testing.T) {
 	logger.SetOutput(testutils.NewTestOutput(t))
 	execScheduler, err := local.NewExecutionScheduler(&minirunner.MiniRunner{}, logger)
 	require.NoError(t, err)
-	engine, err := core.NewEngine(execScheduler, lib.Options{}, logger)
+	engine, err := core.NewEngine(execScheduler, lib.Options{}, lib.RuntimeOptions{}, logger)
 	require.NoError(t, err)
 
 	engine.Metrics = map[string]*stats.Metric{

api/v1/setup_teardown_routes_test.go

@@ -155,7 +155,7 @@ func TestSetupData(t *testing.T) {
 			})
 			execScheduler, err := local.NewExecutionScheduler(runner, logger)
 			require.NoError(t, err)
-			engine, err := core.NewEngine(execScheduler, runner.GetOptions(), logger)
+			engine, err := core.NewEngine(execScheduler, runner.GetOptions(), lib.RuntimeOptions{}, logger)
 			require.NoError(t, err)
 
 			globalCtx, globalCancel := context.WithCancel(context.Background())

api/v1/status_routes_test.go

@@ -47,7 +47,7 @@ func TestGetStatus(t *testing.T) {
 	logger.SetOutput(testutils.NewTestOutput(t))
 	execScheduler, err := local.NewExecutionScheduler(&minirunner.MiniRunner{}, logger)
 	require.NoError(t, err)
-	engine, err := core.NewEngine(execScheduler, lib.Options{}, logger)
+	engine, err := core.NewEngine(execScheduler, lib.Options{}, lib.RuntimeOptions{}, logger)
 	require.NoError(t, err)
 
 	rw := httptest.NewRecorder()
@@ -101,7 +101,7 @@ func TestPatchStatus(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			execScheduler, err := local.NewExecutionScheduler(&minirunner.MiniRunner{Options: options}, logger)
 			require.NoError(t, err)
-			engine, err := core.NewEngine(execScheduler, options, logger)
+			engine, err := core.NewEngine(execScheduler, options, lib.RuntimeOptions{}, logger)
 			require.NoError(t, err)
 			ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 			defer cancel()

cmd/config.go

@@ -53,25 +53,15 @@ func configFlagSet() *pflag.FlagSet {
 	flags.StringArrayP("out", "o", []string{}, "`uri` for an external metrics database")
 	flags.BoolP("linger", "l", false, "keep the API server alive past test end")
 	flags.Bool("no-usage-report", false, "don't send anonymous stats to the developers")
-	flags.Bool("no-thresholds", false, "don't run thresholds")
-	flags.Bool("no-summary", false, "don't show the summary at the end of the test")
-	flags.String(
-		"summary-export",
-		"",
-		"output the end-of-test summary report to JSON file",
-	)
 	return flags
 }
 
 type Config struct {
 	lib.Options
 
-	Out           []string    `json:"out" envconfig:"K6_OUT"`
-	Linger        null.Bool   `json:"linger" envconfig:"K6_LINGER"`
-	NoUsageReport null.Bool   `json:"noUsageReport" envconfig:"K6_NO_USAGE_REPORT"`
-	NoThresholds  null.Bool   `json:"noThresholds" envconfig:"K6_NO_THRESHOLDS"`
-	NoSummary     null.Bool   `json:"noSummary" envconfig:"K6_NO_SUMMARY"`
-	SummaryExport null.String `json:"summaryExport" envconfig:"K6_SUMMARY_EXPORT"`
+	Out           []string  `json:"out" envconfig:"K6_OUT"`
+	Linger        null.Bool `json:"linger" envconfig:"K6_LINGER"`
+	NoUsageReport null.Bool `json:"noUsageReport" envconfig:"K6_NO_USAGE_REPORT"`
 
 	Collectors struct {
 		InfluxDB influxdb.Config `json:"influxdb"`
@@ -103,15 +93,6 @@ func (c Config) Apply(cfg Config) Config {
 	if cfg.NoUsageReport.Valid {
 		c.NoUsageReport = cfg.NoUsageReport
 	}
-	if cfg.NoThresholds.Valid {
-		c.NoThresholds = cfg.NoThresholds
-	}
-	if cfg.NoSummary.Valid {
-		c.NoSummary = cfg.NoSummary
-	}
-	if cfg.SummaryExport.Valid {
-		c.SummaryExport = cfg.SummaryExport
-	}
 	c.Collectors.InfluxDB = c.Collectors.InfluxDB.Apply(cfg.Collectors.InfluxDB)
 	c.Collectors.Cloud = c.Collectors.Cloud.Apply(cfg.Collectors.Cloud)
 	c.Collectors.Kafka = c.Collectors.Kafka.Apply(cfg.Collectors.Kafka)
@@ -136,9 +117,6 @@ func getConfig(flags *pflag.FlagSet) (Config, error) {
 		Out:           out,
 		Linger:        getNullBool(flags, "linger"),
 		NoUsageReport: getNullBool(flags, "no-usage-report"),
-		NoThresholds:  getNullBool(flags, "no-thresholds"),
-		NoSummary:     getNullBool(flags, "no-summary"),
-		SummaryExport: getNullString(flags, "summary-export"),
 	}, nil
 }
 

cmd/run.go

@@ -192,23 +192,11 @@ a commandline interface for interacting with it.`,
 
 			// Create an engine.
 			initBar.Modify(pb.WithConstProgress(0, "Init engine"))
-			engine, err := core.NewEngine(execScheduler, conf.Options, logger)
+			engine, err := core.NewEngine(execScheduler, conf.Options, runtimeOptions, logger)
 			if err != nil {
 				return err
 			}
 
-			// TODO: refactor, the engine should have a copy of the config...
-			// Configure the engine.
-			if conf.NoThresholds.Valid {
-				engine.NoThresholds = conf.NoThresholds.Bool
-			}
-			if conf.NoSummary.Valid {
-				engine.NoSummary = conf.NoSummary.Bool
-			}
-			if conf.SummaryExport.Valid {
-				engine.SummaryExport = conf.SummaryExport.String != ""
-			}
-
 			executionPlan := execScheduler.GetExecutionPlan()
 			// Create a collector and assign it to the engine if requested.
 			initBar.Modify(pb.WithConstProgress(0, "Init metric outputs"))
@@ -309,7 +297,7 @@ a commandline interface for interacting with it.`,
 				TimeUnit:  conf.Options.SummaryTimeUnit.String,
 			}
 			// Print the end-of-test summary.
-			if !conf.NoSummary.Bool {
+			if !runtimeOptions.NoSummary.Bool {
 				fprintf(stdout, "\n")
 
 				s := ui.NewSummary(conf.SummaryTrendStats)
@@ -318,8 +306,8 @@ a commandline interface for interacting with it.`,
 				fprintf(stdout, "\n")
 			}
 
-			if conf.SummaryExport.ValueOrZero() != "" { //nolint:nestif
-				f, err := os.Create(conf.SummaryExport.String)
+			if runtimeOptions.SummaryExport.ValueOrZero() != "" { //nolint:nestif
+				f, err := os.Create(runtimeOptions.SummaryExport.String)
 				if err != nil {
 					logger.WithError(err).Error("failed to create summary export file")
 				} else {

cmd/runtime_options.go

@@ -21,6 +21,7 @@
 package cmd
 
 import (
+	"fmt"
 	"regexp"
 	"strconv"
 	"strings"
@@ -65,18 +66,47 @@ extended: base + Babel with ES2015 preset + core.js v2,
           slower and memory consuming but with greater JS support
 `)
 	flags.StringArrayP("env", "e", nil, "add/override environment variable with `VAR=value`")
+	flags.Bool("no-thresholds", false, "don't run thresholds")
+	flags.Bool("no-summary", false, "don't show the summary at the end of the test")
+	flags.String(
+		"summary-export",
+		"",
+		"output the end-of-test summary report to JSON file",
+	)
 	return flags
 }
 
+func saveBoolFromEnv(env map[string]string, varName string, placeholder *null.Bool) error {
+	strValue, ok := env[varName]
+	if !ok {
+		return nil
+	}
+	val, err := strconv.ParseBool(strValue)
+	if err != nil {
+		return fmt.Errorf("env var '%s' is not a valid boolean value: %w", varName, err)
+	}
+	// Only override if not explicitly set via the CLI flag
+	if !placeholder.Valid {
+		*placeholder = null.BoolFrom(val)
+	}
+	return nil
+}
+
 func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib.RuntimeOptions, error) {
+	// TODO: refactor with composable helpers as a part of #883, to reduce copy-paste
+	// TODO: get these options out of the JSON config file as well?
 	opts := lib.RuntimeOptions{
 		IncludeSystemEnvVars: getNullBool(flags, "include-system-env-vars"),
 		CompatibilityMode:    getNullString(flags, "compatibility-mode"),
+		NoThresholds:         getNullBool(flags, "no-thresholds"),
+		NoSummary:            getNullBool(flags, "no-summary"),
+		SummaryExport:        getNullString(flags, "summary-export"),
 		Env:                  make(map[string]string),
 	}
 
-	if !opts.CompatibilityMode.Valid { // If not explicitly set via CLI flags, look for an environment variable
-		if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok {
+	if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok {
+		// Only override if not explicitly set via the CLI flag
+		if !opts.CompatibilityMode.Valid {
 			opts.CompatibilityMode = null.StringFrom(envVar)
 		}
 	}
@@ -85,13 +115,19 @@ func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib
 		return opts, err
 	}
 
-	if !opts.IncludeSystemEnvVars.Valid { // If not explicitly set via CLI flags, look for an environment variable
-		if envVar, ok := environment["K6_INCLUDE_SYSTEM_ENV_VARS"]; ok {
-			val, err := strconv.ParseBool(envVar)
-			if err != nil {
-				return opts, err
-			}
-			opts.IncludeSystemEnvVars = null.BoolFrom(val)
+	if err := saveBoolFromEnv(environment, "K6_INCLUDE_SYSTEM_ENV_VARS", &opts.IncludeSystemEnvVars); err != nil {
+		return opts, err
+	}
+	if err := saveBoolFromEnv(environment, "K6_NO_THRESHOLDS", &opts.NoThresholds); err != nil {
+		return opts, err
+	}
+	if err := saveBoolFromEnv(environment, "K6_NO_SUMMARY", &opts.NoSummary); err != nil {
+		return opts, err
+	}
+
+	if envVar, ok := environment["K6_SUMMARY_EXPORT"]; ok {
+		if !opts.SummaryExport.Valid {
+			opts.SummaryExport = null.StringFrom(envVar)
 		}
 	}