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

Config refactoring and tests #935

Merged
merged 17 commits into from
Mar 8, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cmd/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ func archiveCmdFlagSet() *pflag.FlagSet {
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
Expand Down
57 changes: 20 additions & 37 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package cmd

import (
"encoding/json"
"io/ioutil"
"os"

"github.com/kelseyhightower/envconfig"
Expand All @@ -34,26 +33,12 @@ import (
"github.com/loadimpact/k6/stats/kafka"
"github.com/loadimpact/k6/stats/statsd/common"
"github.com/pkg/errors"
"github.com/shibukawa/configdir"
na-- marked this conversation as resolved.
Show resolved Hide resolved
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 {
//TODO: remove?
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)
Expand All @@ -63,7 +48,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
}

Expand Down Expand Up @@ -130,41 +114,40 @@ 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 a configuration file from disk and returns it and its path.
na-- marked this conversation as resolved.
Show resolved Hide resolved
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.
na-- marked this conversation as resolved.
Show resolved Hide resolved
func writeDiskConfig(fs afero.Fs, cdir *configdir.Config, conf Config) error {
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)
}
return cdir.WriteFile(configFilename, data)
return afero.WriteFile(fs, configPath, data, 0644)
}

// Reads configuration variables from the environment.
Expand Down
4 changes: 2 additions & 2 deletions cmd/login_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`,
return err
}

currentDiskConf, cdir, err := readDiskConfig(fs)
currentDiskConf, configPath, err := readDiskConfig(fs)
if err != nil {
return err
}
Expand Down Expand Up @@ -109,7 +109,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`,
}

currentDiskConf.Collectors.Cloud = newCloudConf
if err := writeDiskConfig(fs, cdir, currentDiskConf); err != nil {
if err := writeDiskConfig(fs, configPath, currentDiskConf); err != nil {
return err
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/login_influxdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ This will set the default server used when just "-o influxdb" is passed.`,
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
fs := afero.NewOsFs()
config, cdir, err := readDiskConfig(fs)
config, configPath, err := readDiskConfig(fs)
if err != nil {
return err
}
Expand Down Expand Up @@ -105,7 +105,7 @@ This will set the default server used when just "-o influxdb" is passed.`,
}

config.Collectors.InfluxDB = conf
return writeDiskConfig(fs, cdir, config)
return writeDiskConfig(fs, configPath, config)
},
}

Expand Down
15 changes: 10 additions & 5 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,12 @@ var (
stderr = consoleWriter{colorable.NewColorableStderr(), stderrTTY, outMutex}
)

var (
cfgFile string
const defaultConfigFileName = "config.json"

var defaultConfigFilePath = defaultConfigFileName // Updated with the user's config folder in the init() function below
var configFilePath = os.Getenv("K6_CONFIG") // Overridden by `-c`/`--config` flag!

var (
verbose bool
quiet bool
noColor bool
Expand Down Expand Up @@ -94,10 +97,12 @@ func Execute() {
}

func init() {
defaultConfigPathMsg := ""
// TODO: find a better library... or better yet, simply port the few dozen lines of code for getting the
// per-user config folder in a cross-platform way
configDirs := configdir.New("loadimpact", "k6")
configFolders := configDirs.QueryFolders(configdir.Global)
if len(configFolders) > 0 {
defaultConfigPathMsg = fmt.Sprintf(" (default %s)", filepath.Join(configFolders[0].Path, configFilename))
defaultConfigFilePath = filepath.Join(configFolders[0].Path, defaultConfigFileName)
}

//TODO: figure out a better way to handle the CLI flags - global variables are not very testable... :/
Expand All @@ -106,7 +111,7 @@ func init() {
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) //TODO: figure out and fix?
RootCmd.PersistentFlags().StringVarP(&configFilePath, "config", "c", "", fmt.Sprintf("config file (default %s)", defaultConfigFilePath))
must(cobra.MarkFlagFilename(RootCmd.PersistentFlags(), "config"))
}

Expand Down