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,18 @@ 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))
+	//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"
 )
@@ -54,7 +55,8 @@ This will execute the test on the Load Impact cloud service. Use "k6 login cloud
         k6 cloud script.js`[1:],
 	Args: exactArgsWithMsg(1, "arg should either be \"-\", if reading script from stdin, or a path to a script file"),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		_, _ = BannerColor.Fprint(stdout, Banner+"\n\n")
+		//TODO: disable in quiet mode?
+		_, _ = BannerColor.Fprintf(stdout, "\n%s\n\n", Banner)
 		initBar := ui.ProgressBar{
 			Width: 60,
 			Left:  func() string { return "    uploading script" },
@@ -235,10 +237,26 @@ 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:
+	// - the default value is specified in this way so we don't overwrire whatever
+	//   was specified via the environment variable
+	// - global variables are not very testable... :/
+	flags.BoolVar(&exitOnRunning, "exit-on-running", exitOnRunning, "exits when test reaches the running status")
+	// We also need to explicitly set the default value for the usage message here, so setting
+	// K6_EXIT_ON_RUNNING=true won't affect the usage message
+	flags.Lookup("exit-on-running").DefValue = "false"
+
+	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/common.go

@@ -68,6 +68,8 @@ func (w consoleWriter) Write(p []byte) (n int, err error) {
 	return
 }
 
+//TODO: refactor the CLI config so these functions aren't needed - they
+// can mask errors by failing only at runtime, not at compile time
 func getNullBool(flags *pflag.FlagSet, key string) null.Bool {
 	v, err := flags.GetBool(key)
 	if err != nil {

cmd/config.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
@@ -22,8 +22,8 @@ package cmd
 
 import (
 	"encoding/json"
-	"io/ioutil"
 	"os"
+	"path/filepath"
 
 	"github.com/kelseyhightower/envconfig"
 	"github.com/loadimpact/k6/lib"
@@ -34,25 +34,12 @@ import (
 	"github.com/loadimpact/k6/stats/kafka"
 	"github.com/loadimpact/k6/stats/statsd/common"
 	"github.com/pkg/errors"
-	"github.com/shibukawa/configdir"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
 	"github.com/spf13/pflag"
 	null "gopkg.in/guregu/null.v3"
 )
 
-const configFilename = "config.json"
-
-var configDirs = configdir.New("loadimpact", "k6")
-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 {
-	flags := pflag.NewFlagSet("", 0)
-	flags.StringVarP(&configFile, "config", "c", configFile, "specify config file to read")
-	return flags
-}
-
 // configFlagSet returns a FlagSet with the default run configuration flags.
 func configFlagSet() *pflag.FlagSet {
 	flags := pflag.NewFlagSet("", 0)
@@ -62,7 +49,6 @@ func configFlagSet() *pflag.FlagSet {
 	flags.Bool("no-usage-report", false, "don't send anonymous stats to the developers")
 	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.AddFlagSet(configFileFlagSet())
 	return flags
 }
 
@@ -129,41 +115,51 @@ func getConfig(flags *pflag.FlagSet) (Config, error) {
 	}, nil
 }
 
-// Reads a configuration file from disk.
-func readDiskConfig(fs afero.Fs) (Config, *configdir.Config, error) {
-	if configFile != "" {
-		data, err := ioutil.ReadFile(configFile)
-		if err != nil {
-			return Config{}, nil, err
-		}
-		var conf Config
-		err = json.Unmarshal(data, &conf)
-		return conf, nil, err
+// Reads the configuration file from the supplied filesystem and returns it and its path.
+// It will first try to see if the user explicitly specified a custom config file and will
+// try to read that. If there's a custom config specified and it couldn't be read or parsed,
+// an error will be returned.
+// If there's no custom config specified and no file exists in the default config path, it will
+// return an empty config struct, the default config location and *no* error.
+func readDiskConfig(fs afero.Fs) (Config, string, error) {
+	realConfigFilePath := configFilePath
+	if realConfigFilePath == "" {
+		// The user didn't specify K6_CONFIG or --config, use the default path
+		realConfigFilePath = defaultConfigFilePath
 	}
 
-	cdir := configDirs.QueryFolderContainsFile(configFilename)
-	if cdir == nil {
-		return Config{}, configDirs.QueryFolders(configdir.Global)[0], nil
+	// Try to see if the file exists in the supplied filesystem
+	if _, err := fs.Stat(realConfigFilePath); err != nil {
+		if os.IsNotExist(err) && configFilePath == "" {
+			// If the file doesn't exist, but it was the default config file (i.e. the user
+			// didn't specify anything), silence the error
+			err = nil
+		}
+		return Config{}, realConfigFilePath, err
 	}
-	data, err := cdir.ReadFile(configFilename)
+
+	data, err := afero.ReadFile(fs, realConfigFilePath)
 	if err != nil {
-		return Config{}, cdir, err
+		return Config{}, realConfigFilePath, err
 	}
 	var conf Config
 	err = json.Unmarshal(data, &conf)
-	return conf, cdir, err
+	return conf, realConfigFilePath, err
 }
 
-// Writes configuration back to disk.
-func writeDiskConfig(fs afero.Fs, cdir *configdir.Config, conf Config) error {
+// Serializes the configuration to a JSON file and writes it in the supplied
+// location on the supplied filesystem
+func writeDiskConfig(fs afero.Fs, configPath string, conf Config) error {
 	data, err := json.MarshalIndent(conf, "", "  ")
 	if err != nil {
 		return err
 	}
-	if configFile != "" {
-		return afero.WriteFile(fs, configFilename, data, 0644)
+
+	if err := fs.MkdirAll(filepath.Dir(configPath), 0755); err != nil {
+		return err
 	}
-	return cdir.WriteFile(configFilename, data)
+
+	return afero.WriteFile(fs, configPath, data, 0644)
 }
 
 // Reads configuration variables from the environment.
@@ -239,15 +235,19 @@ func buildExecutionConfig(conf Config) (Config, error) {
 		ds.VUs = conf.VUs
 		ds.Iterations = conf.Iterations
 		result.Execution = scheduler.ConfigMap{lib.DefaultSchedulerName: ds}
-	} else if conf.Execution != nil {
-		//TODO: remove this warning in the next version
-		log.Warnf("The execution settings are not functional in this k6 release, they will be ignored")
 	} else {
-		// No execution parameters whatsoever were specified, so we'll create a per-VU iterations config
-		// with 1 VU and 1 iteration. We're choosing the per-VU config, since that one could also
-		// be executed both locally, and in the cloud.
-		result.Execution = scheduler.ConfigMap{
-			lib.DefaultSchedulerName: scheduler.NewPerVUIterationsConfig(lib.DefaultSchedulerName),
+		if conf.Execution != nil { // If someone set this, regardless if its empty
+			//TODO: remove this warning in the next version
+			log.Warnf("The execution settings are not functional in this k6 release, they will be ignored")
+		}
+
+		if len(conf.Execution) == 0 { // If unset or set to empty
+			// No execution parameters whatsoever were specified, so we'll create a per-VU iterations config
+			// with 1 VU and 1 iteration. We're choosing the per-VU config, since that one could also
+			// be executed both locally, and in the cloud.
+			result.Execution = scheduler.ConfigMap{
+				lib.DefaultSchedulerName: scheduler.NewPerVUIterationsConfig(lib.DefaultSchedulerName),
+			}
 		}
 	}