-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revamp the end-of-test summary #4089
base: master
Are you sure you want to change the base?
Changes from all commits
55617fa
0d6eab2
a6d0c68
f324c2c
a6b496b
89379a7
40f3e1c
e088fab
589b0e6
099d0c4
9beea69
ce95d36
6bceb9c
b0215a7
5d30088
b80a09c
b2b5823
4153ba7
af4d6f4
8b1da1d
126a188
63d89e0
21e18dd
644623b
061e60e
8b75bed
037c16a
3a6a7d4
d8e2805
04cd35b
f0aa13a
28fd762
27769d8
ce4b92f
cd633bc
f62c2d4
eee0c99
da116ac
eaf59f5
8cb412c
52ce9db
cf87057
1bb7aaf
bf19d6c
7a057b6
75af9ab
9c1ed70
58e3d02
b8ed6e0
a419570
ff453f9
b4c0509
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,8 @@ extended: base + sets "global" as alias for "globalThis" | |
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("with-summary", lib.SummaryModeCompact.String(), "determine the summary mode,"+ | ||
" \"compact\", \"full\" or \"legacy\"") | ||
flags.String( | ||
"summary-export", | ||
"", | ||
|
@@ -41,94 +43,119 @@ 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"), | ||
CompatibilityMode: getNullString(flags, "compatibility-mode"), | ||
NoThresholds: getNullBool(flags, "no-thresholds"), | ||
NoSummary: getNullBool(flags, "no-summary"), | ||
SummaryMode: getNullString(flags, "with-summary"), | ||
SummaryExport: getNullString(flags, "summary-export"), | ||
TracesOutput: getNullString(flags, "traces-output"), | ||
Env: make(map[string]string), | ||
} | ||
return opts | ||
} | ||
|
||
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"]; ok && !opts.TestType.Valid { | ||
// 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"]; !opts.SummaryMode.Valid && ok { | ||
opts.SummaryMode = null.StringFrom(envVar) | ||
} | ||
|
||
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) | ||
Comment on lines
+127
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of those refactorings would've be nicer as a separate PR so that we don't mix them up here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can try that now, yeah! |
||
} | ||
|
||
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 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: not certain about the name
with-summary
, it doesn't really sound like any other option we have. I would expect this will be eithersummary
orsummary-mode
or something like this.Is there particular reason for the
with-
part ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the initial suggestion was from @oleiade, but as far as I know it's not set in stone.
I'm happy with
--summary-mode
, wdyt? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was the original suggestion indeed, but it is not set in stone indeed. I'm not completely hung upon
--with-summary
, but I proposed it because I like how it is close to natural language and somewhat guessable: "run k6 with its summary in full mode".But like I said, I really don't have a strong opinion on this one, I'd be fine with
--sumary-mode
too. What I care about is that summary related options are consistent; thus if we go for--summary-mode
, when we revamp the machine readable summary, we should make sure the option also keeps the--summary-*
prefix 🙇🏻