From 977b10e76345905c9e2ea5da9385a2d9b20ecce6 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Fri, 1 Mar 2019 19:03:50 +0200 Subject: [PATCH] Refactor some CLI configs and add some basic configuration tests This is woefully insufficient, but even that was painful to write... Further refactoring and tests will follow, especially relating to JSON/JS configs and mixing configuration between the CLI/env-vars/JS/JSON layers. The effort will hopefully be worth it, since we should be able to reuse these tests to verify that we're not breaking things when we finally tackle https://github.com/loadimpact/k6/issues/883 properly. --- cmd/archive.go | 17 +++- cmd/cloud.go | 15 ++- cmd/config.go | 1 + cmd/config_test.go | 242 +++++++++++++++++++++++++++++++++++++++++++-- cmd/root.go | 3 +- cmd/run.go | 22 +++-- 6 files changed, 279 insertions(+), 21 deletions(-) diff --git a/cmd/archive.go b/cmd/archive.go index ede0553929c..8263715870f 100644 --- a/cmd/archive.go +++ b/cmd/archive.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/afero" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) var archiveOut = "archive.tar" @@ -90,11 +91,19 @@ An archive is a fully self-contained test run, and can be executed identically e }, } +func archiveCmdFlagSet() *pflag.FlagSet { + flags := pflag.NewFlagSet("", pflag.ContinueOnError) + flags.SortFlags = false + flags.AddFlagSet(optionFlagSet()) + flags.AddFlagSet(runtimeOptionFlagSet(false)) + flags.AddFlagSet(configFileFlagSet()) + //TODO: figure out a better way to handle the CLI flags - global variables are not very testable... :/ + flags.StringVarP(&archiveOut, "archive-out", "O", archiveOut, "archive output filename") + return flags +} + func init() { RootCmd.AddCommand(archiveCmd) archiveCmd.Flags().SortFlags = false - archiveCmd.Flags().AddFlagSet(optionFlagSet()) - archiveCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false)) - archiveCmd.Flags().AddFlagSet(configFileFlagSet()) - archiveCmd.Flags().StringVarP(&archiveOut, "archive-out", "O", archiveOut, "archive output filename") + archiveCmd.Flags().AddFlagSet(archiveCmdFlagSet()) } diff --git a/cmd/cloud.go b/cmd/cloud.go index 1850f27d93e..27f3c01c6fc 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -36,6 +36,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/afero" "github.com/spf13/cobra" + "github.com/spf13/pflag" log "github.com/sirupsen/logrus" ) @@ -235,10 +236,18 @@ This will execute the test on the Load Impact cloud service. Use "k6 login cloud }, } +func cloudCmdFlagSet() *pflag.FlagSet { + flags := pflag.NewFlagSet("", pflag.ContinueOnError) + flags.SortFlags = false + flags.AddFlagSet(optionFlagSet()) + flags.AddFlagSet(runtimeOptionFlagSet(false)) + //TODO: figure out a better way to handle the CLI flags - global variables are not very testable... :/ + flags.BoolVar(&exitOnRunning, "exit-on-running", exitOnRunning, "exits when test reaches the running status") + return flags +} + func init() { RootCmd.AddCommand(cloudCmd) cloudCmd.Flags().SortFlags = false - cloudCmd.Flags().AddFlagSet(optionFlagSet()) - cloudCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false)) - cloudCmd.Flags().BoolVar(&exitOnRunning, "exit-on-running", exitOnRunning, "exits when test reaches the running status") + cloudCmd.Flags().AddFlagSet(cloudCmdFlagSet()) } diff --git a/cmd/config.go b/cmd/config.go index 21ae22f6fd2..7d92536f2dd 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -48,6 +48,7 @@ var configFile = os.Getenv("K6_CONFIG") // overridden by `-c` flag! // configFileFlagSet returns a FlagSet that contains flags needed for specifying a config file. func configFileFlagSet() *pflag.FlagSet { + //TODO: remove? flags := pflag.NewFlagSet("", 0) flags.StringVarP(&configFile, "config", "c", configFile, "specify config file to read") return flags diff --git a/cmd/config_test.go b/cmd/config_test.go index a82daed850f..b0db5ebde60 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -1,7 +1,7 @@ /* * * k6 - a next-generation load testing tool - * Copyright (C) 2016 Load Impact + * Copyright (C) 2019 Load Impact * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -21,11 +21,25 @@ package cmd import ( + "fmt" + "io/ioutil" "os" + "strings" + "sync" "testing" + "time" + + "github.com/loadimpact/k6/lib/scheduler" + "github.com/loadimpact/k6/lib/types" + + "github.com/spf13/afero" + "github.com/spf13/pflag" "github.com/kelseyhightower/envconfig" + "github.com/loadimpact/k6/lib" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "gopkg.in/guregu/null.v3" ) @@ -83,8 +97,227 @@ func TestConfigCmd(t *testing.T) { } } -//TODO: write end-to-end configuration tests - how the config file, in-script options, environment variables and CLI flags are parsed and interact... and how the end result is then propagated back into the script +// A simple logrus hook that could be used to check if log messages were outputted +type simpleLogrusHook struct { + mutex sync.Mutex + levels []log.Level + messageCache []log.Entry +} + +func (smh *simpleLogrusHook) Levels() []log.Level { + return smh.levels +} +func (smh *simpleLogrusHook) Fire(e *log.Entry) error { + smh.mutex.Lock() + defer smh.mutex.Unlock() + smh.messageCache = append(smh.messageCache, *e) + return nil +} + +func (smh *simpleLogrusHook) drain() []log.Entry { + smh.mutex.Lock() + defer smh.mutex.Unlock() + res := smh.messageCache + smh.messageCache = []log.Entry{} + return res +} + +var _ log.Hook = &simpleLogrusHook{} + +// A helper funcion for setting arbitrary environment variables and +// restoring the old ones at the end, usually by deferring the returned callback +//TODO: remove these hacks when we improve the configuration... we shouldn't +// have to mess with the global environment at all... +func setEnv(t *testing.T, newEnv []string) (restoreEnv func()) { + actuallSetEnv := func(env []string) { + os.Clearenv() + for _, e := range env { + val := "" + pair := strings.SplitN(e, "=", 2) + if len(pair) > 1 { + val = pair[1] + } + require.NoError(t, os.Setenv(pair[0], val)) + } + } + oldEnv := os.Environ() + actuallSetEnv(newEnv) + + return func() { + actuallSetEnv(oldEnv) + } +} + +var verifyOneIterPerOneVU = func(t *testing.T, c Config) { + // No config anywhere should result in a 1 VU with a 1 uninterruptible iteration config + sched := c.Execution[lib.DefaultSchedulerName] + require.NotEmpty(t, sched) + require.IsType(t, scheduler.PerVUIteationsConfig{}, sched) + perVuIters, ok := sched.(scheduler.PerVUIteationsConfig) + require.True(t, ok) + assert.Equal(t, null.NewBool(false, false), perVuIters.Interruptible) + assert.Equal(t, null.NewInt(1, false), perVuIters.Iterations) + assert.Equal(t, null.NewInt(1, false), perVuIters.VUs) +} + +var verifySharedIters = func(vus, iters int64) func(t *testing.T, c Config) { + return func(t *testing.T, c Config) { + sched := c.Execution[lib.DefaultSchedulerName] + require.NotEmpty(t, sched) + require.IsType(t, scheduler.SharedIteationsConfig{}, sched) + sharedIterConfig, ok := sched.(scheduler.SharedIteationsConfig) + require.True(t, ok) + assert.Equal(t, null.NewInt(vus, true), sharedIterConfig.VUs) + assert.Equal(t, null.NewInt(iters, true), sharedIterConfig.Iterations) + } +} + +var verifyConstantLoopingVUs = func(vus int64, duration time.Duration) func(t *testing.T, c Config) { + return func(t *testing.T, c Config) { + sched := c.Execution[lib.DefaultSchedulerName] + require.NotEmpty(t, sched) + require.IsType(t, scheduler.ConstantLoopingVUsConfig{}, sched) + clvc, ok := sched.(scheduler.ConstantLoopingVUsConfig) + require.True(t, ok) + assert.Equal(t, null.NewBool(true, false), clvc.Interruptible) + assert.Equal(t, null.NewInt(vus, true), clvc.VUs) + assert.Equal(t, types.NullDurationFrom(duration), clvc.Duration) + } +} + +func mostFlagSets() []*pflag.FlagSet { + // sigh... compromises... + return []*pflag.FlagSet{runCmdFlagSet(), archiveCmdFlagSet(), cloudCmdFlagSet()} +} + +// exp contains the different events or errors we expect our test case to trigger. +// for space and clarity, we use the fact that by default, all of the struct values are false +type exp struct { + cliError bool + consolidationError bool + validationErrors bool + logWarning bool //TODO: remove in the next version? +} + +// A hell of a complicated test case, that still doesn't test things fully... +type configConsolidationTestCase struct { + cliFlagSets []*pflag.FlagSet + cliFlagValues []string + env []string + runnerOptions *lib.Options + //TODO: test the JSON config as well... after most of https://github.com/loadimpact/k6/issues/883#issuecomment-468646291 is fixed + expected exp + customValidator func(t *testing.T, c Config) +} + +var configConsolidationTestCases = []configConsolidationTestCase{ + // Check that no options will result in 1 VU 1 iter value for execution + {mostFlagSets(), nil, nil, nil, exp{}, verifyOneIterPerOneVU}, + // Verify some CLI errors + {mostFlagSets(), []string{"--blah", "blah"}, nil, nil, exp{cliError: true}, nil}, + {mostFlagSets(), []string{"--duration", "blah"}, nil, nil, exp{cliError: true}, nil}, + {mostFlagSets(), []string{"--iterations", "blah"}, nil, nil, exp{cliError: true}, nil}, + {mostFlagSets(), []string{"--execution", ""}, nil, nil, exp{cliError: true}, nil}, + {mostFlagSets(), []string{"--stage", "10:20s"}, nil, nil, exp{cliError: true}, nil}, + // Check if CLI shortcuts generate correct execution values + {mostFlagSets(), []string{"--vus", "1", "--iterations", "5"}, nil, nil, exp{}, verifySharedIters(1, 5)}, + {mostFlagSets(), []string{"-u", "2", "-i", "6"}, nil, nil, exp{}, verifySharedIters(2, 6)}, + {mostFlagSets(), []string{"-u", "3", "-d", "30s"}, nil, nil, exp{}, verifyConstantLoopingVUs(3, 30*time.Second)}, + {mostFlagSets(), []string{"-u", "4", "--duration", "60s"}, nil, nil, exp{}, verifyConstantLoopingVUs(4, 1*time.Minute)}, + //TODO: verify stages + // This should get a validation error since VUs are more than the shared iterations + {mostFlagSets(), []string{"--vus", "10", "-i", "6"}, nil, nil, exp{validationErrors: true}, verifySharedIters(10, 6)}, + // These should emit a warning + {mostFlagSets(), []string{"-u", "1", "-i", "6", "-d", "10s"}, nil, nil, exp{logWarning: true}, nil}, + {mostFlagSets(), []string{"-u", "2", "-d", "10s", "-s", "10s:20"}, nil, nil, exp{logWarning: true}, nil}, + {mostFlagSets(), []string{"-u", "3", "-i", "5", "-s", "10s:20"}, nil, nil, exp{logWarning: true}, nil}, + {mostFlagSets(), []string{"-u", "3", "-d", "0"}, nil, nil, exp{logWarning: true}, nil}, + // Test if environment variable shortcuts are working as expected + {mostFlagSets(), nil, []string{"K6_VUS=5", "K6_ITERATIONS=15"}, nil, exp{}, verifySharedIters(5, 15)}, + {mostFlagSets(), nil, []string{"K6_VUS=10", "K6_DURATION=20s"}, nil, exp{}, verifyConstantLoopingVUs(10, 20*time.Second)}, + + //TODO: test combinations between options and levels + //TODO: test the future full overwriting of the duration/iterations/stages/execution options + + // Just in case, verify that no options will result in the same 1 vu 1 iter config + {mostFlagSets(), nil, nil, nil, exp{}, verifyOneIterPerOneVU}, + //TODO: test for differences between flagsets + //TODO: more tests in general... +} + +func TestConfigConsolidation(t *testing.T) { + logHook := simpleLogrusHook{levels: []log.Level{log.WarnLevel}} + log.AddHook(&logHook) + log.SetOutput(ioutil.Discard) + defer log.SetOutput(os.Stderr) + + runTestCase := func(t *testing.T, testCase configConsolidationTestCase, flagSet *pflag.FlagSet) { + logHook.drain() + + restoreEnv := setEnv(t, testCase.env) + defer restoreEnv() + + //TODO: also remove these hacks when we improve the configuration... + getTestCaseCliConf := func() (Config, error) { + if err := flagSet.Parse(testCase.cliFlagValues); err != nil { + return Config{}, err + } + if flagSet.Lookup("out") != nil { + return getConfig(flagSet) + } + opts, errOpts := getOptions(flagSet) + return Config{Options: opts}, errOpts + } + + cliConf, err := getTestCaseCliConf() + if testCase.expected.cliError { + require.Error(t, err) + return + } + require.NoError(t, err) + + var runner lib.Runner + if testCase.runnerOptions != nil { + runner = &lib.MiniRunner{Options: *testCase.runnerOptions} + } + fs := afero.NewMemMapFs() //TODO: test JSON configs as well! + result, err := getConsolidatedConfig(fs, cliConf, runner) + if testCase.expected.consolidationError { + require.Error(t, err) + return + } + require.NoError(t, err) + warnings := logHook.drain() + if testCase.expected.logWarning { + assert.NotEmpty(t, warnings) + } else { + assert.Empty(t, warnings) + } + + validationErrors := result.Validate() + if testCase.expected.validationErrors { + assert.NotEmpty(t, validationErrors) + } else { + assert.Empty(t, validationErrors) + } + + if testCase.customValidator != nil { + testCase.customValidator(t, result) + } + } + + for tcNum, testCase := range configConsolidationTestCases { + for fsNum, flagSet := range testCase.cliFlagSets { + // I want to paralelize this, but I cannot... due to global variables and other + // questionable architectural choices... :| + t.Run( + fmt.Sprintf("TestCase#%d_FlagSet#%d", tcNum, fsNum), + func(t *testing.T) { runTestCase(t, testCase, flagSet) }, + ) + } + } +} func TestConfigEnv(t *testing.T) { testdata := map[struct{ Name, Key string }]map[string]func(Config){ {"Linger", "K6_LINGER"}: { @@ -134,8 +367,3 @@ func TestConfigApply(t *testing.T) { assert.Equal(t, []string{"influxdb", "json"}, conf.Out) }) } - -func TestBuildExecutionConfig(t *testing.T) { - //TODO: test the current config building and constructing of the execution plan, and the emitted warnings - //TODO: test the future full overwriting of the duration/iterations/stages/execution options -} diff --git a/cmd/root.go b/cmd/root.go index 521ba5c4be3..715e2cb2f78 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -100,12 +100,13 @@ func init() { defaultConfigPathMsg = fmt.Sprintf(" (default %s)", filepath.Join(configFolders[0].Path, configFilename)) } + //TODO: figure out a better way to handle the CLI flags - global variables are not very testable... :/ RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "enable debug logging") RootCmd.PersistentFlags().BoolVarP(&quiet, "quiet", "q", false, "disable progress updates") RootCmd.PersistentFlags().BoolVar(&noColor, "no-color", false, "disable colored output") RootCmd.PersistentFlags().StringVar(&logFmt, "logformat", "", "log output format") RootCmd.PersistentFlags().StringVarP(&address, "address", "a", "localhost:6565", "address for the api server") - RootCmd.PersistentFlags().StringVarP(&cfgFile, "config", "c", "", "config file"+defaultConfigPathMsg) + RootCmd.PersistentFlags().StringVarP(&cfgFile, "config", "c", "", "config file"+defaultConfigPathMsg) //TODO: figure out and fix? must(cobra.MarkFlagFilename(RootCmd.PersistentFlags(), "config")) } diff --git a/cmd/run.go b/cmd/run.go index ba63d5a1b6b..6e1c1612906 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -49,6 +49,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/spf13/cobra" + "github.com/spf13/pflag" null "gopkg.in/guregu/null.v3" ) @@ -65,6 +66,7 @@ const ( ) var ( + //TODO: fix this, global variables are not very testable... runType = os.Getenv("K6_TYPE") runNoSetup = os.Getenv("K6_NO_SETUP") != "" runNoTeardown = os.Getenv("K6_NO_TEARDOWN") != "" @@ -472,16 +474,24 @@ a commandline interface for interacting with it.`, }, } +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 - global variables are not very testable... :/ + flags.StringVarP(&runType, "type", "t", runType, "override file `type`, \"js\" or \"archive\"") + flags.BoolVar(&runNoSetup, "no-setup", runNoSetup, "don't run setup()") + flags.BoolVar(&runNoTeardown, "no-teardown", runNoTeardown, "don't run teardown()") + return flags +} + func init() { RootCmd.AddCommand(runCmd) runCmd.Flags().SortFlags = false - runCmd.Flags().AddFlagSet(optionFlagSet()) - runCmd.Flags().AddFlagSet(runtimeOptionFlagSet(true)) - runCmd.Flags().AddFlagSet(configFlagSet()) - runCmd.Flags().StringVarP(&runType, "type", "t", runType, "override file `type`, \"js\" or \"archive\"") - runCmd.Flags().BoolVar(&runNoSetup, "no-setup", runNoSetup, "don't run setup()") - runCmd.Flags().BoolVar(&runNoTeardown, "no-teardown", runNoTeardown, "don't run teardown()") + runCmd.Flags().AddFlagSet(runCmdFlagSet()) } // Reads a source file from any supported destination.