Skip to content
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

More cmd/ refactoring to simplify test loading and support integration tests #2412

Merged
merged 10 commits into from
Mar 18, 2022
2 changes: 0 additions & 2 deletions cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ func getInspectCmd(gs *globalState) *cobra.Command {

inspectCmd.Flags().SortFlags = false
inspectCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false))
inspectCmd.Flags().StringVarP(&gs.flags.testType, "type", "t",
gs.flags.testType, "override file `type`, \"js\" or \"archive\"")
inspectCmd.Flags().BoolVar(&addExecReqs,
"execution-requirements",
false,
Expand Down
4 changes: 0 additions & 4 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ const (
// globalFlags contains global config values that apply for all k6 sub-commands.
type globalFlags struct {
configFilePath string
testType string // TODO: move to RuntimeOptions, it's not trully global
quiet bool
noColor bool
address string
Expand Down Expand Up @@ -174,9 +173,6 @@ func getFlags(defaultFlags globalFlags, env map[string]string) globalFlags {
if val, ok := env["K6_CONFIG"]; ok {
result.configFilePath = val
}
if val, ok := env["K6_TYPE"]; ok {
result.testType = val
}
if val, ok := env["K6_LOG_OUTPUT"]; ok {
result.logOutput = val
}
Expand Down
15 changes: 2 additions & 13 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ a commandline interface for interacting with it.`,
}

runCmd.Flags().SortFlags = false
runCmd.Flags().AddFlagSet(runCmdFlagSet(globalState))
runCmd.Flags().AddFlagSet(runCmdFlagSet())

return runCmd
}
Expand Down Expand Up @@ -329,23 +329,12 @@ func reportUsage(execScheduler *local.ExecutionScheduler) error {
return err
}

func runCmdFlagSet(globalState *globalState) *pflag.FlagSet {
func runCmdFlagSet() *pflag.FlagSet {
flags := pflag.NewFlagSet("", pflag.ContinueOnError)
flags.SortFlags = false
flags.AddFlagSet(optionFlagSet())
flags.AddFlagSet(runtimeOptionFlagSet(true))
flags.AddFlagSet(configFlagSet())

// TODO: Figure out a better way to handle the CLI flags:
// - the default values are specified in this way so we don't overwrire whatever
// was specified via the environment variables
// - but we need to manually specify the DefValue, since that's the default value
// that will be used in the help/usage message - if we don't set it, the environment
// variables will affect the usage message
// - and finally, global variables are not very testable... :/
flags.StringVarP(&globalState.flags.testType, "type", "t",
globalState.flags.testType, "override file `type`, \"js\" or \"archive\"")
flags.Lookup("type").DefValue = ""
return flags
}

Expand Down
12 changes: 8 additions & 4 deletions cmd/runtime_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ base: pure goja - Golang JS VM supporting ES5.1+
extended: base + Babel with parts of ES2015 preset
slower to compile in case the script uses syntax unsupported by base
`)
flags.StringP("type", "t", "", "override test type, \"js\" or \"archive\"")
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")
Expand Down Expand Up @@ -78,6 +79,7 @@ func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib
// 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{
TestType: getNullString(flags, "type"),
IncludeSystemEnvVars: getNullBool(flags, "include-system-env-vars"),
CompatibilityMode: getNullString(flags, "compatibility-mode"),
NoThresholds: getNullBool(flags, "no-thresholds"),
Expand All @@ -86,11 +88,13 @@ func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib
Env: make(map[string]string),
}

if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok {
if envVar, ok := environment["K6_TYPE"]; ok && !opts.TestType.Valid {
// Only override if not explicitly set via the CLI flag
if !opts.CompatibilityMode.Valid {
opts.CompatibilityMode = null.StringFrom(envVar)
}
opts.TestType = null.StringFrom(envVar)
}
if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok && !opts.CompatibilityMode.Valid {
oleiade marked this conversation as resolved.
Show resolved Hide resolved
// Only override if not explicitly set via the CLI flag
opts.CompatibilityMode = null.StringFrom(envVar)
}
if _, err := lib.ValidateCompatibilityMode(opts.CompatibilityMode.String); err != nil {
// some early validation
Expand Down
2 changes: 1 addition & 1 deletion cmd/test_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (lt *loadedTest) initializeFirstRunner(gs *globalState) error {
testPath := lt.source.URL.String()
logger := gs.logger.WithField("test_path", testPath)

testType := gs.flags.testType
testType := lt.runtimeOptions.TestType.String
if testType == "" {
logger.Debug("Detecting test type for...")
testType = detectTestType(lt.source.Data)
Expand Down
2 changes: 2 additions & 0 deletions lib/runtime_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const (

// RuntimeOptions are settings passed onto the goja JS runtime
type RuntimeOptions struct {
TestType null.String `json:"-"`

Comment on lines +44 to +45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do actually write this in the archive under type

https://github.com/grafana/k6/blob/master/js/bundle.go#L190

maybe one day we can make MakeArchive outside of a Runner 🤷

// Whether to pass the actual system environment variables to the JS runtime
IncludeSystemEnvVars null.Bool `json:"includeSystemEnvVars"`

Expand Down