Chore: Refactor lib.RuntimeOptions load

Author: joanlopez

internal/cmd/runtime_options.go

@@ -41,25 +41,35 @@ extended: base + sets "global" as alias for "globalThis"
 	return flags
 }
 
-func saveBoolFromEnv(env map[string]string, varName string, placeholder *null.Bool) error {
-	strValue, ok := env[varName]
-	if !ok {
-		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, err := populateRuntimeOptionsFromEnv(runtimeOptionsFromFlags(flags), environment)
+	if err != nil {
+		return opts, err
 	}
-	val, err := strconv.ParseBool(strValue)
+
+	// Set/overwrite environment variables with custom user-supplied values
+	envVars, err := flags.GetStringArray("env")
 	if err != nil {
-		return fmt.Errorf("env var '%s' is not a valid boolean value: %w", varName, err)
+		return opts, err
 	}
-	// Only override if not explicitly set via the CLI flag
-	if !placeholder.Valid {
-		*placeholder = null.BoolFrom(val)
+	for _, kv := range envVars {
+		k, v := state.ParseEnvKeyValue(kv)
+		// Allow only alphanumeric ASCII variable names for now
+		if !userEnvVarName.MatchString(k) {
+			return opts, fmt.Errorf("invalid environment variable name '%s'", k)
+		}
+		opts.Env[k] = v
 	}
-	return nil
+
+	return opts, 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?
+func runtimeOptionsFromFlags(flags *pflag.FlagSet) lib.RuntimeOptions {
 	opts := lib.RuntimeOptions{
 		TestType:             getNullString(flags, "type"),
 		IncludeSystemEnvVars: getNullBool(flags, "include-system-env-vars"),
@@ -70,65 +80,69 @@ func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib
 		TracesOutput:         getNullString(flags, "traces-output"),
 		Env:                  make(map[string]string),
 	}
+	return opts
+}
 
-	if envVar, ok := environment["K6_TYPE"]; ok && !opts.TestType.Valid {
-		// Only override if not explicitly set via the CLI flag
+func populateRuntimeOptionsFromEnv(opts lib.RuntimeOptions, environment map[string]string) (lib.RuntimeOptions, error) {
+	// Only override if not explicitly set via the CLI flag
+
+	if envVar, ok := environment["K6_TYPE"]; !opts.TestType.Valid && ok {
 		opts.TestType = null.StringFrom(envVar)
 	}
-	if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok && !opts.CompatibilityMode.Valid {
-		// Only override if not explicitly set via the CLI flag
+
+	if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; !opts.CompatibilityMode.Valid && ok {
 		opts.CompatibilityMode = null.StringFrom(envVar)
 	}
-	if _, err := lib.ValidateCompatibilityMode(opts.CompatibilityMode.String); err != nil {
-		// some early validation
-		return opts, err
-	}
 
 	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)
-		}
+	if _, err := lib.ValidateCompatibilityMode(opts.CompatibilityMode.String); err != nil {
+		// some early validation
+		return opts, err
 	}
 
-	if envVar, ok := environment["SSLKEYLOGFILE"]; ok {
-		if !opts.KeyWriter.Valid {
-			opts.KeyWriter = null.StringFrom(envVar)
-		}
+	if envVar, ok := environment["K6_SUMMARY_EXPORT"]; !opts.SummaryExport.Valid && ok {
+		opts.SummaryExport = null.StringFrom(envVar)
 	}
 
-	if envVar, ok := environment["K6_TRACES_OUTPUT"]; ok {
-		if !opts.TracesOutput.Valid {
-			opts.TracesOutput = null.StringFrom(envVar)
-		}
+	if envVar, ok := environment["SSLKEYLOGFILE"]; !opts.KeyWriter.Valid && ok {
+		opts.KeyWriter = null.StringFrom(envVar)
+	}
+
+	if envVar, ok := environment["K6_TRACES_OUTPUT"]; !opts.TracesOutput.Valid && ok {
+		opts.TracesOutput = null.StringFrom(envVar)
 	}
 
-	if opts.IncludeSystemEnvVars.Bool { // If enabled, gather the actual system environment variables
+	// If enabled, gather the actual system environment variables
+	if opts.IncludeSystemEnvVars.Bool {
 		opts.Env = environment
 	}
 
-	// Set/overwrite environment variables with custom user-supplied values
-	envVars, err := flags.GetStringArray("env")
+	return opts, nil
+}
+
+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 opts, err
+		return fmt.Errorf("env var '%s' is not a valid boolean value: %w", varName, err)
 	}
-	for _, kv := range envVars {
-		k, v := state.ParseEnvKeyValue(kv)
-		// Allow only alphanumeric ASCII variable names for now
-		if !userEnvVarName.MatchString(k) {
-			return opts, fmt.Errorf("invalid environment variable name '%s'", k)
-		}
-		opts.Env[k] = v
+	// Only override if not explicitly set via the CLI flag
+	if !placeholder.Valid {
+		*placeholder = null.BoolFrom(val)
 	}
-
-	return opts, nil
+	return nil
 }