Config refactoring and tests

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())
 }

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())
 }

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

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,239 @@ 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)
+	//TODO: verify shortcut options as well?
+}
+
+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)
+		//TODO: verify shortcut options as well?
+	}
+}
+
+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)
+		//TODO: verify shortcut options as well?
+	}
+}
+
+func mostFlagSets() []*pflag.FlagSet {
+	// sigh... compromises...
+	return []*pflag.FlagSet{runCmdFlagSet(), archiveCmdFlagSet(), cloudCmdFlagSet()}
+}
+
+type opts struct {
+	cliFlagSets []*pflag.FlagSet
+	cli         []string
+	env         []string
+	runner      *lib.Options
+	//TODO: test the JSON config as well... after most of https://github.com/loadimpact/k6/issues/883#issuecomment-468646291 is fixed
+}
+
+// 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 {
+	options         opts
+	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
+	{opts{}, exp{}, verifyOneIterPerOneVU},
+	// Verify some CLI errors
+	{opts{cli: []string{"--blah", "blah"}}, exp{cliError: true}, nil},
+	{opts{cli: []string{"--duration", "blah"}}, exp{cliError: true}, nil},
+	{opts{cli: []string{"--iterations", "blah"}}, exp{cliError: true}, nil},
+	{opts{cli: []string{"--execution", ""}}, exp{cliError: true}, nil},
+	{opts{cli: []string{"--stage", "10:20s"}}, exp{cliError: true}, nil},
+	// Check if CLI shortcuts generate correct execution values
+	{opts{cli: []string{"--vus", "1", "--iterations", "5"}}, exp{}, verifySharedIters(1, 5)},
+	{opts{cli: []string{"-u", "2", "-i", "6"}}, exp{}, verifySharedIters(2, 6)},
+	{opts{cli: []string{"-u", "3", "-d", "30s"}}, exp{}, verifyConstantLoopingVUs(3, 30*time.Second)},
+	{opts{cli: []string{"-u", "4", "--duration", "60s"}}, exp{}, verifyConstantLoopingVUs(4, 1*time.Minute)},
+	//TODO: verify stages
+	// This should get a validation error since VUs are more than the shared iterations
+	{opts{cli: []string{"--vus", "10", "-i", "6"}}, exp{validationErrors: true}, verifySharedIters(10, 6)},
+	// These should emit a warning
+	{opts{cli: []string{"-u", "1", "-i", "6", "-d", "10s"}}, exp{logWarning: true}, nil},
+	{opts{cli: []string{"-u", "2", "-d", "10s", "-s", "10s:20"}}, exp{logWarning: true}, nil},
+	{opts{cli: []string{"-u", "3", "-i", "5", "-s", "10s:20"}}, exp{logWarning: true}, nil},
+	{opts{cli: []string{"-u", "3", "-d", "0"}}, exp{logWarning: true}, nil},
+	// Test if environment variable shortcuts are working as expected
+	{opts{env: []string{"K6_VUS=5", "K6_ITERATIONS=15"}}, exp{}, verifySharedIters(5, 15)},
+	{opts{env: []string{"K6_VUS=10", "K6_DURATION=20s"}}, 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
+	{opts{}, 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) {
+		t.Logf("Test with opts=%#v and exp=%#v\n", testCase.options, testCase.expected)
+		logHook.drain()
+
+		restoreEnv := setEnv(t, testCase.options.env)
+		defer restoreEnv()
+
+		//TODO: also remove these hacks when we improve the configuration...
+		getTestCaseCliConf := func() (Config, error) {
+			if err := flagSet.Parse(testCase.options.cli); 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.options.runner != nil {
+			runner = &lib.MiniRunner{Options: *testCase.options.runner}
+		}
+		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 {
+		flagSets := testCase.options.cliFlagSets
+		if flagSets == nil { // handle the most common case
+			flagSets = mostFlagSets()
+		}
+		for fsNum, flagSet := range flagSets {
+			// 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 +379,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
-}

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"))
 }