Skip to content

Commit

Permalink
Flip --no-system-env-vars to --include-system-env-vars
Browse files Browse the repository at this point in the history
The default value is true for the run command and false for archive and cloud.
  • Loading branch information
na-- committed Feb 7, 2018
1 parent b5bcc13 commit e6dc0fd
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 19 deletions.
3 changes: 2 additions & 1 deletion cmd/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ An archive is a fully self-contained test run, and can be executed identically e

func init() {
RootCmd.AddCommand(archiveCmd)
archiveCmd.Flags().SortFlags = false
archiveCmd.Flags().AddFlagSet(optionFlagSet())
archiveCmd.Flags().AddFlagSet(runtimeOptionFlagSet())
archiveCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false))
archiveCmd.Flags().StringVarP(&archiveOut, "archive-out", "O", archiveOut, "archive output filename")
}
3 changes: 2 additions & 1 deletion cmd/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ This will execute the test on the Load Impact cloud service. Use "k6 login cloud

func init() {
RootCmd.AddCommand(cloudCmd)
cloudCmd.Flags().SortFlags = false
cloudCmd.Flags().AddFlagSet(optionFlagSet())
cloudCmd.Flags().AddFlagSet(runtimeOptionFlagSet())
cloudCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false))
}
2 changes: 1 addition & 1 deletion cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ func init() {

runCmd.Flags().SortFlags = false
runCmd.Flags().AddFlagSet(optionFlagSet())
runCmd.Flags().AddFlagSet(runtimeOptionFlagSet())
runCmd.Flags().AddFlagSet(runtimeOptionFlagSet(true))
runCmd.Flags().AddFlagSet(configFlagSet())
runCmd.Flags().StringVarP(&runType, "type", "t", runType, "override file `type`, \"js\" or \"archive\"")
}
Expand Down
14 changes: 7 additions & 7 deletions cmd/runtime_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,26 @@ func collectEnv() map[string]string {
return env
}

