From 9c1ed7028419c25b3a66a501f2b46360dd8310fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20L=C3=B3pez=20de=20la=20Franca=20Beltran?= Date: Fri, 7 Feb 2025 22:14:05 +0100 Subject: [PATCH] Refactor runtime options init func --- internal/cmd/runtime_options.go | 121 ++++++++++++++++------------- internal/cmd/tests/cmd_run_test.go | 38 ++++----- 2 files changed, 86 insertions(+), 73 deletions(-) diff --git a/internal/cmd/runtime_options.go b/internal/cmd/runtime_options.go index 5c9f88afaed..dceda9f955f 100644 --- a/internal/cmd/runtime_options.go +++ b/internal/cmd/runtime_options.go @@ -43,28 +43,36 @@ 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) { //nolint:funlen - // 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"), @@ -76,73 +84,78 @@ func getRuntimeOptions( 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 envVar, ok := environment["K6_WITH_SUMMARY"]; ok && !opts.SummaryMode.Valid { + if envVar, ok := environment["K6_WITH_SUMMARY"]; !opts.SummaryMode.Valid && ok { opts.SummaryMode = null.StringFrom(envVar) } - if _, err := lib.ValidateSummaryMode(opts.SummaryMode.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 _, err := lib.ValidateSummaryMode(opts.SummaryMode.String); err != nil { + // some early validation + return opts, err } - if envVar, ok := environment["K6_TRACES_OUTPUT"]; ok { - if !opts.TracesOutput.Valid { - opts.TracesOutput = null.StringFrom(envVar) - } + if envVar, ok := environment["K6_SUMMARY_EXPORT"]; !opts.SummaryExport.Valid && ok { + opts.SummaryExport = null.StringFrom(envVar) } - if opts.IncludeSystemEnvVars.Bool { // If enabled, gather the actual system environment variables - opts.Env = environment + if envVar, ok := environment["SSLKEYLOGFILE"]; !opts.KeyWriter.Valid && ok { + opts.KeyWriter = null.StringFrom(envVar) } - // Set/overwrite environment variables with custom user-supplied values - envVars, err := flags.GetStringArray("env") - if err != nil { - return opts, err + if envVar, ok := environment["K6_TRACES_OUTPUT"]; !opts.TracesOutput.Valid && ok { + opts.TracesOutput = null.StringFrom(envVar) } - 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 + + // If enabled, gather the actual system environment variables + if opts.IncludeSystemEnvVars.Bool { + opts.Env = environment } 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 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 +} diff --git a/internal/cmd/tests/cmd_run_test.go b/internal/cmd/tests/cmd_run_test.go index a173675ea6c..a9dc41326ca 100644 --- a/internal/cmd/tests/cmd_run_test.go +++ b/internal/cmd/tests/cmd_run_test.go @@ -2104,14 +2104,14 @@ func TestEventSystemError(t *testing.T) { test.abort('oops!'); } `, expLog: []string{ - "got event Init with data ''", - "got event TestStart with data ''", - "got event IterStart with data '{Iteration:0 VUID:1 ScenarioName:default Error:}'", - "got event IterEnd with data '{Iteration:0 VUID:1 ScenarioName:default Error:test aborted: oops! at default (file:///-:11:16(5))}'", - "got event TestEnd with data ''", - "got event Exit with data '&{Error:test aborted: oops! at default (file:///-:11:16(5))}'", - "test aborted: oops! at default (file:///-:11:16(5))", - }, + "got event Init with data ''", + "got event TestStart with data ''", + "got event IterStart with data '{Iteration:0 VUID:1 ScenarioName:default Error:}'", + "got event IterEnd with data '{Iteration:0 VUID:1 ScenarioName:default Error:test aborted: oops! at default (file:///-:11:16(5))}'", + "got event TestEnd with data ''", + "got event Exit with data '&{Error:test aborted: oops! at default (file:///-:11:16(5))}'", + "test aborted: oops! at default (file:///-:11:16(5))", + }, expExitCode: exitcodes.ScriptAborted, }, { @@ -2136,17 +2136,17 @@ func TestEventSystemError(t *testing.T) { throw new Error('oops!'); } `, expLog: []string{ - "got event Init with data ''", - "got event TestStart with data ''", - "got event IterStart with data '{Iteration:0 VUID:1 ScenarioName:default Error:}'", - "got event IterEnd with data '{Iteration:0 VUID:1 ScenarioName:default Error:Error: oops!\n\tat default (file:///-:9:12(3))\n}'", - "Error: oops!\n\tat default (file:///-:9:12(3))\n", - "got event IterStart with data '{Iteration:1 VUID:1 ScenarioName:default Error:}'", - "got event IterEnd with data '{Iteration:1 VUID:1 ScenarioName:default Error:Error: oops!\n\tat default (file:///-:9:12(3))\n}'", - "Error: oops!\n\tat default (file:///-:9:12(3))\n", - "got event TestEnd with data ''", - "got event Exit with data '&{Error:}'", - }, + "got event Init with data ''", + "got event TestStart with data ''", + "got event IterStart with data '{Iteration:0 VUID:1 ScenarioName:default Error:}'", + "got event IterEnd with data '{Iteration:0 VUID:1 ScenarioName:default Error:Error: oops!\n\tat default (file:///-:9:12(3))\n}'", + "Error: oops!\n\tat default (file:///-:9:12(3))\n", + "got event IterStart with data '{Iteration:1 VUID:1 ScenarioName:default Error:}'", + "got event IterEnd with data '{Iteration:1 VUID:1 ScenarioName:default Error:Error: oops!\n\tat default (file:///-:9:12(3))\n}'", + "Error: oops!\n\tat default (file:///-:9:12(3))\n", + "got event TestEnd with data ''", + "got event Exit with data '&{Error:}'", + }, expExitCode: 0, }, }