func runtimeOptionFlagSet() *pflag.FlagSet {
func runtimeOptionFlagSet(includeSysEnv bool) *pflag.FlagSet {
flags := pflag.NewFlagSet("", 0)
flags.SortFlags = false
flags.Bool("no-system-env-vars", false, "don't pass actual system environment variables to the runtime")
flags.Bool("include-system-env-vars", includeSysEnv, "pass the real system environment variables to the runtime")
flags.StringSliceP("env", "e", nil, "add/override environment variable with `VAR=value`")
return flags
}

func getRuntimeOptions(flags *pflag.FlagSet) (lib.RuntimeOptions, error) {
opts := lib.RuntimeOptions{
NoSystemEnvVars: getNullBool(flags, "no-system-env-vars"),
Env: make(map[string]string),
IncludeSystemEnvVars: getNullBool(flags, "include-system-env-vars"),
Env: make(map[string]string),
}

// If not disabled, gather the actual system environment variables
if !opts.NoSystemEnvVars.Bool {
// If enabled, gather the actual system environment variables
if opts.IncludeSystemEnvVars.Bool {
opts.Env = collectEnv()
}

// Set/overwrite environment varialbes with custom user-supplied values
// Set/overwrite environment variables with custom user-supplied values
envVars, err := flags.GetStringSlice("env")
if err != nil {
return opts, err
Expand Down
69 changes: 64 additions & 5 deletions cmd/runtime_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

type EnvVarTest struct {
name string
useSysEnv bool // Whether to include the system env vars by default (run) or not (cloud/archive/inspect)
systemEnv map[string]string
cliOpts []string
expErr bool
Expand All @@ -42,69 +43,127 @@ type EnvVarTest struct {
var envVarTestCases = []EnvVarTest{
{
"empty env",
true,
map[string]string{},
[]string{},
false,
map[string]string{},
},
{
"disabled sys env by default",
false,
map[string]string{"test1": "val1"},
[]string{},
false,
map[string]string{},
},
{
"disabled sys env",
"disabled sys env by cli 1",
true,
map[string]string{"test1": "val1"},
[]string{"--include-system-env-vars=false"},
false,
map[string]string{},
},
{
"disabled sys env by cli 2",
true,
map[string]string{"test1": "val1"},
[]string{"--no-system-env-vars"},
[]string{"--include-system-env-vars=0"},
false,
map[string]string{},
},
{
"only system env",
"enabled sys env by default",
true,
map[string]string{"test1": "val1"},
[]string{},
false,
map[string]string{"test1": "val1"},
},
{
"enabled sys env by cli 1",
false,
map[string]string{"test1": "val1"},
[]string{"--include-system-env-vars"},
false,
map[string]string{"test1": "val1"},
},
{
"enabled sys env by cli 2",
false,
map[string]string{"test1": "val1"},
[]string{"--include-system-env-vars=true"},
false,
map[string]string{"test1": "val1"},
},
{
"run only system env",
true,
map[string]string{"test1": "val1"},
[]string{},
false,
map[string]string{"test1": "val1"},
},
{
"mixed system and cli env",
true,
map[string]string{"test1": "val1", "test2": ""},
[]string{"--env", "test3=val3", "-e", "test4", "-e", "test5="},
false,
map[string]string{"test1": "val1", "test2": "", "test3": "val3", "test4": "", "test5": ""},
},
{
"mixed system and cli env 2",
false,
map[string]string{"test1": "val1", "test2": ""},
[]string{"--env", "test3=val3", "-e", "test4", "-e", "test5=", "--include-system-env-vars=1"},
false,
map[string]string{"test1": "val1", "test2": "", "test3": "val3", "test4": "", "test5": ""},
},
{
"disabled system env with cli params",
false,
map[string]string{"test1": "val1"},
[]string{"-e", "test2=overwriten", "-e", "test2=val2", "--no-system-env-vars"},
[]string{"-e", "test2=overwriten", "-e", "test2=val2"},
false,
map[string]string{"test2": "val2"},
},
{
"overwriting system env with cli param",
true,
map[string]string{"test1": "val1sys"},
[]string{"--env", "test1=val1cli"},
false,
map[string]string{"test1": "val1cli"},
},
{
"error invalid cli var name 1",
true,
map[string]string{},
[]string{"--env", "test a=error"},
true,
map[string]string{},
},
{
"error invalid cli var name 2",
true,
map[string]string{},
[]string{"--env", "1var=error"},
true,
map[string]string{},
},
{
"error invalid cli var name 3",
true,
map[string]string{},
[]string{"--env", "уникод=unicode-disabled"},
true,
map[string]string{},
},
{
"valid env vars with spaces",
true,
map[string]string{"test1": "value 1"},
[]string{"--env", "test2=value 2"},
false,
Expand All @@ -119,7 +178,7 @@ func TestEnvVars(t *testing.T) {
for key, val := range tc.systemEnv {
require.NoError(t, os.Setenv(key, val))
}
flags := runtimeOptionFlagSet()
flags := runtimeOptionFlagSet(tc.useSysEnv)
require.NoError(t, flags.Parse(tc.cliOpts))

rtOpts, err := getRuntimeOptions(flags)
Expand Down
8 changes: 4 additions & 4 deletions lib/runtime_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import null "gopkg.in/guregu/null.v3"

// RuntimeOptions are settings passed onto the goja JS runtime
type RuntimeOptions struct {
// Whether to disable the passing of actual system environment variables to the JS runtime
NoSystemEnvVars null.Bool `json:"noSystemEnvVars" envconfig:"no_system_env_vars"`
// Whether to pass the actual system environment variables to the JS runtime
IncludeSystemEnvVars null.Bool `json:"includeSystemEnvVars" envconfig:"include_system_env_vars"`

// Environment variables passed onto the runner
Env map[string]string `json:"env" envconfig:"env"`
Expand All @@ -34,8 +34,8 @@ type RuntimeOptions struct {
// Apply overwrites the receiver RuntimeOptions' fields with any that are set
// on the argument struct and returns the receiver
func (o RuntimeOptions) Apply(opts RuntimeOptions) RuntimeOptions {
if opts.NoSystemEnvVars.Valid {
o.NoSystemEnvVars = opts.NoSystemEnvVars
if opts.IncludeSystemEnvVars.Valid {
o.IncludeSystemEnvVars = opts.IncludeSystemEnvVars
}
if opts.Env != nil {
o.Env = opts.Env
Expand Down

0 comments on commit e6dc0fd

Please sign in to comment.